From 0a999951950bcc1abf58a272c28f8fe17158e12b Mon Sep 17 00:00:00 2001 From: Tim Disney Date: Sun, 26 Jul 2026 21:10:41 -0700 Subject: [PATCH] fix ui link to tangle pull request --- .../ui/src/lib/components/UnitDetail.svelte | 23 ++++++- packages/ui/src/lib/requests.test.ts | 26 +++++++- packages/ui/src/lib/requests.ts | 6 +- packages/ui/src/lib/units.ts | 66 +++++++++++++++---- 4 files changed, 103 insertions(+), 18 deletions(-) diff --git a/packages/ui/src/lib/components/UnitDetail.svelte b/packages/ui/src/lib/components/UnitDetail.svelte index d07b6ed..88d8b3b 100644 --- a/packages/ui/src/lib/components/UnitDetail.svelte +++ b/packages/ui/src/lib/components/UnitDetail.svelte @@ -75,7 +75,6 @@ const distilled = $derived(distilledInto(space.index, unit)) const checks = $derived(version ? checkResults(version.checkruns) : []) - const pr = $derived(pullRequest(version)) const links = $derived(version?.artifact.value.links) const findings = $derived( @@ -92,6 +91,9 @@ ) const drift = $derived(project ? driftOf(space.index, project.target.uri, unit) : 0) const durability = $derived(durabilityOf(space.index, unit.type)) + // Needs the project: a tangled pull is a record, and the page it can be reached through is the + // repository's, which only the remote names. + const pr = $derived(pullRequest(version, project?.gitUrl)) // ── what can be asked for from here ─────────────────────────────────────────────────────────── // Every button below is generated from the registry: the successor of this unit's own type, and @@ -320,8 +322,23 @@
{#if links?.branch}branch {links.branch}{/if} {#if links?.commit}commit {shortCommit(links.commit)}{/if} - {#if pr} - pull request{pr.number ? ` #${pr.number}` : ''} ↗ + {#if pr?.href} + + + {pr.number ? `pull request #${pr.number}` : pr.uri ? 'pull requests' : 'pull request'} ↗ + + {#if pr.uri} + pull + {/if} + {:else if pr?.uri} + pull {/if} {#if version?.mergedAt} merged · {stamp(version.mergedAt)} diff --git a/packages/ui/src/lib/requests.test.ts b/packages/ui/src/lib/requests.test.ts index 1e65a7c..7be7e56 100644 --- a/packages/ui/src/lib/requests.test.ts +++ b/packages/ui/src/lib/requests.test.ts @@ -710,9 +710,33 @@ describe('a pull request that is a record rather than a page', () => { const version = disclosure().current if (!version) throw new Error('fixture version missing') expect(pullRequest(asRecord(version))).toEqual({ - url: 'at://did:plc:agent/sh.tangled.repo.pull/3ktid1', + uri: 'at://did:plc:agent/sh.tangled.repo.pull/3ktid1', number: '', }) expect(pullRequest(version)?.number).toBe('263') }) + + // An at-uri in an href is a dead link. The pull's own page cannot be built either — tangled numbers + // a pull while ingesting it, and that number is in no record — so the repository's pull list is + // where the link goes, and the record travels beside it instead of inside it. + it('links a record pull through its repository, never through the at-uri', () => { + const version = disclosure().current + if (!version) throw new Error('fixture version missing') + const link = pullRequest(asRecord(version), 'https://tangled.org/@alice/widget') + expect(link).toEqual({ + href: 'https://tangled.org/@alice/widget/pulls', + uri: 'at://did:plc:agent/sh.tangled.repo.pull/3ktid1', + number: '', + }) + // A self-hosted knot links to itself, and a `.git` suffix is not part of the page's path. + expect(pullRequest(asRecord(version), 'https://knot.example/alice/widget.git')?.href).toBe( + 'https://knot.example/alice/widget/pulls', + ) + // Nothing to build it from is a chip with no link, which is honest. A GitHub pull is untouched. + expect(pullRequest(asRecord(version), 'not a url')?.href).toBeUndefined() + expect(pullRequest(asRecord(version))?.href).toBeUndefined() + expect(pullRequest(version, 'https://github.com/acme/widget.git')?.href).toBe( + version.artifact.value.links?.pr, + ) + }) }) diff --git a/packages/ui/src/lib/requests.ts b/packages/ui/src/lib/requests.ts index 92c2a68..333b563 100644 --- a/packages/ui/src/lib/requests.ts +++ b/packages/ui/src/lib/requests.ts @@ -186,9 +186,11 @@ export function prContinuity( // the daemon does too (its adapter answers for one forge, and a URL no adapter parses falls // through to a fresh branch — but a browser that cannot parse a URL has not learned that the PR // is elsewhere). A tangled pull recorded as an `at://` record URI names no repository at all, so - // it lands here as "no opinion" rather than as a mismatch. + // it lands here as "no opinion" rather than as a mismatch — and the page `pullRequest` can offer + // for one is derived from THIS project's remote, so reading a repository out of it would only be + // comparing the project with itself. const project = gitUrl ? repoIdentity(gitUrl) : undefined - const host = repoIdentity(pr.url) + const host = pr.uri || !pr.href ? undefined : repoIdentity(pr.href) if (project && host && project.host === host.host && project.slug !== host.slug) { return fresh(`is not in ${project.slug}`) } diff --git a/packages/ui/src/lib/units.ts b/packages/ui/src/lib/units.ts index 0d7b636..293b8ad 100644 --- a/packages/ui/src/lib/units.ts +++ b/packages/ui/src/lib/units.ts @@ -170,21 +170,63 @@ export function checkTally(version: UnitVersion | undefined): { pass: number; to export const checkResults = (checkruns: Array>) => checkruns.flatMap((run) => run.value.results) +/** An `at:///sh.tangled.repo.pull/` — a pull request that IS a record. */ +const RECORD_PULL = /^at:\/\/did:[^/]+\/sh\.tangled\.repo\.pull\/[^/#?]+$/ + +export interface PullLink { + /** Somewhere a browser can actually go, when the forge has such a page. */ + href?: string + /** The pull record's at-uri, when the forge addresses pulls as records rather than as pages. */ + uri?: string + /** The number a human calls it by, when the forge issues one. */ + number: string +} + /** - * The PR a version links, and the number a human calls it by. + * The pull request a version links: where a browser can open it, what to call it, and — where those + * are not the same thing — the record it really is. * - * `links.pr` is a plain URI and not every forge puts a number in it: a GitHub pull ends in - * `…/pull/201`, a tangled one is the pull RECORD's `at://…/sh.tangled.repo.pull/`, whose last - * segment is a timestamp-ish key that would read as a pull number if it happened to be digits. - * So the number is only taken from a path that actually ends in one, and an `at://` value never - * offers one — it renders as a plain chip instead. + * `links.pr` is not uniformly a URL. A GitHub pull is a page ending in `…/pull/201`. A tangled pull + * is the `sh.tangled.repo.pull` RECORD's at-uri, which no browser can follow, and whose last segment + * is a tid that would read as a pull number if it happened to be digits — so a number is only taken + * from a path that actually ends in one. */ -export function pullRequest(version: UnitVersion | undefined): { url: string; number: string } | undefined { - const url = version?.artifact.value.links?.pr ?? version?.merges[0]?.value.pr - if (!url) return undefined - if (url.startsWith('at://')) return { url, number: '' } - const number = url.slice(url.lastIndexOf('/') + 1) - return { url, number: /^\d+$/.test(number) ? number : '' } +export function pullRequest( + version: UnitVersion | undefined, + /** The project's remote, when the caller has it: what a record-addressed pull's repository page + * is built from, since the record itself names no host. */ + gitUrl?: string, +): PullLink | undefined { + const value = version?.artifact.value.links?.pr ?? version?.merges[0]?.value.pr + if (!value) return undefined + if (value.startsWith('at://')) { + const list = RECORD_PULL.test(value) ? pullListPage(gitUrl) : undefined + return { ...(list ? { href: list } : {}), uri: value, number: '' } + } + const number = value.slice(value.lastIndexOf('/') + 1) + return { href: value, number: /^\d+$/.test(number) ? number : '' } +} + +/** + * The repository's pull LIST, which is as close as a tangled pull can be linked. + * + * Its own page cannot be: the appview addresses a pull by a sequential number it assigns while + * ingesting, its route parses that segment with `strconv.Atoi`, and the number appears in no record + * and in no API Radial reads. Guessing one would land on somebody else's pull. Built from the + * project's remote and its host, so a self-hosted knot links to itself rather than to tangled.org. + */ +function pullListPage(gitUrl: string | undefined): string | undefined { + if (!gitUrl) return undefined + let parsed: URL + try { + parsed = new URL(gitUrl) + } catch { + return undefined + } + if (parsed.protocol !== 'https:') return undefined + const [owner, repo] = parsed.pathname.replace(/^\/+/, '').split('/') + if (!owner || !repo) return undefined + return `https://${parsed.host}/${owner}/${repo.replace(/\.git$/, '')}/pulls` } const verdictBadge = (verdict: 'approve' | 'request_changes'): RowBadge => -- 2.51.2