From b6d0b31cb174f61c25e66e76ca28dbc3832182e6 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 16:43:18 -0700 Subject: [PATCH 1/2] Make intent_canonical_rel() Crash-Safe, Fix Two Missed Docstring Wraps qodo findings on PR #978, the develop -> main promotion PR: - intent_canonical_rel() called .split() on intentRef through a bare truthiness check, so a non-string value (a malformed files.json entry) still crashed the whole audit run. spec/validate.py's own type check (added on PR #977) only helps a caller that runs it first, and spec/audit.py does not: it loads files.json directly. reference's parallel or-based use elsewhere never method-calls the value, so it carried no matching risk, isinstance guards on this function specifically close the gap. Verified by hand: calling the function with a non-string intentRef used to raise AttributeError, now returns path. New _selftest() case covers it. - escapes_repo_root()'s docstring, and one line of intent_canonical_rel()'s, still wrapped a single sentence across physical lines. prose_lint.py's comment-wrap check reads `#` comments, not `"""` docstrings, so neither round that touched these functions caught it, the fleet's own known gap. Reformatted both to one sentence per line, and did the same for a leftover _selftest() comment that had the identical wrap. --- spec/audit.py | 17 +++++++++++------ spec/validate.py | 7 +++---- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index 63d64648..85d26b94 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -1682,14 +1682,14 @@ def intent_canonical_rel(item, path): 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. + 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 ref: + if isinstance(ref, str) and ref: return ref intent = item.get("intentRef") - if intent: + if isinstance(intent, str) and intent: return intent.split("#", 1)[0] return path @@ -4686,8 +4686,7 @@ 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. + # 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", @@ -4725,6 +4724,12 @@ def _selftest(): "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", + ), ] for label, item, path, want in canonical_cases: got = intent_canonical_rel(item, path) diff --git a/spec/validate.py b/spec/validate.py index f2580f0d..112ef7c8 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -57,10 +57,9 @@ def is_str_list(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:`. + """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 From f691758fdd714bb7605d940c1f1531e565c6840f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 16:53:41 -0700 Subject: [PATCH 2/2] Fall Back to path on an Anchor-Only intentRef CodeRabbit finding on PR #980: an intentRef of "#section" (anchor, no file component) survives the isinstance/truthiness guard just added, and intent_canonical_rel() returns the split's empty string as the canonical. check_intent_staleness() then calls hub_last_change on that empty string, `git log -- ""` errors (empty pathspec), and the function returns no finding, silently. validate.py already rejects this shape via escapes_repo_root()'s not-value check, but audit.py runs standalone and does not invoke it first, same as the non-string case fixed a commit ago. Fall back to path when the fragment-stripped intentRef is empty. New _selftest() case covers it. --- spec/audit.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spec/audit.py b/spec/audit.py index 85d26b94..6580993a 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -1690,7 +1690,9 @@ def intent_canonical_rel(item, path): return ref intent = item.get("intentRef") if isinstance(intent, str) and intent: - return intent.split("#", 1)[0] + canonical = intent.split("#", 1)[0] + if canonical: + return canonical return path @@ -4730,6 +4732,12 @@ def _selftest(): "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)