mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(scripts): tolerate an unusable integration.json in the Python helper (#3785)
* fix(scripts): tolerate an unusable integration.json in the Python helper
`get_invoke_separator()` in scripts/python/common.py indexed the parsed JSON
directly, so two shapes escaped its `except (OSError, json.JSONDecodeError)`
while BOTH of its twins fall back to "." for them:
* A non-mapping top level is valid JSON, so JSONDecodeError never fires and
`state.get(...)` raised AttributeError.
* A non-UTF-8 file raises UnicodeDecodeError -- a ValueError, not an OSError.
Realistic on Windows, where PowerShell 5.1's Out-File/`>` default to UTF-16.
Measured on main -- 6 of 7 inputs crashed the Python helper while bash and
PowerShell 5.1 returned "." for every one:
input python bash pwsh 5.1
{"default_integration":"forge"} '.' . .
[] AttributeError . .
"forge" AttributeError . .
42 AttributeError . .
null AttributeError . .
UTF-16 file UnicodeDecodeError . .
Split the parse out of the lookup, complete the exception tuple, and guard the
top-level shape -- matching `read_feature_json_feature_directory` in this same
module, which already does exactly this. The hyphen-separator feature is
unchanged (regression test included).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(scripts): point the parity comment at the sibling above, not below
read_feature_json_feature_directory is defined at line 81, above
get_invoke_separator, so "below" sent maintainers the wrong way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -258,16 +258,27 @@ def get_invoke_separator(repo_root: Path) -> str:
|
||||
integration_json = repo_root / ".specify" / "integration.json"
|
||||
if not integration_json.is_file():
|
||||
return "."
|
||||
# Split the parse out of the lookup and guard the top-level shape, matching
|
||||
# read_feature_json_feature_directory above and the bash/PowerShell twins,
|
||||
# which both fall back to "." for any unusable integration.json:
|
||||
# * a non-mapping top level ([], "forge", 42, null) is valid JSON, so
|
||||
# json.JSONDecodeError never fires and state.get(...) raised
|
||||
# AttributeError;
|
||||
# * a non-UTF-8 file raises UnicodeDecodeError, which is a ValueError --
|
||||
# not an OSError -- so it escaped the except tuple. Realistic on
|
||||
# Windows, where PowerShell 5.1's Out-File/`>` default to UTF-16.
|
||||
try:
|
||||
state = json.loads(integration_json.read_text(encoding="utf-8"))
|
||||
key = state.get("default_integration") or state.get("integration") or ""
|
||||
settings = state.get("integration_settings")
|
||||
if isinstance(key, str) and isinstance(settings, dict):
|
||||
entry = settings.get(key)
|
||||
if isinstance(entry, dict) and entry.get("invoke_separator") in {".", "-"}:
|
||||
return entry["invoke_separator"]
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
except (OSError, UnicodeError, json.JSONDecodeError):
|
||||
return "."
|
||||
if not isinstance(state, dict):
|
||||
return "."
|
||||
key = state.get("default_integration") or state.get("integration") or ""
|
||||
settings = state.get("integration_settings")
|
||||
if isinstance(key, str) and isinstance(settings, dict):
|
||||
entry = settings.get(key)
|
||||
if isinstance(entry, dict) and entry.get("invoke_separator") in {".", "-"}:
|
||||
return entry["invoke_separator"]
|
||||
return "."
|
||||
|
||||
|
||||
|
||||
@@ -370,3 +370,69 @@ def test_python_branch_falls_back_to_feature_dir_basename(prereq_repo: Path) ->
|
||||
|
||||
assert py.returncode == 0, py.stderr
|
||||
assert _json_stdout(py)["BRANCH"] == "001-my-feature"
|
||||
|
||||
|
||||
class TestGetInvokeSeparatorTolerance:
|
||||
"""`get_invoke_separator` must fall back to "." for an unusable
|
||||
`integration.json`, matching its bash and PowerShell twins.
|
||||
|
||||
The bash twin tries jq -> python3 -> awk and keeps its `separator="."`
|
||||
default on any parse failure; the PowerShell twin likewise returns ".".
|
||||
The Python twin instead indexed the parsed value directly, so two shapes
|
||||
escaped its `except (OSError, json.JSONDecodeError)`:
|
||||
|
||||
* a non-mapping top level (`[]`, `"forge"`, `42`, `null`) is valid JSON,
|
||||
so JSONDecodeError never fires and `.get()` raised AttributeError;
|
||||
* a non-UTF-8 file raises UnicodeDecodeError -- a ValueError, not an
|
||||
OSError. Realistic on Windows, where PowerShell 5.1's `Out-File`/`>`
|
||||
default to UTF-16.
|
||||
|
||||
The sibling `read_feature_json_feature_directory` in the same module
|
||||
already guards both.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _load_common():
|
||||
import importlib.util
|
||||
|
||||
spec = importlib.util.spec_from_file_location("_speckit_common_py", COMMON_PY)
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
# Register before exec: the module defines @dataclass types, and
|
||||
# dataclasses resolves cls.__module__ through sys.modules.
|
||||
sys.modules[spec.name] = module
|
||||
try:
|
||||
spec.loader.exec_module(module)
|
||||
except Exception: # pragma: no cover - defensive cleanup
|
||||
sys.modules.pop(spec.name, None)
|
||||
raise
|
||||
return module
|
||||
|
||||
def _repo(self, tmp_path: Path, body: str | bytes) -> Path:
|
||||
(tmp_path / ".specify").mkdir(parents=True, exist_ok=True)
|
||||
target = tmp_path / ".specify" / "integration.json"
|
||||
if isinstance(body, bytes):
|
||||
target.write_bytes(body)
|
||||
else:
|
||||
target.write_text(body, encoding="utf-8")
|
||||
return tmp_path
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"body", ["[]", '[{"a": 1}]', '"forge"', "42", "true", "null"]
|
||||
)
|
||||
def test_non_mapping_integration_json_falls_back(self, tmp_path: Path, body: str):
|
||||
common = self._load_common()
|
||||
assert common.get_invoke_separator(self._repo(tmp_path, body)) == "."
|
||||
|
||||
def test_non_utf8_integration_json_falls_back(self, tmp_path: Path):
|
||||
common = self._load_common()
|
||||
raw = '{"default_integration": "forge"}'.encode("utf-16")
|
||||
assert common.get_invoke_separator(self._repo(tmp_path, raw)) == "."
|
||||
|
||||
def test_hyphen_separator_is_still_honoured(self, tmp_path: Path):
|
||||
"""Regression guard: the real feature must keep working."""
|
||||
common = self._load_common()
|
||||
body = json.dumps({
|
||||
"default_integration": "droid",
|
||||
"integration_settings": {"droid": {"invoke_separator": "-"}},
|
||||
})
|
||||
assert common.get_invoke_separator(self._repo(tmp_path, body)) == "-"
|
||||
|
||||
Reference in New Issue
Block a user