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
68 changes: 67 additions & 1 deletion spec/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -1675,6 +1675,25 @@ 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.
Comment thread
coderabbitai[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.
"""
ref = item.get("reference")
if ref:
return ref
intent = item.get("intentRef")
if intent:
return intent.split("#", 1)[0]
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 +2128,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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
# 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 +4686,53 @@ def _selftest():
finally:
globals()["owner_repos"] = real_owner_repos

# intent_canonical_rel: 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",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
),
]
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
48 changes: 42 additions & 6 deletions spec/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ 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 POSIX `..` segment, a leading `/`, a backslash (Windows treats it as a separator
even though POSIX reads the whole thing as one filename), or 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
)


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 +813,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 (ROOT / ref).is_file():
# 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")
Comment thread
ptr727 marked this conversation as resolved.
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"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
elif not (ROOT / intent_path).is_file():
# 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"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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