Skip to content

fix(evidence): make evidence.file repo-relative so site deep-links resolve#82

Merged
askalf merged 1 commit into
masterfrom
fix/evidence-deep-link-path
Jul 21, 2026
Merged

fix(evidence): make evidence.file repo-relative so site deep-links resolve#82
askalf merged 1 commit into
masterfrom
fix/evidence-deep-link-path

Conversation

@askalf

@askalf askalf commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Why

#80's phase 4 spec calls for swapping the site's /tree/<sha>/<path> source link for /blob/<sha>/<path>/<file>#L<line> per evidence item — a real deep link to the exact matched line, not just file:line as text.

That's blocked on the data side: evidence.file (from support/evidence.mjs's locate()) is relative to the skill's own directory (what scanPieces uses), e.g. bare SKILL.md. But the site resolves one source link per plugin, anchored at the plugin's repo root — and a skill is normally nested under that as skills/<name>/…. Appending a skill-relative file to the plugin's tree link produces a broken URL.

What changed

support/marketplace-watch.mjs now captures skillPath — the skill's own directory relative to the repo root discoverMarketplaceSkills resolved it under (always skills/<name> in practice, since that's the only shape collectPluginSkills recognizes) — for every skill, in both corpus and legacy scan modes. Each evidence entry's file is joined with skillPath before publishing, so it's now genuinely repo-relative and safe to append directly to the plugin's resolved tree/blob link.

evidence.mjs's locate() is untouched — it still matches against skill-root-relative scanPieces, which is correct; the join happens once, at publish time, in the watch.

Verification

New test: evidence file is repo-relative to the skill's source repo, not just the skill dir — a skill nested at <repo>/skills/sneaky/ now publishes evidence with file: "skills/sneaky/SKILL.md", not bare SKILL.md. Full suite 156/156 (4 environment-skipped, unrelated).

Companion site PR (renders the actual <a href=".../blob/.../file#Lline"> instead of plain text) lands separately in truecopy-site.

Part of #80.

…solve

Phase 2/3 (#81) published evidence.file relative to the skill's own
directory (what scanPieces uses), e.g. bare "SKILL.md" — but the site
resolves a per-plugin source link from the plugin's REPO root, and a
skill is usually nested under it (skills/<name>/…). A bare skill-
relative file can't be appended to that link to build a working
github.com/.../blob/<sha>/<file>#L<line> deep link.

Thread through skillPath (the skill's own dir relative to the repo
root discoverMarketplaceSkills resolved it under) and join it onto
each evidence entry's file before publishing, in both corpus and
legacy scan modes.

Closes #80 (phase 4's actual requirement — the tree/ -> blob/#Lline
swap — needs this on the data side; site-side render in a
truecopy-site PR).

@sprayberry-reviewer sprayberry-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.

Verdict: No blocking issues found — approving.

What I checked

  • Diff: support/marketplace-watch.mjs (+17/-5) and test/marketplace-watch.test.mjs (+19).
  • Traced the join invariant end-to-end: in support/marketplace-fetch.mjs, canon-corpus.json rows store dir as the exact directory the site's tree/blob link resolves to — for kind:'local' it's path.resolve(clone, e.rel), for kind:'external' with a sub path it's path.resolve(r.dir, e.sub) (already sub-adjusted). discoverMarketplaceSkills(row.dir) (via collectPluginSkills, src/claude.mjs:84-92) always nests discovered skills at row.dir/skills/<name> (or one level deeper for plugins//external_plugins/ trees), so path.relative(row.dir, s.dir) (support/marketplace-watch.mjs:98,106) never produces .. and always lands on the correct relative join for the tree link's root.
  • New test test/marketplace-watch.test.mjs:105-121 exercises exactly the case described in the PR body (skill nested at <repo>/skills/sneaky/) and asserts evidence[].file === 'skills/sneaky/SKILL.md', not the bare SKILL.md that evidence.mjs's locate() would otherwise return.
  • CI check-rollup: all 9 checks green (CI on ubuntu/windows/macos × node 20/22, CodeQL, truecopy gate) — gh pr checks 82.

What's good

  • Good in-code comment (support/marketplace-watch.mjs:56-62) explaining why the join is needed, tied back to issue #80's phase-4 spec — makes the non-obvious repo-root vs. skill-root distinction easy to verify later.
  • Keeps evidence.mjs's locate() untouched and does the join once at publish time, which is the right layer — avoids threading a new concept through the confabulation-guard matching logic.
  • Test targets the actual regression shape (nested skill dir), not just a trivial case.

Minor (non-blocking)

  • repoRelative's skillPath !== '.' branch (support/marketplace-watch.mjs:63) looks currently unreachable: every path into discoverMarketplaceSkills/collectPluginSkills nests skill dirs under <pluginRoot>/skills/<name>, so s.dir is never equal to row.dir/root and skillPath is never '.'. Harmless defensive code, but worth a one-line note if it's meant to guard a future discovery shape rather than dead code.

@askalf
askalf merged commit e8b062e into master Jul 21, 2026
9 checks passed
@askalf
askalf deleted the fix/evidence-deep-link-path branch July 21, 2026 16:20
askalf added a commit that referenced this pull request Jul 21, 2026
#83)

#81 and #82 shipped user-facing behavior with no changelog entry:
`scan --json` findings/advisories now carry `evidence[]`, and the watch
publishes `evidenceMismatches` plus repo-relative evidence paths. Both
belong under Unreleased so the next release notes are complete.

Documentation only — no code or behavior change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants