Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 12 additions & 34 deletions repo-config/configure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
ptr727 marked this conversation as resolved.
else
echo "No Python 3.7+ interpreter found (python3 or py -3). See docs/host-setup.md." >&2
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
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) -----
Expand Down
63 changes: 63 additions & 0 deletions scripts/tests/test_resolve_description.py
Original file line number Diff line number Diff line change
@@ -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()
8 changes: 7 additions & 1 deletion spec/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 += [
(
Expand Down Expand Up @@ -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,
Expand Down
92 changes: 92 additions & 0 deletions spec/resolve_description.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
#!/usr/bin/env python3
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
"""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.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

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
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

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:
Comment thread
ptr727 marked this conversation as resolved.
"""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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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]
Comment thread
ptr727 marked this conversation as resolved.
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())
5 changes: 5 additions & 0 deletions spec/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down