fix(serve): external artifact links route to the artifact detail (REQ-108, item 1)#336
Merged
Merged
Conversation
… (REQ-108) User-flagged as a suspected regression in external-artifact browsing. It is NOT a regression — the behaviour has been present since PR #86 (LSP WebView rendering) — but it is a real navigation gap, surfaced now that cross-repo externals (REQ-085) are in active use. An artifact's outgoing link to a cross-repo (prefix:id) target rendered as `href="/externals/<prefix>"` — the external *project-list* page — rather than `/artifacts/<prefix>:<id>`, the external artifact's own detail view. render_artifact_detail already resolves prefix:id against the synced external's store (artifacts.rs:425-433), so the detail view worked; only the link pointed at the wrong place. You could reach an external artifact by typing the URL but never by clicking a link. Fix: point the link at `/artifacts/<full prefix:id target>`, keeping the badge-info prefix chip for visual origin. One behavioural line. Coverage (answering "do we have enough Playwright tests?"): no — the existing externals.spec.ts only covered the /externals project-list page, not artifact-level cross-repo navigation. Adds a Playwright test that walks artifact detail pages and asserts any external reference links to /artifacts/, never /externals/. It is conditional on synced externals being present (skips cleanly otherwise); a deterministic synced-external Playwright fixture is tracked as a follow-up under REQ-108, along with the remaining REQ-108 items (followable recursive prefix:ref links from the external detail, inbound internal links, and shortcut-link contrast on the light background). Refs: REQ-108, REQ-085
📐 Rivet artifact deltaNo artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
You flagged a suspected regression in external-artifact browsing. It's not a regression — the behaviour has existed since PR #86 (LSP WebView rendering) — but it is a real navigation gap, surfaced now that cross-repo externals (REQ-085) are in active use.
An artifact's outgoing link to a cross-repo
prefix:idtarget rendered ashref="/externals/<prefix>"(the external project-list page) instead of/artifacts/<prefix>:<id>(the external artifact's own detail view).render_artifact_detailalready resolvesprefix:idagainst the synced external's store, so the detail view worked — you could reach it by typing the URL, but never by clicking a link.Fix: point the link at
/artifacts/<prefix>:<id>, keeping the badge-info prefix chip. One behavioural line.Playwright coverage (your question: "do we have enough?")
No —
externals.spec.tsonly covered the/externalsproject-list page, not artifact-level cross-repo navigation. Adds a test that walks artifact detail pages and asserts any external reference links to/artifacts/, never/externals/. It's conditional on synced externals being present (skips cleanly otherwise).Tracked follow-ups (REQ-108, not in this PR)
prefix:reflinks from the external detail view.Test plan
cargo build -p rivet-cli+ render::artifacts unit tests passRefs: REQ-108, REQ-085
🤖 Generated with Claude Code