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
81 changes: 80 additions & 1 deletion spec/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -1675,6 +1675,27 @@ def hub_last_change(rel_path):
return date, sha


def intent_canonical_rel(item, path):
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
"""The hub path an intent unit's copy is judged against for staleness.

`reference` wins where the manifest sets one.
Otherwise the intent unit's own canonical, `intentRef`, wins.
Otherwise the unit compares against its own `path`.
Only `intentRef` ever carries a `#anchor`, routing a reader to one section of a larger doc.
The anchor names a place to read, not a narrower file to diff against, so it is stripped there and nowhere else, comparing the whole canonical file instead.
"""
# spec/validate.py shape-checks these fields, but this engine runs standalone and does not invoke it first.
ref = item.get("reference")
if isinstance(ref, str) and ref:
return ref
intent = item.get("intentRef")
if isinstance(intent, str) and intent:
canonical = intent.split("#", 1)[0]
if canonical:
return canonical
return path


def check_intent_staleness(slug, ground, path, canonical_rel, down_text):
"""The intent-staleness advisory: a last-modified comparison, since intent has no content check.

Expand Down Expand Up @@ -2109,7 +2130,7 @@ def audit_repo(entry, spec, branch=None):
# The hub's own copies are the canonicals, so the hub itself has nothing to trail.
elif item is not None and fid == "intent" and entry.get("name") != HUB_NAME:
findings.extend(
check_intent_staleness(slug, ground, path, item.get("reference") or path, text)
check_intent_staleness(slug, ground, path, intent_canonical_rel(item, path), text)
)
# Heading-based presence is only meaningful for Markdown.
# A "section" named on a non-md file, a tasks.json task group being one, is an intent marker judged per AUDIT.md rather than a heading grep.
Expand Down Expand Up @@ -4667,6 +4688,64 @@ def _selftest():
finally:
globals()["owner_repos"] = real_owner_repos

# An intentRef with an anchor resolves to the whole hub file, not the anchor-qualified name git cannot look up, and reference still wins where the manifest sets both.
canonical_cases = [
(
"no reference or intentRef falls back to the file's own path",
{},
"AGENTS.md",
"AGENTS.md",
),
(
"an intentRef equal to the path is itself the canonical",
{"intentRef": "GOVERNANCE.md"},
"GOVERNANCE.md",
"GOVERNANCE.md",
),
(
"an anchored intentRef strips the anchor and keeps the whole file",
{"intentRef": "GOVERNANCE.md#line-endings"},
".editorconfig",
"GOVERNANCE.md",
),
(
"reference wins over intentRef when the manifest sets both",
{"reference": "catalog/snippets/configs/codecov.yml", "intentRef": "WORKFLOW.md"},
"codecov.yml",
"catalog/snippets/configs/codecov.yml",
),
(
"a literal '#' in reference is preserved rather than treated as an anchor",
{"reference": "docs/notes#1.md"},
"notes.md",
"docs/notes#1.md",
),
(
"a literal '#' in path is preserved rather than treated as an anchor",
{},
"docs/notes#1.md",
"docs/notes#1.md",
),
(
"a non-string intentRef falls back to path instead of crashing",
{"intentRef": 123},
"AUDIT.md",
"AUDIT.md",
),
(
"an anchor-only intentRef falls back to path rather than an empty canonical",
{"intentRef": "#line-endings"},
"AUDIT.md",
"AUDIT.md",
),
]
for label, item, path, want in canonical_cases:
got = intent_canonical_rel(item, path)
good = got == want
if not good:
ok = False
print(f" {'ok ' if good else 'FAIL'} intent_canonical_rel: {label} -> {got!r}")

print("SELFTEST PASS" if ok else "SELFTEST FAIL")
return 0 if ok else 1

Expand Down
59 changes: 53 additions & 6 deletions spec/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,32 @@ def is_str_list(v):
return isinstance(v, list) and all(isinstance(x, str) for x in v)


def escapes_repo_root(value):
"""Whether `ROOT / value` could resolve outside ROOT on some host.

`PurePosixPath` alone misses a backslash (Windows treats it as a separator, though POSIX reads it as one filename) and a Windows drive letter such as `C:`.
"""
return (
not value
or value.startswith("/")
or "\\" in value
or re.match(r"^[A-Za-z]:", value) is not None
or ".." in pathlib.PurePosixPath(value).parts
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)


def canonical_file_in_root(rel_path):
"""Whether `ROOT / rel_path` resolves, symlinks followed, to an existing file under ROOT.

A tracked symlink whose target escapes ROOT passes a bare `Path.is_file()` the same way a real file would, since both follow the link.
"""
try:
resolved = (ROOT / rel_path).resolve(strict=True)
except OSError:
return False
return resolved.is_file() and resolved.is_relative_to(ROOT)


def description_errors_for_repo(repo, name):
"""The per-repo optional-field guard: an explicit `"description": null` is declared-but-invalid, not absent.

Expand Down Expand Up @@ -798,19 +824,40 @@ def check_selector(where, applies_to):
if ref is not None and not isinstance(ref, str):
errors.append(f"files.json: {path} reference must be a string")
ref = None
elif isinstance(ref, str) and (
ref.startswith("/") or ".." in pathlib.PurePosixPath(ref).parts
):
errors.append(
f"files.json: {path} reference '{ref}' must be a repo-relative path (no leading / or ..)"
)
elif isinstance(ref, str):
if escapes_repo_root(ref):
errors.append(f"files.json: {path} reference '{ref}' must be a repo-relative path")
elif fid == "intent" and not canonical_file_in_root(ref):
# This field outranks intentRef in the audit engine's canonical resolution.
# An intent unit's reference needs the same existing-file check intentRef gets below.
errors.append(
f"files.json: {path} reference '{ref}' is not a file in this checkout"
)
if fid == "verbatim":
src = ref if isinstance(ref, str) else path
if isinstance(src, str) and not (ROOT / src).exists():
errors.append(
f"files.json: {path} fidelity 'verbatim' but its canonical source {src} is missing"
)

# The audit engine's intent_canonical_rel() trusts this is a string once validated, the same way it trusts reference above.
intent_ref = item.get("intentRef")
if intent_ref is not None and not isinstance(intent_ref, str):
errors.append(f"files.json: {path} intentRef must be a string")
elif isinstance(intent_ref, str):
# The audit engine strips a trailing #anchor before ever joining this with ROOT, so validate the same part it will actually read.
intent_path = intent_ref.split("#", 1)[0]
if escapes_repo_root(intent_path):
errors.append(
f"files.json: {path} intentRef '{intent_ref}' must be a repo-relative path"
)
elif not canonical_file_in_root(intent_path):
# A directory such as "." exists but is not a file.
# The staleness check would then read the whole repo's most recent commit as this one file's, false-flagging every intent unit as stale.
errors.append(
f"files.json: {path} intentRef '{intent_ref}' canonical {intent_path} is not a file in this checkout"
)

sections = item.get("sections", [])
if not isinstance(sections, list):
errors.append(f"files.json: {path} sections must be an array")
Expand Down