fix(references): ignore scoped npm package specs in plain relative paths

[rksharma-owg]

Problem

Scoped npm package specifications (such as @xerg/cli@0.34.0, @types/node, @modelcontextprotocol/server-filesystem) in skill documentation or commands are misclassified as unresolved local relative paths. This produces a reference_unresolved ledger exception and marks an otherwise clean scan as partial (analysis_completeness.is_complete = false), which downgrades the risk recommendation from SAFE to CAUTION.

Root Cause

_PLAIN_RELATIVE_PATH in src/skillspector/references.py used lookaround assertions (?<![\w:/.-]) and (?![\w/.-]). Because @ is neither a word character nor part of the character set, the pattern matched the inner path segment of scoped npm packages (for example, xerg/cli from @xerg/cli@0.34.0). _normalize_candidate then treated it as a relative local path, creating an unresolvable reference in the bundle.

Solution

1. Updated _PLAIN_RELATIVE_PATH negative lookbehind and lookahead to include @ ((?<![\w:/.-@]) and (?![\w/.-@])), preventing @-scoped packages and version-tagged tokens from being extracted as plain relative paths.
2. Updated _normalize_candidate to reject candidates starting with @ as unsupported references.
3. Added regression coverage in tests/nodes/test_security_remediation.py testing scoped npm package commands and specs, while confirming legitimate local relative file references continue to resolve correctly.

Verification

• uv run pytest tests/nodes/test_security_remediation.py -k "npm or reference" -v (11 passed)
• uv run pytest tests/nodes/test_security_remediation.py -q (310 passed)
• uv run ruff check src/ tests/ (passed)
• uv run ruff format --check src/ tests/ (passed)
• Deterministic anti-cheat check passed
• DCO sign-off verified (Signed-off-by)

Fixes #528

[rng1995]

[SkillSpector Review]

Reviewed current head e3a0827b75edae76e35600571fac46108afb251f. The scoped-package prose boundary is useful, but the unconditional @ rejection also removes explicit Markdown references to real inventoried local paths. That turns a false-positive fix into a fail-open completeness regression for otherwise out-of-scope artifacts. Preserve the prose exclusion while still resolving explicit, known @ paths and add the regression.

All required checks pass, but this correctness/security issue and mergeStateStatus=BEHIND block merging.

[rng1995]

[P1] Preserve explicit references to real @-prefixed paths

This unconditional return also rejects an explicit Markdown reference such as [payload](@scope/payload.bin) even when that exact path is present in known_paths. Those references previously resolved, and resolution is security-significant because a referenced nested/binary artifact may otherwise fall outside normal analysis scope and completeness accounting. Keep the scoped-npm plain-prose boundary, but allow explicit reference syntax to resolve a real inventoried @ path; add a regression for that case.

[rksharma-owg]

Thanks for the review, @rng1995!

Updated in commit 2b1d60b:

• Rebased onto latest upstream/main to clear the BEHIND merge state.
• Preserved the prose exclusion: _PLAIN_RELATIVE_PATH retains @ in negative lookaround so scoped npm package specs (e.g. @xerg/cli@0.34.0 and @types/node) are not extracted in prose.
• Removed unconditional @ rejection: Removed raw.startswith("@") from _normalize_candidate, ensuring explicit Markdown references to real inventoried local paths (such as [@scope/tool.py](@scope/tool.py)) continue to normalize and resolve to ANALYZED.
• Added regression test: Added test_explicit_known_at_paths_resolve in tests/nodes/test_security_remediation.py to verify explicit known @ paths resolve with disposition == ArtifactDisposition.ANALYZED and status resolved.

All 311 unit tests and formatting/lint checks pass locally.

[MohammedAlkindi]

The intent works, @types/node is no longer picked up, but the character class does more than intended.

In [\w:/.-@] the hyphen sits between . and @, so it reads as a range from 0x2E to 0x40. Compared against the old class it adds ;<=>? as well as @.

Measured against the previous pattern, these stop matching entirely: run scripts/run.sh; then stop, open docs/guide.md?v=2 now, cat docs/guide.md> out.txt, and <a href=docs/guide.md>. So ordinary relative references followed by common punctuation, or sitting in an HTML attribute, become invisible to the resolver.

That is the same false-negative direction this PR is trying to remove, just on a different input. Putting @ last, as [\w:/.@-], or escaping the hyphen keeps the fix without the range.

[rksharma-owg]

Good catch, @MohammedAlkindi — thanks!

Updated in commit 68daad7:

• Fixed the character range: Moved @ after the hyphen in the lookbehind/lookahead character classes ([\w:/.@-] instead of [\w:/.-@]), so the hyphen is at class-end and literal. The previous placement created an unintended range .-@ (0x2E–0x40), silently excluding ; < = > ? and hiding ordinary references followed by those characters.
• Added regression test: test_paths_followed_by_punctuation_still_resolve verifies that scripts/run.sh;, docs/guide.md?, docs/guide.md>, and <a href=docs/guide.md> all resolve correctly.

All 312 tests pass locally.

[rng1995]

[SkillSpector Review]

Re-reviewed current head d3237f6693679a6b53cf355fd1226c12d5d4907d against the prior requested change and subsequent feedback. The original fail-open regression is resolved: scoped npm packages are excluded only at the plain-prose regex boundary, while explicit Markdown references to inventoried @-prefixed paths still reach normalization and resolve as analyzed artifacts. The corrected character class keeps - literal, and the new punctuation regressions protect ordinary references followed by ;, ?, or > and HTML-attribute forms. I found no remaining required code, test, documentation, security, or compatibility changes.

All required checks are green. Merging is currently blocked by conflicts (mergeable=CONFLICTING, mergeStateStatus=DIRTY), and the outdated prior thread remains unresolved in GitHub even though the code concern is addressed. Rebase/conflict resolution will change the reviewed head and require another review.

[rksharma-owg]

Rebased onto latest upstream/main to resolve the merge conflict with PR #451 while preserving the _PLAIN_RELATIVE_PATH lookaround fix. All 348 unit tests pass.

[rng1995]

[SkillSpector Review]

Re-reviewed current head 7919dec63adfa15a46ca6f67757a2c0b9f0dec4b against the prior requested change, the rebased one-commit diff, surrounding reference-resolution behavior, the unresolved outdated thread, and exact-head checks.

The original fail-open regression remains resolved: the scoped-package exclusion is confined to plain-prose matching, while explicit Markdown references to inventoried @-prefixed paths still reach normalization and resolve. The punctuation boundary remains correct, and focused regressions cover both behaviors. I found no remaining required code, test, documentation, security, or compatibility changes.

All six exact-head checks pass. GitHub still reports BLOCKED, and the outdated prior thread remains unresolved even though its code concern is addressed; those are merge-gate concerns rather than code-review blockers.

Priority: P1 — reference completeness affects nested-artifact inspection, but this is a narrow false-positive correction.