Conversation
f785d45 to
1f2739f
Compare
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Reviewed head 1f2739f1fbe004fa10a3a8a809609f25473a646c — REQUEST_CHANGES.
src/skillspector/references.py:137: the numeric-suffix heuristic rejects real paths, including paths with an explicit directory signal such asdocs/tool.1. Numeric extensions are valid (manual pages and numbered artifacts are common), and this rejection happens before the resolver can checkknown_paths, so a shipped, explicitly referenced file becomes invisible to reference coverage. Apply the version heuristic only to genuinely ambiguous metadata tokens, or first preserve candidates that resolve to a known bundled path. Add a regression for a knowndocs/tool.1reference.
Required checks pass, but the correctness regression and mergeStateStatus=BEHIND block merging.
metadata.version values like "1.2" matched the quoted/code-span path
pattern (name="1", ext="2"), so a version-only metadata field made an
otherwise clean scan report is_complete=false and raised the
recommendation from SAFE to CAUTION. Reject candidates whose
extension is purely numeric ("1.2", "v1.2", "1.2.0-beta.1"), since
real file extensions are never all digits.
This is the same class of ambiguity as NVIDIA#451 for NVIDIA#450: a bare version
number is indistinguishable from a root-level file name without
another path signal, so this only narrows the existing quoted/code
path pattern rather than trying to fully disambiguate intent.
Fixes NVIDIA#524.
Signed-off-by: goodhee <goodhee.dev@gmail.com>
1f2739f to
4305b84
Compare
A directory signal ("/") already disambiguates a path from a bare
version string, so docs/tool.1 no longer gets rejected before the
known_paths resolver ever sees it.
Addresses review feedback on PR NVIDIA#525.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A directory signal ("/") already disambiguates a path from a bare
version string, so docs/tool.1 no longer gets rejected before the
known_paths resolver ever sees it.
Addresses review feedback on PR NVIDIA#525.
8d59519 to
6274002
Compare
|
Thanks for the catch — fixed.
Added Branch has been rebased onto latest |
A directory signal ("/") already disambiguates a path from a bare
version string, so docs/tool.1 no longer gets rejected before the
known_paths resolver ever sees it.
Addresses review feedback on PR NVIDIA#525.
Signed-off-by: goodhee <goodhee.dev@gmail.com>
6274002 to
6d9b525
Compare
Summary
numeric (e.g. "1.2", "v1.2", "1.2.0-beta.1")
Before this change,
metadata.version: "1.2"in SKILL.md frontmatter matchedthe quoted-path pattern (name="1", ext="2") and was treated as a missing
local file reference, flipping an otherwise clean scan from
SAFE/is_complete: truetoCAUTION/is_complete: false.Fixes #524.
Testing
uv run pytest tests/nodes/test_security_remediation.py -q(310 passed)uv run ruff check src/ tests/uv run ruff format --check src/ tests/