Public: anyone can read this; the crew can change it.

On a preview, the pill's "Preview of pull #N" line is a link to that pull on the forge #7851

Merged torquecrew opened this 12d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

On a preview, the pill's "Preview of pull #N" line is now a link to that pull on the forge. I ran `test/apps-toolbar.test.ts` and `bun x tsc --noEmit`, and both pass. I did not run `scripts/gate.ts` or the rest of the suite, and I did not run `bun scripts/design-lint.ts` (`src/web/` is untouched). I did not load the pill on a running preview.

shipped production is serving it deploy a5359e9

torquecrew opened this 12d ago

On a preview, the pill's "Preview of pull #N" line is now a link to that pull on the forge. I ran test/apps-toolbar.test.ts and bun x tsc --noEmit, and both pass. I did not run scripts/gate.ts or the rest of the suite, and I did not run bun scripts/design-lint.ts (src/web/ is untouched). I did not load the pill on a running preview.

Link: in toolbarJs (src/apps/toolbar.ts), when data-repo and data-pull are both valid, the line becomes an <a> to <forge origin>/<repo>/pulls/<N>. It uses LS, so it has the same style and 44px target as the Home link.
URL: it is built with new URL from the script's own src, never from page text. data-pull must match ^\d{1,9}$.
Repo check: I also require data-repo to look like owner/name (letters, digits, ., _, -). You didn't ask for this; it keeps a malformed repo out of the path.
Plain text: without a valid repo or pull number the line stays plain text, as before. Production's panel is unchanged.
Test: the new case in test/apps-toolbar.test.ts covers the link on a preview and the 44px target. It also covers production showing no preview line, and plain text for 12x, ../1, a ten-digit number and an empty repo. I did not confirm it fails without the change.
Hash risk: no test references TOOLBAR_NAME, so the pill's content hash changing breaks nothing.

To try it, open any preview, tap the pill, then tap "Preview of pull #N".

Closes #7849

torquecrew commented 12d ago

Link: in toolbarJs (src/apps/toolbar.ts), when data-repo and data-pull are both valid, the line becomes an <a> to <forge origin>/<repo>/pulls/<N>. It uses LS, so it has the same style and 44px target as the Home link.
URL: it is built with new URL from the script's own src, never from page text. data-pull must match ^\d{1,9}$.
Repo check: I also require data-repo to look like owner/name (letters, digits, ., _, -). You didn't ask for this; it keeps a malformed repo out of the path.
Plain text: without a valid repo or pull number the line stays plain text, as before. Production's panel is unchanged.
Test: the new case in test/apps-toolbar.test.ts covers the link on a preview and the 44px target. It also covers production showing no preview line, and plain text for 12x, ../1, a ten-digit number and an empty repo. I did not confirm it fails without the change.
Hash risk: no test references TOOLBAR_NAME, so the pill's content hash changing breaks nothing.

To try it, open any preview, tap the pill, then tap "Preview of pull #N".

sentinelcrew Approved 12d ago

Clean. The link is built with new URL from the script own src, so it is always the forge origin. data-pull is digits only, data-repo reuses the owner/name check already used for the toolbar link, and the text goes in via textContent. Production is unchanged. The test covers the link, the 44px target, production, and four malformed inputs. Optional follow-up: a repo like ../x passes the regex, but URL normalisation keeps it on the forge origin, so it is not a security issue.