Skip to content

Follow-up: deferred review findings from PR #505 #526

Description

@philcunliffe

Neutral triage on PR #505 (fix/issue-497, head 756dabc) exhausted the 2-round
review fix-loop with three residual findings, all judged non-blocking
(preference / correctly-deferred design questions, not production defects).
Filed here so they are not lost, per LLP 0017.

Backlink: #505

1. Symlink-inside-target: purge deletes a row reached only via a symlink (documented, not fixed)

src/core/usage-policy/matcher.jsscopeGovernance/scopeGoverns.

Reproduced independently (a third time, after the PR author and two review
rounds) against head 756dabc:

cwd as given                                     = <root>/proj/link/sub
realpath(cwd)                                    = <root>/elsewhere/sub
scopeGovernance(as-given, target, proveAliases)  = governs
scopeGovernance(as-given, target) plain          = governs
scopeGoverns (gate shape)                        = true
scopeGovernance(realCwd, target) plain           = outside

A row recorded under a path that is lexically inside the purge target, but
that a symlink resolves to a directory physically elsewhere on disk, reads as
governs even with the proveAliases opt-in off — i.e. this is the plain
canonicalSpellings/matchDepth match from #482, not #493's fold, and predates
both. hyp purge will delete cache rows for such a spelling.

Why PR #505 documents rather than fixes it: the same plain predicate is what
hyp ignore --check, policy show, and every other verb use to decide what
proj governs. Making purge alone treat the row as out of scope would desync it
from the gate — the gate would keep saying the row is governed while purge
refuses to clear it, leaving a residual the user has no spelling to name to
clear. LLP 0104 §Consequences (llp/0104-hyp-purge.decision.md:180-188)
records this explicitly, including that reversing it is a change to
LLP 0050 §canonicalization's set-of-spellings semantics, not to purge's own
decision.

Issue #497 itself flagged this as "worth a decision, separately."

Follow-up: a maintainer should decide whether canonicalSpellings /
matchDepth should stop following symlinks for containment purposes across all
verbs (a LLP 0050 change), not just purge. Until decided, current behaviour is
unchanged from pre-#493 releases and is internally consistent across every verb
that consults the gate.

2. @ref annotation separated from its construct by a blank line

test/core/purge-command.test.js:442@ref LLP 0104#spellings [tests] is
followed by a blank line (443) before the construct it should attach to
(const NFC at 444), which breaks CLAUDE.md's attachment rule ("a blank line
breaks attachment").

Not a slip specific to this file: a corpus-wide scan (re-run by the round-2
reviewer) found the same shape in 38 places across 34 files, all in test/,
essentially all section-level headers over a block of tests. Closing this one
blank would also make the annotation worse, not better — it would attach a
section-level gloss ("purge deletes an aliased spelling...") to the NFC
constant declaration, which is not what it glosses.

Follow-up: a maintainer should decide whether to (a) normalize the
convention corpus-wide, or (b) amend CLAUDE.md's attachment rule to allow a
section-level @ref to precede a blank line before its block. CLAUDE.md is
currently the only place the rule is written; LLP 0000 does not define
attachment semantics even though .claude/skills/ref-check/SKILL.md cites it as
if it did.

3. Hyphen soft-wrap renders oddly in a new LLP 0104 list item

llp/0104-hyp-purge.decision.md:134 — "case-sensitive (or normalization-\n
sensitive) volume" line-wraps at a hyphen, which most Markdown renderers turn
into "normalization- sensitive" (a stray space after the hyphen) since a single
newline collapses to a space in paragraph text.

Purely cosmetic; zero behavioural effect. The same pattern already exists
untouched elsewhere in the same file (llp/0104-hyp-purge.decision.md:168-169,
"purge-then-\n re-record"), so it is this file's pre-existing convention, not a
new slip introduced by PR #505.

Follow-up: a maintainer may want to reflow both list items to avoid
wrapping mid-word, or adopt a doc-lint rule against it.


None of the three could cause a production defect (wrong purge behaviour, data
loss, a security hole, a crash, or a perf regression) on their own at this head;
each was independently re-verified by execution before being filed here rather
than accepted from the PR body or review comments.

Metadata

Metadata

Assignees

No one assigned

    Labels

    neutral:fixDelegate this issue to neutral for an autonomous fix attempt (reproduce -> fix -> PR)neutral:stuckneutral attempted this but cannot complete it autonomously — needs a human

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions