diff --git a/repo-config/configure.sh b/repo-config/configure.sh index 4995ad59..5016f373 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -68,44 +68,22 @@ 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 - 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 + # 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). + # 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 "from __future__ import annotations" >/dev/null 2>&1; then + py_cmd=(py -3) + else + echo "No Python 3.7+ interpreter found (python3 or py -3). See docs/host-setup.md." >&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 + # 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 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/scripts/tests/test_resolve_description.py b/scripts/tests/test_resolve_description.py new file mode 100755 index 00000000..4be26443 --- /dev/null +++ b/scripts/tests/test_resolve_description.py @@ -0,0 +1,63 @@ +#!/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") + + 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/audit.py b/spec/audit.py index ee0b8650..b7831303 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, 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}") + 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..aaefd38c --- /dev/null +++ b/spec/resolve_description.py @@ -0,0 +1,92 @@ +#!/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 that 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) + + +class ResolveError(Exception): + """A condition resolve_description() must fail loud on.""" + + +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, 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") + 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}. " + "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) + 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 + 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 + if desc is not None: + sys.stdout.write(desc) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/spec/validate.py b/spec/validate.py index d686dd5b..83185e7b 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -484,6 +484,11 @@ 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. + # 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: errors.append(f"{name}: duplicate registry entry for name '{name}'") seen_names.add(name)