From 5efeb4e85b539e1e74ab78b90f121b136063e46b Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 12:59:21 -0700 Subject: [PATCH 1/4] Delegate configure.sh's declared-description validation to Python too; close two smaller gaps Real fixes, from PR #914's fresh review round after #915 merged: - repo-config/configure.sh: the bash/jq validation checked emptiness, edge whitespace, and embedded newlines, but never checked for Markdown links or the 100-char cap - so a value like 'See [docs](url)' or a 150-character string would pass configure.sh and reach the About payload, even though spec/audit.py's description_findings() (already delegating to validate.py as of 827b9d4) would immediately flag it as a DEFECT. Rather than adding a fourth hand-rolled partial copy of the same rules, configure.sh now shells out to a new spec/resolve_description.py, which calls spec/validate.py's description_errors() directly - the same function spec/audit.py already uses. This also simplifies the script: since a value that passes that check can never contain a newline, the sentinel-preservation dance from three commits ago is no longer needed at all. Manually verified against 11 description shapes end to end, including the two this specifically closes (a Markdown link, an over-cap string). - spec/audit.py: description_findings() passed the literal string 'registry' into validate.description_errors() instead of the actual repo slug, so every DEFECT for an invalid declared description read identically regardless of which repo had the problem - not actionable in a fleet-wide audit run. Now passes slug. New selftest case locks in that the message names the repo. - spec/validate.py: added a check that a registry entry's name itself carries no leading/trailing whitespace, since both configure.sh and audit.py use it as an exact-match lookup key - a padded name would make the entry silently unresolvable there, and would also defeat the duplicate-name check added in the previous commit. Manually verified against a crafted padded name; the real registry is clean. Declined (reasoning posted to PR #914): - CodeRabbit's sentence-length finding on spec/readme-structure.md - same pervasive pre-existing violation already declined in an earlier round. - CodeRabbit's type-annotation request for description_errors_for_repo - this repo's pyproject.toml explicitly runs the lint-only Scripts profile (pyright 'standard', not 'strict'), and none of the other seven functions in spec/validate.py carry any type hints either. --- repo-config/configure.sh | 39 ++----------------------- spec/audit.py | 8 ++++- spec/resolve_description.py | 58 +++++++++++++++++++++++++++++++++++++ spec/validate.py | 4 +++ 4 files changed, 72 insertions(+), 37 deletions(-) create mode 100755 spec/resolve_description.py diff --git a/repo-config/configure.sh b/repo-config/configure.sh index 4995ad59..5193f915 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -68,44 +68,11 @@ settings_file="$script_dir/settings.json" # Absence keeps the About panel following the README. description="" if [ -f "$registry" ]; then - # Fails loud on a duplicate name (already a validate.py DEFECT) rather than picking one entry over the other. - if ! match_count="$(jq -r --arg n "$name" '[.repos[] | select(.name==$n)] | length' "$registry")"; then - echo "Failed to read $registry (invalid JSON?)." >&2 + # Delegates to spec/resolve_description.py rather than a third hand-rolled copy of description_errors(). + # A description that passes that check can never contain a newline, so command substitution has nothing to strip. + if ! description="$(python3 "$script_dir/../spec/resolve_description.py" "$registry" "$name")"; then exit 1 fi - if [ "$match_count" -gt 1 ]; then - echo "$match_count registry entries named $name in $registry. Resolve the duplicate before its description can be read (spec/validate.py rejects this once run)." >&2 - exit 1 - fi - if ! declared="$(jq -r --arg n "$name" '.repos[] | select(.name==$n) | has("description")' "$registry")"; then - echo "Failed to read $registry (invalid JSON?)." >&2 - exit 1 - fi - if [ "$declared" = "true" ]; then - # Exactly one match is already established above, so select() itself yields exactly one value here. - # No trim: this only ever validates the value against spec/validate.py's contract, never normalizes it. - # A non-string value (including an explicit null) resolves to empty here, caught by the same guard. - # -j plus the trailing sentinel keeps command substitution from stripping a genuine trailing newline. - if ! description="$(jq -j --arg n "$name" \ - '(.repos[] | select(.name==$n) | .description) | if type == "string" then . else empty end' \ - "$registry" && printf x)"; then - echo "Failed to read description from $registry (invalid JSON?)." >&2 - exit 1 - fi - description="${description%x}" - case "$description" in - "" | [[:space:]]* | *[[:space:]]) - echo "The declared description for $name in $registry is not a non-empty string with no leading or trailing whitespace. Fix it there (spec/validate.py rejects this once run)." >&2 - exit 1 - ;; - esac - case "$description" in - *$'\n'* | *$'\r'*) - echo "The declared description for $name in $registry carries an embedded newline. Fix it there (spec/validate.py rejects this once run)." >&2 - exit 1 - ;; - esac - fi fi # ----- Ruleset id lookup (shared by apply and check) ----- diff --git a/spec/audit.py b/spec/audit.py index ee0b8650..7f9dbe25 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -1309,7 +1309,7 @@ def description_findings(doc_texts, entry, live, slug): if "description" in entry: # Delegates to validate.py's own contract instead of re-checking a second, easily-incomplete copy of it # (an earlier version here missed the link and length rules, accepting either as canonical). - shape_errors = validate.description_errors("registry", entry["description"]) + shape_errors = validate.description_errors(slug, entry["description"]) if shape_errors: findings += [ ( @@ -4301,6 +4301,12 @@ def _selftest(): print(f" FAIL description: null-declared-field DEFECT contract -> {null_declared}") else: print(" ok description: a null declared field is a DEFECT via validate.py's contract") + # The DEFECT names the actual repo, not a generic "registry" label - actionable across a fleet-wide run. + if not any(k == "DEFECT" and t.startswith("owner/Fixture:") for k, t in null_declared): + ok = False + print(f" FAIL description: DEFECT does not name the repo -> {null_declared}") + else: + print(" ok description: a declared-field DEFECT names the repo, not a generic label") # The declared field, once present, is what the wording names as the source - not "the README". declared_mismatch = description_findings( desc_readme, diff --git a/spec/resolve_description.py b/spec/resolve_description.py new file mode 100755 index 00000000..5d1b68a1 --- /dev/null +++ b/spec/resolve_description.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +"""Resolve one repo's declared registry/repos.json description, for repo-config/configure.sh. + +Delegates to description_errors() (validate.py) so configure.sh validates a declared description +against the exact same contract spec/audit.py's description_findings() does, rather than a third +hand-rolled copy of the same rules. + +Prints the declared description to stdout and exits 0 when the repo has no declared description +(nothing printed) or exactly one valid one. Exits 1 with a message on stderr for anything +configure.sh should fail loud on: a malformed registry, more than one entry named NAME, or a +declared description description_errors() rejects. + +Usage: resolve_description.py REGISTRY_PATH NAME +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import validate # sibling, import-safe (its main is guarded) + + +def main() -> int: + if len(sys.argv) != 3: + print("usage: resolve_description.py REGISTRY_PATH NAME", file=sys.stderr) + return 1 + registry_path, name = sys.argv[1], sys.argv[2] + try: + registry = json.loads(Path(registry_path).read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as e: + print(f"Failed to read {registry_path}: {e}", file=sys.stderr) + return 1 + matches = [ + r for r in registry.get("repos", []) if isinstance(r, dict) and r.get("name") == name + ] + if len(matches) > 1: + print( + f"{len(matches)} registry entries named {name} in {registry_path}. Resolve the " + "duplicate before its description can be read (spec/validate.py rejects this once run).", + file=sys.stderr, + ) + return 1 + if not matches or "description" not in matches[0]: + return 0 + desc = matches[0]["description"] + errors = validate.description_errors(name, desc) + if errors: + for msg in errors: + print(f"{msg} (spec/validate.py rejects this once run).", file=sys.stderr) + return 1 + sys.stdout.write(desc) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/spec/validate.py b/spec/validate.py index d686dd5b..864bbd53 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -484,6 +484,10 @@ def check_secret_set(label, entry, need_kind): if not isinstance(repo.get("name"), str) or not repo["name"].strip(): errors.append(f"repo #{i}: missing or empty 'name'") continue + if name != name.strip(): + # Both configure.sh and audit.py key their per-repo lookup off an exact match on name, so a padded value would make the entry unresolvable there rather than merely cosmetic here. + errors.append(f"repo #{i}: name '{name}' carries leading/trailing whitespace") + continue if name in seen_names: errors.append(f"{name}: duplicate registry entry for name '{name}'") seen_names.add(name) From ce82780acfec75a9cf41af9933b655aef87a31c3 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 13:14:34 -0700 Subject: [PATCH 2/4] Fail loud on a malformed registry, resolve python3 across platforms (PR #918 round 1) Real fixes: - spec/resolve_description.py: registry.get("repos", []) silently read a registry missing its 'repos' array (or carrying a non-list one) as "no match", so configure.sh would skip an intended declared description instead of failing loud as the script's own contract describes. Extracted a pure resolve_description(registry, name) (raising ResolveError for anything fail-loud-worthy) out of main(), now directly unit tested (scripts/tests/test_resolve_description.py, 7 cases) rather than only reachable through a subprocess. - repo-config/configure.sh: hardcoded python3, which docs/host-setup.md documents at length is not universal - native Windows commonly registers a Microsoft Store stub under that name that resolves on PATH but fails when actually run, and Git Bash inherits the same broken PATH entry. Resolves an interpreter by actually running it (python3 -c "", falling back to py -3), the only way to tell the stub apart from a working interpreter, matching the exact failure mode this repo's own docs name. - spec/audit.py: fixed a spaced-hyphen slip in a new comment. - spec/validate.py: applied CodeRabbit's suggested split of the new padded-name comment into two shorter sentences. Declined (reasoning posted to PR #918): - qodo: PR title's "configure.sh's" flagged as not Title Case - a code identifier kept in its natural casing, the same shape this repo's own PR-title example already uses ("Pin softprops/action-gh-release to commit SHA"). - qodo: docstring wraps across multiple lines - matches the established wrapped-paragraph docstring convention already used throughout spec/audit.py and spec/validate.py. - qodo: 'from __future__ import annotations' called unnecessary boilerplate - it's the standing convention in every scripts/*.py and scripts/tests/*.py file in this repo, used defensively rather than only when strictly required. --- repo-config/configure.sh | 13 +++++- scripts/tests/test_resolve_description.py | 56 +++++++++++++++++++++++ spec/audit.py | 2 +- spec/resolve_description.py | 54 ++++++++++++++-------- spec/validate.py | 3 +- 5 files changed, 106 insertions(+), 22 deletions(-) create mode 100755 scripts/tests/test_resolve_description.py diff --git a/repo-config/configure.sh b/repo-config/configure.sh index 5193f915..cea86221 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -38,6 +38,17 @@ case "$repo_arg" in release|operational) model="$repo_arg"; repo_arg="" ;; esac repo="${repo_arg:-$(gh repo view --json nameWithOwner --jq '.nameWithOwner')}" script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# ----- Resolve a working Python 3 interpreter ----- +# The name python3 is not universal, and on native Windows it can resolve to a Microsoft Store stub that exists on PATH but fails when actually run, so this runs it rather than just checking PATH (docs/host-setup.md). +if python3 -c "" >/dev/null 2>&1; then + py_cmd=(python3) +elif py -3 -c "" >/dev/null 2>&1; then + py_cmd=(py -3) +else + echo "No working Python 3 interpreter found (python3 or py -3). See docs/host-setup.md." >&2 + exit 1 +fi + # ----- Resolve the workflow model (selects the develop ruleset), shared by apply and check ----- registry="$script_dir/../registry/repos.json" name="${repo##*/}" @@ -70,7 +81,7 @@ description="" if [ -f "$registry" ]; then # Delegates to spec/resolve_description.py rather than a third hand-rolled copy of description_errors(). # A description that passes that check can never contain a newline, so command substitution has nothing to strip. - if ! description="$(python3 "$script_dir/../spec/resolve_description.py" "$registry" "$name")"; then + if ! description="$("${py_cmd[@]}" "$script_dir/../spec/resolve_description.py" "$registry" "$name")"; then exit 1 fi fi diff --git a/scripts/tests/test_resolve_description.py b/scripts/tests/test_resolve_description.py new file mode 100755 index 00000000..a544aef7 --- /dev/null +++ b/scripts/tests/test_resolve_description.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +"""Exercise resolve_description()'s registry-shape and fail-loud guards directly.""" + +from __future__ import annotations + +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "spec")) +import resolve_description + + +class ResolveDescriptionCase(unittest.TestCase): + """repo-config/configure.sh's declared-description resolution (spec/resolve_description.py).""" + + def test_a_repo_with_no_declared_description_resolves_to_none(self) -> None: + registry = {"repos": [{"name": "Fixture"}]} + self.assertIsNone(resolve_description.resolve_description(registry, "Fixture")) + + def test_a_repo_absent_from_the_registry_resolves_to_none(self) -> None: + registry = {"repos": [{"name": "Other"}]} + self.assertIsNone(resolve_description.resolve_description(registry, "Fixture")) + + def test_a_valid_declared_description_is_returned(self) -> None: + registry = {"repos": [{"name": "Fixture", "description": "A short tagline."}]} + self.assertEqual( + resolve_description.resolve_description(registry, "Fixture"), "A short tagline." + ) + + def test_a_duplicate_name_raises_rather_than_picking_one(self) -> None: + registry = { + "repos": [ + {"name": "Fixture", "description": "First."}, + {"name": "Fixture", "description": "Second."}, + ] + } + with self.assertRaises(resolve_description.ResolveError): + resolve_description.resolve_description(registry, "Fixture") + + def test_an_invalid_declared_description_raises(self) -> None: + registry = {"repos": [{"name": "Fixture", "description": None}]} + with self.assertRaises(resolve_description.ResolveError): + resolve_description.resolve_description(registry, "Fixture") + + def test_a_registry_with_no_repos_array_raises_rather_than_reading_as_no_match(self) -> None: + with self.assertRaises(resolve_description.ResolveError): + resolve_description.resolve_description({}, "Fixture") + + def test_a_repos_value_that_is_not_a_list_raises(self) -> None: + with self.assertRaises(resolve_description.ResolveError): + resolve_description.resolve_description({"repos": "not-a-list"}, "Fixture") + + +if __name__ == "__main__": + unittest.main() diff --git a/spec/audit.py b/spec/audit.py index 7f9dbe25..b7831303 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -4301,7 +4301,7 @@ def _selftest(): print(f" FAIL description: null-declared-field DEFECT contract -> {null_declared}") else: print(" ok description: a null declared field is a DEFECT via validate.py's contract") - # The DEFECT names the actual repo, not a generic "registry" label - actionable across a fleet-wide run. + # The DEFECT names the actual repo, not a generic "registry" label, so it stays actionable in a fleet-wide run. if not any(k == "DEFECT" and t.startswith("owner/Fixture:") for k, t in null_declared): ok = False print(f" FAIL description: DEFECT does not name the repo -> {null_declared}") diff --git a/spec/resolve_description.py b/spec/resolve_description.py index 5d1b68a1..701d30c7 100755 --- a/spec/resolve_description.py +++ b/spec/resolve_description.py @@ -8,7 +8,7 @@ Prints the declared description to stdout and exits 0 when the repo has no declared description (nothing printed) or exactly one valid one. Exits 1 with a message on stderr for anything configure.sh should fail loud on: a malformed registry, more than one entry named NAME, or a -declared description description_errors() rejects. +declared description that description_errors() rejects. Usage: resolve_description.py REGISTRY_PATH NAME """ @@ -22,6 +22,34 @@ import validate # sibling, import-safe (its main is guarded) +class ResolveError(Exception): + """A condition resolve_description() must fail loud on.""" + + +def resolve_description(registry, name): + """The declared description for NAME in REGISTRY, or None if the repo has none declared. + + Raises ResolveError for anything the caller should fail loud on rather than silently read as + absent: a registry that is not an object carrying a `repos` array, more than one entry named + NAME, or a declared description description_errors() rejects. + """ + if not isinstance(registry, dict) or not isinstance(registry.get("repos"), list): + raise ResolveError("registry is not an object with a 'repos' array") + matches = [r for r in registry["repos"] if isinstance(r, dict) and r.get("name") == name] + if len(matches) > 1: + raise ResolveError( + f"{len(matches)} registry entries named {name}. " + "Resolve the duplicate before its description can be read" + ) + if not matches or "description" not in matches[0]: + return None + desc = matches[0]["description"] + errors = validate.description_errors(name, desc) + if errors: + raise ResolveError("; ".join(errors)) + return desc + + def main() -> int: if len(sys.argv) != 3: print("usage: resolve_description.py REGISTRY_PATH NAME", file=sys.stderr) @@ -32,25 +60,13 @@ def main() -> int: except (OSError, json.JSONDecodeError) as e: print(f"Failed to read {registry_path}: {e}", file=sys.stderr) return 1 - matches = [ - r for r in registry.get("repos", []) if isinstance(r, dict) and r.get("name") == name - ] - if len(matches) > 1: - print( - f"{len(matches)} registry entries named {name} in {registry_path}. Resolve the " - "duplicate before its description can be read (spec/validate.py rejects this once run).", - file=sys.stderr, - ) - return 1 - if not matches or "description" not in matches[0]: - return 0 - desc = matches[0]["description"] - errors = validate.description_errors(name, desc) - if errors: - for msg in errors: - print(f"{msg} (spec/validate.py rejects this once run).", file=sys.stderr) + try: + desc = resolve_description(registry, name) + except ResolveError as e: + print(f"{e} (spec/validate.py rejects this once run).", file=sys.stderr) return 1 - sys.stdout.write(desc) + if desc is not None: + sys.stdout.write(desc) return 0 diff --git a/spec/validate.py b/spec/validate.py index 864bbd53..83185e7b 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -485,7 +485,8 @@ def check_secret_set(label, entry, need_kind): errors.append(f"repo #{i}: missing or empty 'name'") continue if name != name.strip(): - # Both configure.sh and audit.py key their per-repo lookup off an exact match on name, so a padded value would make the entry unresolvable there rather than merely cosmetic here. + # Both configure.sh and audit.py key their per-repo lookup off an exact match on name. + # A padded value would therefore make the entry unresolvable there, not merely cosmetic here. errors.append(f"repo #{i}: name '{name}' carries leading/trailing whitespace") continue if name in seen_names: From cc7bd944c6a337f329901317b91a3e8d3ed9e3c8 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 13:21:17 -0700 Subject: [PATCH 3/4] Make the Python interpreter resolution lazy (PR #918 round 2) Real regression: the python3/py -3 probe ran unconditionally near the top of the script, so 'configure.sh check operational' (an explicit model, no registry needed) would now fail on a host with no working Python even when registry/repos.json is absent and nothing in that run actually needs it - contradicting the script's own documented contract that an explicit model avoids needing the registry at all. Moved the probe inside the same 'if [ -f "$registry" ]' block as its only caller, so it runs exactly when a description might need resolving and never otherwise. Manually verified: with the registry absent and both python3 and py stubbed to fail, the script no longer invokes either at all. --- repo-config/configure.sh | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/repo-config/configure.sh b/repo-config/configure.sh index cea86221..fed36793 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -38,17 +38,6 @@ case "$repo_arg" in release|operational) model="$repo_arg"; repo_arg="" ;; esac repo="${repo_arg:-$(gh repo view --json nameWithOwner --jq '.nameWithOwner')}" script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# ----- Resolve a working Python 3 interpreter ----- -# The name python3 is not universal, and on native Windows it can resolve to a Microsoft Store stub that exists on PATH but fails when actually run, so this runs it rather than just checking PATH (docs/host-setup.md). -if python3 -c "" >/dev/null 2>&1; then - py_cmd=(python3) -elif py -3 -c "" >/dev/null 2>&1; then - py_cmd=(py -3) -else - echo "No working Python 3 interpreter found (python3 or py -3). See docs/host-setup.md." >&2 - exit 1 -fi - # ----- Resolve the workflow model (selects the develop ruleset), shared by apply and check ----- registry="$script_dir/../registry/repos.json" name="${repo##*/}" @@ -79,6 +68,16 @@ settings_file="$script_dir/settings.json" # Absence keeps the About panel following the README. description="" if [ -f "$registry" ]; then + # Resolved here, not near the top, so a run with no registry (an explicit model, no hub checkout) never needs Python at all. + # The name python3 is not universal: native Windows can register a Microsoft Store stub under that name that resolves on PATH but fails when actually run, so this runs it rather than just checking PATH (docs/host-setup.md). + if python3 -c "" >/dev/null 2>&1; then + py_cmd=(python3) + elif py -3 -c "" >/dev/null 2>&1; then + py_cmd=(py -3) + else + echo "No working Python 3 interpreter found (python3 or py -3). See docs/host-setup.md." >&2 + exit 1 + fi # Delegates to spec/resolve_description.py rather than a third hand-rolled copy of description_errors(). # A description that passes that check can never contain a newline, so command substitution has nothing to strip. if ! description="$("${py_cmd[@]}" "$script_dir/../spec/resolve_description.py" "$registry" "$name")"; then From 8de62f9983ead9d7dbedac14f0919aaf87b2132e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sat, 22 Aug 2026 13:30:44 -0700 Subject: [PATCH 4/4] Fix a real syntax-error gap, close a padded-name near-miss, type resolve_description() (PR #918 round 3) Real fixes: - configure.sh: the interpreter probe only checked that python3/py -3 could start at all (python3 -c ""), not that it could parse spec/resolve_description.py's own syntax. from __future__ import annotations (PEP 563) genuinely requires Python 3.7+, so a pre-3.7 python3 would pass the probe and then hit a bare SyntaxError from the script itself with a confusing message. The probe now runs that exact import instead of an empty program, so an interpreter too old for the script fails at the probe with a clear message naming the actual floor, rather than surfacing as a script crash. - spec/resolve_description.py: a registry entry like {"name": " Fixture ", ...} would silently resolve as "Fixture has no declared description" instead of being flagged - the exact-match lookup never saw it as related to the query at all. Added a targeted check: an entry whose name matches only after stripping is a near-miss worth failing loud on, not a full re-validation of every entry in the registry (that's spec/validate.py's job, and raising for an unrelated entry's malformed name would be a needlessly wide blast radius for a single repo's lookup). New test case. - spec/resolve_description(): added type hints (dict, str, str | None) for consistency with main() -> int already in the same file - this is a new function in a new file, not an addition to a pre-existing untyped convention like the earlier declined validate.py case. Declined (reasoning posted to PR #918): CodeRabbit's ask to enforce the full Python 3.13 floor in the probe. spec/resolve_description.py carries no 3.13-specific syntax, and docs/host-setup.md itself describes that floor as a 'target' (what the toolchain is configured for) rather than a 'measured' one (known to break something) - scripts/host_gate.py is the dedicated tool for that check. The narrower from __future__ import annotations probe above is the floor this specific script actually needs, addressing the same suppressed Copilot finding (an old interpreter passing the probe then SyntaxError-ing) without the disconnect of an arbitrary version number. --- repo-config/configure.sh | 7 +++--- scripts/tests/test_resolve_description.py | 7 ++++++ spec/resolve_description.py | 26 +++++++++++++++++++---- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/repo-config/configure.sh b/repo-config/configure.sh index fed36793..5016f373 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -70,12 +70,13 @@ description="" if [ -f "$registry" ]; then # Resolved here, not near the top, so a run with no registry (an explicit model, no hub checkout) never needs Python at all. # The name python3 is not universal: native Windows can register a Microsoft Store stub under that name that resolves on PATH but fails when actually run, so this runs it rather than just checking PATH (docs/host-setup.md). - if python3 -c "" >/dev/null 2>&1; then + # The probe itself is spec/resolve_description.py's actual floor (PEP 563, Python 3.7+) rather than an arbitrary version number, so a too-old interpreter fails here with a clear message instead of a bare SyntaxError from the script. + if python3 -c "from __future__ import annotations" >/dev/null 2>&1; then py_cmd=(python3) - elif py -3 -c "" >/dev/null 2>&1; then + elif py -3 -c "from __future__ import annotations" >/dev/null 2>&1; then py_cmd=(py -3) else - echo "No working Python 3 interpreter found (python3 or py -3). See docs/host-setup.md." >&2 + echo "No Python 3.7+ interpreter found (python3 or py -3). See docs/host-setup.md." >&2 exit 1 fi # Delegates to spec/resolve_description.py rather than a third hand-rolled copy of description_errors(). diff --git a/scripts/tests/test_resolve_description.py b/scripts/tests/test_resolve_description.py index a544aef7..4be26443 100755 --- a/scripts/tests/test_resolve_description.py +++ b/scripts/tests/test_resolve_description.py @@ -51,6 +51,13 @@ def test_a_repos_value_that_is_not_a_list_raises(self) -> None: with self.assertRaises(resolve_description.ResolveError): resolve_description.resolve_description({"repos": "not-a-list"}, "Fixture") + def test_a_padded_name_that_would_otherwise_match_raises_rather_than_reading_as_absent( + self, + ) -> None: + registry = {"repos": [{"name": " Fixture ", "description": "A short tagline."}]} + with self.assertRaises(resolve_description.ResolveError): + resolve_description.resolve_description(registry, "Fixture") + if __name__ == "__main__": unittest.main() diff --git a/spec/resolve_description.py b/spec/resolve_description.py index 701d30c7..aaefd38c 100755 --- a/spec/resolve_description.py +++ b/spec/resolve_description.py @@ -26,16 +26,34 @@ class ResolveError(Exception): """A condition resolve_description() must fail loud on.""" -def resolve_description(registry, name): +def resolve_description(registry: dict, name: str) -> str | None: """The declared description for NAME in REGISTRY, or None if the repo has none declared. Raises ResolveError for anything the caller should fail loud on rather than silently read as - absent: a registry that is not an object carrying a `repos` array, more than one entry named - NAME, or a declared description description_errors() rejects. + absent: a registry that is not an object carrying a `repos` array, an entry whose own name + would match NAME but for leading/trailing whitespace (spec/validate.py rejects that shape too, + so it is never the intended way to spell a mismatch), more than one entry named NAME, or a + declared description description_errors() rejects. """ if not isinstance(registry, dict) or not isinstance(registry.get("repos"), list): raise ResolveError("registry is not an object with a 'repos' array") - matches = [r for r in registry["repos"] if isinstance(r, dict) and r.get("name") == name] + repos = registry["repos"] + near_miss = next( + ( + r["name"] + for r in repos + if isinstance(r, dict) + and isinstance(r.get("name"), str) + and r["name"] != name + and r["name"].strip() == name + ), + None, + ) + if near_miss is not None: + raise ResolveError( + f"a registry entry's name {near_miss!r} carries leading/trailing whitespace" + ) + matches = [r for r in repos if isinstance(r, dict) and r.get("name") == name] if len(matches) > 1: raise ResolveError( f"{len(matches)} registry entries named {name}. "