From 02c0162205c0312394e2b2138f0438ee6a611a22 Mon Sep 17 00:00:00 2001 From: flazouh Date: Wed, 16 Sep 2026 21:45:28 +0200 Subject: [PATCH 1/6] Address the interface where it actually stands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things went on addressing the interface as a child of `body` after it moved into a shadow root. None of them raised anything: each is a lookup that answers politely and wrongly, or a rule that matches nothing. `document.getElementById("gitquiet-root")` returns null now. Radix reads a null portal `container` as "put it in `body`", and the gate rule hides every child of `body` that is not marked as ours — so every dropdown in the interface opened, rendered, and could not be seen. Measured on the shipped build: a node portalled to `body` is invisible, the same node inside the root is not. `mount.ts` already had `rootIn`, which asks the shadow root first and their document after; it was simply not exported, so seven call sites kept asking the old way. The theme's fallback target and the motion durations were two of them. A selector led by `html` matches nothing inside a shadow tree, because a shadow tree has no document element. That was the whole weight of the margin reset in `primer.css`, and the whole of the gutter rule in `widths.css` — measured on the shipped build at `padding-left: 0px` on every screen, flush to the window edge with the bar above it still inset, and 14px of margin on a paragraph that asked for none. The reset is split: nothing of GitHub's reaches inside the shadow root, so one class-lighter selector wins there uncontested, while our furniture out in their document still needs the weight. The gutter's `html[data-gitquiet-taken]` guard was never load-bearing — their page has no element with this id, as its own comment said. And the gate rule was hiding something of ours: the SVG filter the bar's glass is made of. `refraction.ts` says in its own comment that Chrome drops a `backdrop-filter` whose filter it cannot resolve, and declines to run one defined in a hidden subtree. It was in a hidden subtree, and the bar had no backdrop. `shadowReach.test.ts` tests the class rather than the three instances, because no test that renders a component can catch a lookup that answers null: no source may ask `document` for the root, no rule may reach the root from `html` or `body`, and nothing we put in their `body` may go unmarked. All three fail against the code as shipped. Co-Authored-By: Claude Opus 5 --- src/ui/Ask.tsx | 6 +- src/ui/Doings.tsx | 4 +- src/ui/SettingsMenu.tsx | 4 +- src/ui/Settle.tsx | 4 +- src/ui/Theme.tsx | 4 +- src/ui/motion.ts | 4 +- src/ui/mount.ts | 12 +++- src/ui/primer.css | 22 ++++++- src/ui/refraction.ts | 13 ++++ src/ui/shadowReach.test.ts | 131 +++++++++++++++++++++++++++++++++++++ src/ui/widths.css | 13 +++- 11 files changed, 200 insertions(+), 17 deletions(-) create mode 100644 src/ui/shadowReach.test.ts diff --git a/src/ui/Ask.tsx b/src/ui/Ask.tsx index 9fe23f57..e38d9772 100644 --- a/src/ui/Ask.tsx +++ b/src/ui/Ask.tsx @@ -6,7 +6,7 @@ import type { ArtName } from "./art" import { useArt } from "./art" import { ASK, ASK_GROUP, ASK_MORE, ASK_NO, ASK_OUT, ASK_YES, FLOAT } from "./dress" import { LOOK } from "./rowDoings" -import { ROOT_ID } from "./mount" +import { rootIn } from "./mount" import { Says } from "./says" export type MergeActions = { @@ -378,7 +378,7 @@ const Caret = ({ const art = useArt() const Down = art["chevron-down"] const Tick = art.tick - const inOurs = typeof document === "undefined" ? null : document.getElementById(ROOT_ID) + const inOurs = typeof document === "undefined" ? null : rootIn(document) return ( @@ -470,7 +470,7 @@ export const Overflow = ({ }) => { const art = useArt() const More = art.more - const inOurs = typeof document === "undefined" ? null : document.getElementById(ROOT_ID) + const inOurs = typeof document === "undefined" ? null : rootIn(document) const offered = verbs.filter((doing) => actions?.[doing] !== undefined) if (offered.length === 0) return null diff --git a/src/ui/Doings.tsx b/src/ui/Doings.tsx index 31984585..47eee268 100644 --- a/src/ui/Doings.tsx +++ b/src/ui/Doings.tsx @@ -8,7 +8,7 @@ import { type Set, useArt } from "./art" import { askAndSay } from "./askAndSay" import { FLOAT } from "./dress" import { Cap } from "./Cap" -import { ROOT_ID } from "./mount" +import { rootIn } from "./mount" import { SpinnerIcon } from "./spinner" import { ARMED, COPY_LETTER, LETTER, LOOK, ORDER, WORD } from "./rowDoings" import { useKeying, useLetters } from "./useLetters" @@ -269,7 +269,7 @@ export const Doings = ({ > {waiting ? : } - + - typeof document === "undefined" ? null : document.getElementById(ROOT_ID) + typeof document === "undefined" ? null : rootIn(document) export type SettingsMenuProps = { readonly settings: Settings diff --git a/src/ui/Settle.tsx b/src/ui/Settle.tsx index db65389f..267f9ca3 100644 --- a/src/ui/Settle.tsx +++ b/src/ui/Settle.tsx @@ -8,7 +8,7 @@ import type { IssueState } from "../domain/issues" import { type ArtName, useArt } from "./art" import { Cap } from "./Cap" import { FIELD, FLOAT, PRESSABLE } from "./dress" -import { ROOT_ID } from "./mount" +import { rootIn } from "./mount" import { useKeying, useLetters } from "./useLetters" import { onward } from "@/observability/report" @@ -175,7 +175,7 @@ export const Settle = ({ state, where, allowed, onSettle, onReopen }: SettleProp Close issue - + { if (scope === "document") return document.documentElement - return document.getElementById(ROOT_ID) + return rootIn(document) } /** diff --git a/src/ui/motion.ts b/src/ui/motion.ts index 75ad2e78..cf437717 100644 --- a/src/ui/motion.ts +++ b/src/ui/motion.ts @@ -1,3 +1,5 @@ +import { rootIn } from "./mount" + /** * How long the stylesheet says a piece of motion takes. * @@ -8,7 +10,7 @@ * tunes the first one. */ export const millisOf = (name: string, fallback: number): number => { - const root = document.getElementById("gitquiet-root") + const root = rootIn(document) if (root === null) return fallback const said = /^\s*([\d.]+)(ms|s)\s*$/.exec(getComputedStyle(root).getPropertyValue(name)) diff --git a/src/ui/mount.ts b/src/ui/mount.ts index 5df50fbe..0417f6e8 100644 --- a/src/ui/mount.ts +++ b/src/ui/mount.ts @@ -118,8 +118,18 @@ const oursIn = (target: Document): ParentNode => ourTree(target) ?? target * `getElementById` found it there without being asked twice. Looking only in the * shadow root made such a container invisible to everything here: the screen was * on the page, nothing could see it, and the bar it had drawn never came down. + * + * Exported because this is the only way to ask, and the components were asking + * the other way. `document.getElementById(ROOT_ID)` was right for as long as the + * root was a child of `body`; the shadow root ended that and the call sites were + * left behind, each one silently answering null. Radix reads a null `container` + * as "portal to `body`", so every dropdown in the interface rendered into their + * document, where the gate rule hides anything unmarked — a menu that opened, + * drew, and could not be seen. The theme's fallback target and the motion + * durations went the same way. There is nothing to find in `document` any more, + * so nothing may look there. */ -const rootIn = (target: Document): HTMLElement | null => +export const rootIn = (target: Document): HTMLElement | null => oursIn(target).querySelector(`#${ROOT_ID}`) ?? target.querySelector(`#${ROOT_ID}`) diff --git a/src/ui/primer.css b/src/ui/primer.css index b1a2ca76..fa9c2558 100644 --- a/src/ui/primer.css +++ b/src/ui/primer.css @@ -151,8 +151,28 @@ * back, and `.markdown-body p` — GitHub's own sixteen pixels between the * paragraphs of a README we insert whole — never hears this at all. */ +:where(#gitquiet-root) :is(p, h1, h2, h3, h4, h5, h6, ul, ol, dl, dd, figure, blockquote, pre) { + margin: 0; +} + +/* + * The same reset for our furniture out in their document, where the weight above + * is still needed and `html` still matches. + * + * These were one rule until the interface moved into a shadow root, and the + * `html` in front of it — the whole of its weight — quietly stopped matching + * anything: `html` is the document's element and a shadow tree has none. So the + * reset was off for every screen, and every paragraph and heading this interface + * writes carried the browser's own margin. It showed as slack between the rows of + * a panel, most of a line of it, on a layout built to sit tight. + * + * Split rather than made to match both, because the two halves want different + * weights for different reasons. Nothing of GitHub's reaches inside the shadow + * root, so one class-lighter selector wins there uncontested; out here theirs is + * still on the page and this has to outweigh `p { margin-bottom: 10px }`. + */ html - :where(#gitquiet-root, [data-gitquiet-outside]) + :where([data-gitquiet-outside]) :is(p, h1, h2, h3, h4, h5, h6, ul, ol, dl, dd, figure, blockquote, pre) { margin: 0; } diff --git a/src/ui/refraction.ts b/src/ui/refraction.ts index c7a4a58f..ec9c605a 100644 --- a/src/ui/refraction.ts +++ b/src/ui/refraction.ts @@ -15,6 +15,8 @@ * floats over a page the reader is still scrolling. */ +import { OUTSIDE } from "./mount" + /** The name the stylesheet asks for. See `glass.css`. */ export const REFRACTION_ID = "gitquiet-refraction" @@ -52,6 +54,17 @@ export const keepRefraction = (page: Document): void => { const host = page.createElement("div") host.id = HOST_ID + /* + * Marked, because the gate rule hides every child of `body` that is not. + * + * The paragraph above says a filter in a hidden subtree is one Chrome declines + * to run, and that the failure is a bar with no backdrop at all. The gate rule + * this interface now hides their page with put `display: none` on exactly this + * host — our own definition, swept up with theirs — so the effect this whole + * file exists for was off in production and nothing said so. The mark is what + * tells that rule ours from theirs. + */ + host.setAttribute(OUTSIDE, "") host.setAttribute("aria-hidden", "true") host.style.position = "absolute" host.style.width = "0" diff --git a/src/ui/shadowReach.test.ts b/src/ui/shadowReach.test.ts new file mode 100644 index 00000000..2ea78ca8 --- /dev/null +++ b/src/ui/shadowReach.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, test } from "bun:test" +import { readdirSync, readFileSync } from "node:fs" +import { join } from "node:path" +import { OUTSIDE, ROOT_ID, rootIn } from "./mount" +import { outsideHost } from "./outside" +import { theHost } from "./theHost" +import { keepRefraction } from "./refraction" + +/** + * What moving into a shadow root put out of reach, and keeps out of reach. + * + * The migration moved the whole interface under `#gitquiet-host`'s shadow root. + * Three kinds of thing carried on addressing it as though it were still a child + * of `body`, and every one of them failed silently — no error, no warning, a + * lookup that answered null or a rule that matched nothing: + * + * - `document.getElementById("gitquiet-root")` returns null now. Radix reads a + * null portal `container` as "put it in `body`", so every dropdown in the + * interface rendered into their document, where the gate rule hid it. The + * reader saw a menu that would not open. + * - A selector led by `html` matches nothing inside a shadow tree, because a + * shadow tree has no document element. That was the whole weight of the margin + * reset, so every paragraph and heading in the interface wore the browser's + * default margin. + * - The gate rule hides every child of `body` without the outside mark, and one + * of the things it was hiding was ours: the SVG filter the bar's glass is made + * of, defined in a subtree Chrome then declined to run. + * + * Each of those was one line. None of them could fail a test that only renders + * components, because each one is a lookup that answers politely and wrongly. So + * the class is tested rather than the three instances. + */ +const uiDir = new URL(".", import.meta.url).pathname +const sourcesIn = (dir: string, ext: string): ReadonlyArray<[string, string]> => + readdirSync(dir) + .filter((name) => name.endsWith(ext) && !name.includes(".test.")) + .map((name) => [name, readFileSync(join(dir, name), "utf8")] as [string, string]) + +describe("nothing looks for our own elements in their document", () => { + test("no source asks `document` for the root", () => { + // The lookup that broke every dropdown. `rootIn` asks the shadow root first + // and their document after, which is the only way that answers on both. + const guilty = [...sourcesIn(uiDir, ".ts"), ...sourcesIn(uiDir, ".tsx")] + .filter(([name]) => name !== "mount.ts") + .filter(([, text]) => + /document\.getElementById\(\s*(ROOT_ID|["'`]gitquiet-root)/.test(text) || + /document\.querySelector\w*\(\s*["'`]#gitquiet-root/.test(text) + ) + .map(([name]) => name) + + expect(guilty).toEqual([]) + }) + + test("and `rootIn` finds it through the shadow boundary", () => { + // The positive half: the helper the rule above points at has to actually + // answer where `document` cannot, or the rule is just a ban. + const page = document.implementation.createHTMLDocument("github") + const { shadow } = theHost(page) + const root = page.createElement("div") + root.id = ROOT_ID + shadow.append(root) + + // The lookup the components used to make, against the tree they now stand in. + expect(page.getElementById(ROOT_ID)).toBeNull() + expect(rootIn(page)).toBe(root) + }) +}) + +describe("no rule reaches our root from the document element", () => { + test("nothing keyed on `html` or `body` styles anything inside the shadow root", () => { + /* + * `html` and `body` are the document's elements. A shadow tree has neither, + * so a rule led by one of them is a rule that matches nothing in here — and + * it fails by doing nothing at all, which is the hardest kind of failure to + * notice. The rules that legitimately key on `html` are the gates and the + * widths, and every one of those styles GitHub's page or our furniture out + * in it, never the interface. + */ + const guilty: string[] = [] + for (const [name, text] of sourcesIn(uiDir, ".css")) { + // Every prelude in the file: the run of characters before an opening brace + // that is not itself a brace, which is a selector list or an at-rule. + for (const found of text.replace(/\/\*[\s\S]*?\*\//g, "").matchAll(/([^{}]+)\{/g)) { + const selector = (found[1] ?? "").trim() + if (selector.startsWith("@")) continue + if (!selector.includes(`#${ROOT_ID}`)) continue + // A selector list, because one rule may carry several. + for (const one of selector.split(",")) { + if (/^(html|body)\b/.test(one.trim()) && one.includes(`#${ROOT_ID}`)) { + guilty.push(`${name}: ${one.trim().replace(/\s+/g, " ")}`) + } + } + } + } + + expect(guilty).toEqual([]) + }) +}) + +describe("everything of ours in their document is marked as ours", () => { + test("the glass filter is spared by the gate rule", () => { + /* + * It was not, and the file that puts it there says in its own comment why + * that is fatal: Chrome drops a `backdrop-filter` whose filter it cannot + * resolve, and a filter inside a `display: none` subtree is one it declines + * to run. The bar had no backdrop in production and nothing failed. + */ + const page = document.implementation.createHTMLDocument("github") + keepRefraction(page) + + const host = page.getElementById("gitquiet-glass") + expect(host).not.toBeNull() + expect(host?.hasAttribute(OUTSIDE)).toBe(true) + }) + + test("and so is everything else we put straight into body", () => { + // The general form. Anything appended to their `body` is subject to the gate + // rule, so the mark is not a detail of one host — it is the condition of + // being allowed to live there at all. + const page = document.implementation.createHTMLDocument("github") + keepRefraction(page) + outsideHost(page, "gitquiet-over") + + const unmarked = [...page.body.children] + .filter((el) => el.id !== "gitquiet-host") + .filter((el) => !el.hasAttribute(OUTSIDE)) + .map((el) => el.id || el.tagName) + + expect(unmarked).toEqual([]) + }) +}) diff --git a/src/ui/widths.css b/src/ui/widths.css index 65105f4c..9657e435 100644 --- a/src/ui/widths.css +++ b/src/ui/widths.css @@ -86,10 +86,17 @@ * that adds its own is a screen that disagrees with the bar the moment either number changes, * which is what `WorkingSetScreen` did until this rule took the inset off it. * - * It cannot reach anything of GitHub's: they have no element with this id, and the takeover - * attribute means it says nothing at all on a page this extension left alone. + * It cannot reach anything of GitHub's: they have no element with this id, which is the whole + * of what keeps it off their page. + * + * It was `html[data-gitquiet-taken] #gitquiet-root` until the interface moved into a shadow + * root, and `html` is the document's element — a shadow tree has none, so the rule matched + * nothing and every screen lost its gutter, flush to the edge of the window with the bar above + * it still inset. The takeover attribute was never load-bearing here, as the line above already + * says: their page has no element with this id to reach. So it goes, and the id stands on its + * own in both trees. */ -html[data-gitquiet-taken] #gitquiet-root { +#gitquiet-root { padding-inline: var(--gitquiet-gutter); } From 97eb1c74578687873ec1b6058c5cc86e6f4b2f7c Mon Sep 17 00:00:00 2001 From: flazouh Date: Wed, 16 Sep 2026 22:15:01 +0200 Subject: [PATCH 2/6] Give our own furniture a font, now that theirs is off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bar came out in Times New Roman. Measured on a pull request: `font-family: Times`, `font-size: 16px` on the bar, against an interface set in Inter at fourteen inside the shadow root — which is also why its padding read as wrong, every row in it a seventh taller than it was drawn to be. `#gitquiet-root` declares the font and the size for itself. Everything outside it — the bar, the hover cards, anything Radix portals — declared nothing, and never needed to: it stood in `body` and inherited whatever GitHub's stylesheet put there. Then we started turning their stylesheets off for the recalculation saving, and `body` was left with no font at all. Nothing failed; the interface simply fell back to the browser's own serif in the one place it could still be seen. So the furniture outside the root declares the same baseline the root does, through `:where` so a utility on the bar still wins. The test asserts the two agree, because there is nothing left to inherit from and the next thing we add out there will not know that either. Also takes the host off the document between tests. It is module state twice over — an element in `body` and an entry in a map keyed by the document — so a file that stands the interface up hands the next file in the same worker a shadow root with the last file's screen still in it. This is offered as the cause of a CI-only failure on the previous commit and is not proven to be: the suite is green here with and without it, on every combination of files tried, and the sharding that decides which files share a worker is a fact about the machine. The cleanup belongs there either way, beside the eight others that are there for the same reason. Co-Authored-By: Claude Opus 5 --- src/ui/primer.css | 23 +++++++++++++++++++++++ src/ui/shadowReach.test.ts | 26 ++++++++++++++++++++++++++ src/ui/theHost.ts | 22 ++++++++++++++++++++++ tests/setup.ts | 14 ++++++++++++++ 4 files changed, 85 insertions(+) diff --git a/src/ui/primer.css b/src/ui/primer.css index fa9c2558..fe293af1 100644 --- a/src/ui/primer.css +++ b/src/ui/primer.css @@ -87,6 +87,29 @@ min-width: 0; } +/** + * The same typographic baseline for our furniture out in their document. + * + * `#gitquiet-root` above declares the font and the size; everything outside it — + * the bar, the hover cards, anything Radix portals — used to need no such + * declaration, because it stood in `body` and inherited whatever GitHub's + * stylesheet put there. Then we started turning their stylesheets off, and + * `body` was left with no font at all: the bar rendered in Times New Roman at + * the browser's own sixteen pixels, against an interface set in Inter at + * fourteen. Measured on a pull request — `font-family: Times`, `font-size: 16px` + * on the bar — which is also why its padding looked wrong, every row inside it + * being a seventh taller than it was drawn to be. + * + * Inheritance is not a thing to rely on once you have taken the page it comes + * from. Through `:where` so it costs nothing: a `text-sm` on the bar itself + * still wins, and so does every utility inside it. + */ +:where([data-gitquiet-outside]) { + color: var(--color-ink); + font-family: var(--font-sans); + font-size: var(--text-base); +} + #gitquiet-root *, #gitquiet-root *::before, #gitquiet-root *::after, diff --git a/src/ui/shadowReach.test.ts b/src/ui/shadowReach.test.ts index 2ea78ca8..5b2bc516 100644 --- a/src/ui/shadowReach.test.ts +++ b/src/ui/shadowReach.test.ts @@ -97,6 +97,32 @@ describe("no rule reaches our root from the document element", () => { }) }) +describe("nothing of ours inherits from a page we have undressed", () => { + test("our furniture in their document declares its own typography", () => { + /* + * The bar and the hover cards stand in `body`, and used to take their font + * from whatever GitHub's stylesheet put there. Then we started turning their + * stylesheets off for the recalculation saving, and `body` was left with no + * font at all: the bar came out in Times New Roman at sixteen pixels beside + * an interface set in Inter at fourteen, which is also why its padding read + * as wrong — every row in it a seventh taller than it was drawn to be. + * + * Whatever the root declares for itself, the furniture outside it has to + * declare too. There is nothing left to inherit from. + */ + const primer = readFileSync(join(uiDir, "primer.css"), "utf8") + const declaredFor = (selector: string): ReadonlyArray => { + const at = primer.indexOf(`\n${selector} {`) + if (at === -1) return [] + const block = primer.slice(at, primer.indexOf("}", at)) + return ["font-family", "font-size", "color"].filter((one) => block.includes(`${one}:`)) + } + + expect(declaredFor("#gitquiet-root")).not.toEqual([]) + expect(declaredFor(":where([data-gitquiet-outside])")).toEqual(declaredFor("#gitquiet-root")) + }) +}) + describe("everything of ours in their document is marked as ours", () => { test("the glass filter is spared by the gate rule", () => { /* diff --git a/src/ui/theHost.ts b/src/ui/theHost.ts index 2fd35636..c8b8c254 100644 --- a/src/ui/theHost.ts +++ b/src/ui/theHost.ts @@ -259,6 +259,28 @@ export const keepTheirStylesOff = (target: Document): void => { watching.observe(target.documentElement, { childList: true, subtree: true }) } +/** + * The host off a document, and forgotten, for a suite that is many documents in one. + * + * The host is module state twice over: an element in `body` and an entry in + * {@link hosts} keyed by the document. A test file leaves both behind, and the + * file that runs next in the same worker inherits a shadow root with the last + * file's screen still standing in it. Everything that looks for our tree then + * finds that one first — `rootIn` answers with a root nobody rendered, and a menu + * portalled into it lands where `screen` cannot see it, so the test reads as a + * menu that never opened. + * + * It surfaced as a CI-only failure, which is the signature of the fault rather + * than a detail of it: `bun test --parallel` shards by core count, so which files + * share a worker differs between a runner and a developer's machine, and the same + * commit was green here and red there. + */ +export const forgetTheHost = (page: Document): void => { + page.getElementById(HOST_ID)?.remove() + hosts.delete(page) + letTheirStylesBack(page) +} + /** Their sheets back on, and the watch let go. */ export const letTheirStylesBack = (target: Document): void => { watching?.disconnect() diff --git a/tests/setup.ts b/tests/setup.ts index ff62ed03..212a23fe 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -3,6 +3,7 @@ import { afterEach, setDefaultTimeout } from "bun:test" import { forgetFlights } from "../src/github/flight" import { forgetDrawn } from "../src/ui/lastDrawn" import { forgetTheSpot } from "../src/shell/handOver" +import { forgetTheHost } from "../src/ui/theHost" import { forgetLanded } from "../src/ui/landing" import { forgetLanded as forgetOurWrites } from "../src/github/landed" import { forgetEverything } from "./storage" @@ -151,3 +152,16 @@ afterEach(forgetDrawn) * must not inherit where the file before it left the thing. */ afterEach(forgetTheSpot) + +/* + * And the host the interface stands in, which is one element and one entry in a map + * keyed by the document. + * + * A file that stands the interface up leaves a shadow root in `body` with its last + * screen still in it, and the file after it in the same worker inherits both. Every + * lookup for our tree finds that one first: a menu portalled into it renders where + * `screen` cannot see it, and the test reads as a menu that never opened. It showed + * up only on CI, because `--parallel` shards by core count and which files share a + * worker is a fact about the machine. + */ +afterEach(() => forgetTheHost(document)) From 1dbfe22ad32ddd71a6214030407095edf36f374a Mon Sep 17 00:00:00 2001 From: flazouh Date: Wed, 16 Sep 2026 22:22:32 +0200 Subject: [PATCH 3/6] Report what a press on a pull request actually did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every live check of Following has been made on a blob page, because that is the only page a probe can reach on its own. Measured rather than assumed: a fresh profile on a *public* pull request gets our own signed-out card, with `user-login` empty and only anonymous cookies, because the diff is drawn from routes that answer to a session. So nothing a blob page proves carries over to a diff, which is a different drawing — two halves, a marker column, line numbers belonging to one side — and a reported fault is on a diff: the underline appears and the press does nothing. This reports rather than judges. For each name it says what the renderer handed over — the row's attributes, the token's `data-char`, where the word sits inside it — then holds the key, then presses, and says which of three things happened: a panel opened, the pane moved, or nothing at all. "Nothing happens" is three faults wearing one coat, and from outside the screen they are identical: a press read as a press on a use scrolls to a line already on screen and looks like a dead click. It needs a profile that has been signed into once and kept, which `GITQUIET_CDP_PROFILE` already provides. No cookie is read or moved anywhere. Checked against a page it can reach, so the probe is not the thing under suspicion: on `p-limit`'s `index.d.ts` it tells all three outcomes apart — `LimitFunction` opens its panel, `concurrency` opens its own, and `Options` moves the pane to where it is written. Co-Authored-By: Claude Opus 5 --- scripts/probe-diff-following.ts | 182 ++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 scripts/probe-diff-following.ts diff --git a/scripts/probe-diff-following.ts b/scripts/probe-diff-following.ts new file mode 100644 index 00000000..ec79e5d6 --- /dev/null +++ b/scripts/probe-diff-following.ts @@ -0,0 +1,182 @@ +/** + * Following on a pull request, reported in full rather than as pass or fail. + * + * Every live check of Following so far was made on a blob page, because that is + * the only page this can reach on its own: a pull request is drawn from routes + * that answer to a session, and a fresh profile has none — measured, not + * assumed, on a *public* pull request, which showed our own signed-out card with + * `user-login` empty and only anonymous cookies. A diff is a different drawing — + * two halves, a marker column, line numbers belonging to one side — so none of + * what a blob page proves carries over on its own. + * + * So this needs a browser that is signed in, which means a profile that has been + * signed in once and kept: + * + * GITQUIET_CDP_PROFILE=~/.gitquiet-qa bun scripts/probe-diff-following.ts \ + * --page 'https://github.com/OWNER/REPO/pull/N/files' \ + * --names 'readBodyWithinLimit@12,AsyncResult@1,BodyReadFailure@5' + * + * Sign in once in that profile and it stays signed in between runs. Nothing here + * reads a cookie or moves one anywhere. + * + * For each name it says what the renderer handed over, what the underline did, + * and what the press did — a panel, a move, or nothing — because "nothing + * happens" is three different faults wearing one coat, and which one it is + * cannot be told from the outside. + */ +import { PANES, withExtension } from "./chrome" + +const argued = (flag: string): string | undefined => { + const at = Bun.argv.indexOf(flag) + return at === -1 ? undefined : Bun.argv[at + 1] +} + +const PAGE = argued("--page") ?? "https://github.com/flazouh/gitquiet/pull/82/files" + +/** `name@line` a few times over, which is what a reader would press. */ +const NAMES = (argued("--names") ?? "Secret@11").split(",").map((one) => { + const [word, line] = one.trim().split("@") + return { word: word ?? "", line: Number(line ?? 1) } +}) + +const session = await withExtension(PAGE, `${import.meta.dir}/../.output/chrome-mv3`) +const sleep = (ms: number) => new Promise((go) => setTimeout(go, ms)) + +type Report = { + readonly drew: boolean + readonly row?: string + readonly token?: { char: string | null; text: string; inside: number } + readonly underlined?: boolean + readonly press?: string + readonly why?: string +} + +const look = (word: string, line: number): Promise => + session.evaluate(` + (async () => { + ${PANES} + const sleep = (ms) => new Promise((go) => setTimeout(go, ms)) + + let drawn = [] + for (let tries = 0; tries < 60; tries++) { + drawn = panes() + if (drawn.length > 0) break + await sleep(500) + } + if (drawn.length === 0) return { drew: false, why: "no pane drew at all — signed out, or the diff never arrived" } + + const edge = "[^A-Za-z0-9_$]" + let row = null, token = null, inside = -1 + for (const root of drawn) { + for (const one of root.querySelectorAll('[data-line="${line}"]')) { + for (const span of one.querySelectorAll("span")) { + if (span.querySelector("span")) continue + const text = span.textContent || "" + const found = text.search(new RegExp("(^|" + edge + ")" + ${JSON.stringify(word)} + "(" + edge + "|$)")) + if (found === -1) continue + row = one; token = span; inside = text.indexOf(${JSON.stringify(word)}, found) + break + } + if (token) break + } + if (token) break + } + if (!token) return { drew: true, why: "the renderer drew no token holding that word on that line" } + + row.scrollIntoView({ block: "center", behavior: "instant" }) + await sleep(400) + + const at = token.getBoundingClientRect() + const text = token.textContent || "" + const wide = at.width / Math.max(1, text.length) + const where = { + bubbles: true, composed: true, cancelable: true, + clientX: at.left + (inside + ${JSON.stringify(word)}.length / 2) * wide, + clientY: at.top + at.height / 2, + metaKey: true, pointerId: 1, isPrimary: true, pointerType: "mouse" + } + + const seen = { + drew: true, + row: [...row.attributes].map((a) => a.name + "=" + a.value).join(" "), + token: { char: token.getAttribute("data-char"), text: text.slice(0, 60), inside } + } + + document.dispatchEvent(new KeyboardEvent("keydown", { key: "Meta", bubbles: true })) + token.dispatchEvent(new PointerEvent("pointerover", where)) + token.dispatchEvent(new PointerEvent("pointermove", where)) + + let underlined = false + for (let waited = 0; waited < 15000; waited += 50) { + if ((token.style.textDecoration || "") !== "" || + [...token.querySelectorAll("span")].some((s) => (s.style.textDecoration || "") !== "")) { + underlined = true + break + } + await sleep(50) + } + if (!underlined) return { ...seen, underlined: false, why: "held the key and it never underlined" } + + // Where the pane is before the press, so a move can be told from nothing. + const scrollerOf = (el) => { + let up = el + while (up) { + if (up.scrollHeight > up.clientHeight + 4) return up + up = up.parentElement || (up.getRootNode() || {}).host || null + } + return document.scrollingElement + } + const scroller = scrollerOf(row) + const before = scroller ? scroller.scrollTop : 0 + + token.dispatchEvent(new PointerEvent("pointerdown", where)) + token.dispatchEvent(new PointerEvent("pointerup", where)) + token.dispatchEvent(new MouseEvent("click", where)) + + let panel = null + for (let waited = 0; waited < 8000; waited += 100) { + panel = document.querySelector('[aria-label^="Uses of "]') + if (panel) break + await sleep(100) + } + const after = scroller ? scroller.scrollTop : 0 + + return { + ...seen, + underlined: true, + press: panel + ? "opened " + JSON.stringify(panel.getAttribute("aria-label")) + : after !== before + ? "moved the pane from " + before + " to " + after + : "nothing: no panel, and the pane did not move" + } + })() + `) + +try { + console.log(`page: ${PAGE}\n`) + for (const { word, line } of NAMES) { + const seen = await look(word, line) + console.log(`${word} @ L${line}`) + for (const [key, value] of Object.entries(seen)) { + console.log(` ${key}: ${typeof value === "object" ? JSON.stringify(value) : value}`) + } + console.log() + await session.evaluate(` + (() => { + document.dispatchEvent(new KeyboardEvent("keyup", { key: "Meta", bubbles: true })) + document.body.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, composed: true, pointerId: 1, isPrimary: true, pointerType: "mouse" })) + })() + `) + await sleep(500) + } + + const problems = session.problems() + if (problems.length > 0) { + console.log("the page logged:") + for (const one of problems.slice(0, 8)) console.log(` ${one.split("\n")[0]}`) + } +} finally { + await session.screenshot(`${import.meta.dir}/../.output/qa/diff-following.png`) + session.stop() +} From fd73b1c26b0d8386684242f4c8b0db5c0111eee4 Mon Sep 17 00:00:00 2001 From: flazouh Date: Wed, 16 Sep 2026 22:32:26 +0200 Subject: [PATCH 4/6] Send the menus to the host every other overlay already used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dropdowns were the only overlays in the interface not portalling to `outsideHost(document, OVER_ID)`. The hover cards go there, the settings dialog goes there, the toasts go there — a child of `body` carrying the outside mark, so the gate rule spares it, and the theme tokens, so it is painted. The menus named `#gitquiet-root` instead, which was a child of `body` too until the interface moved into a shadow root and that lookup started answering null. The previous commit pointed them at `rootIn`, which found the root again and made them visible to a reader. It also put a menu inside the shadow root, where `screen` cannot see it: fifty-three tests across four files went red on CI and stayed green here, because `bun test --parallel` shards by core count and which files share a worker is a fact about the machine. Two commits spent on a test isolation theory that was not the cause — the cause is that the root was never the right target. An overlay exists to escape whatever its row is clipped by, which is the one thing standing inside the root cannot do. `rootIn` stays exported for the two callers that genuinely want the root: the theme's fallback target and the motion durations. The settings panel test asked that the panel be inside `#gitquiet-root`, which was one painted place rather than the property its own comment described. It now asks what it meant: that the panel stands in a host of ours, which is a host the theme paints and the gate rule spares. Co-Authored-By: Claude Opus 5 --- src/ui/Ask.tsx | 6 +++--- src/ui/Doings.tsx | 4 ++-- src/ui/SettingsMenu.tsx | 4 ++-- src/ui/Settle.tsx | 4 ++-- src/ui/settingsMenu.test.tsx | 29 +++++++++++++++++++--------- src/ui/shadowReach.test.ts | 37 +++++++++++++++++++++++++++++++++++- 6 files changed, 65 insertions(+), 19 deletions(-) diff --git a/src/ui/Ask.tsx b/src/ui/Ask.tsx index e38d9772..1fb9d3ec 100644 --- a/src/ui/Ask.tsx +++ b/src/ui/Ask.tsx @@ -6,7 +6,7 @@ import type { ArtName } from "./art" import { useArt } from "./art" import { ASK, ASK_GROUP, ASK_MORE, ASK_NO, ASK_OUT, ASK_YES, FLOAT } from "./dress" import { LOOK } from "./rowDoings" -import { rootIn } from "./mount" +import { OVER_ID, outsideHost } from "./outside" import { Says } from "./says" export type MergeActions = { @@ -378,7 +378,7 @@ const Caret = ({ const art = useArt() const Down = art["chevron-down"] const Tick = art.tick - const inOurs = typeof document === "undefined" ? null : rootIn(document) + const inOurs = typeof document === "undefined" ? null : outsideHost(document, OVER_ID) return ( @@ -470,7 +470,7 @@ export const Overflow = ({ }) => { const art = useArt() const More = art.more - const inOurs = typeof document === "undefined" ? null : rootIn(document) + const inOurs = typeof document === "undefined" ? null : outsideHost(document, OVER_ID) const offered = verbs.filter((doing) => actions?.[doing] !== undefined) if (offered.length === 0) return null diff --git a/src/ui/Doings.tsx b/src/ui/Doings.tsx index 47eee268..fc6f8a8c 100644 --- a/src/ui/Doings.tsx +++ b/src/ui/Doings.tsx @@ -8,7 +8,7 @@ import { type Set, useArt } from "./art" import { askAndSay } from "./askAndSay" import { FLOAT } from "./dress" import { Cap } from "./Cap" -import { rootIn } from "./mount" +import { OVER_ID, outsideHost } from "./outside" import { SpinnerIcon } from "./spinner" import { ARMED, COPY_LETTER, LETTER, LOOK, ORDER, WORD } from "./rowDoings" import { useKeying, useLetters } from "./useLetters" @@ -269,7 +269,7 @@ export const Doings = ({ > {waiting ? : } - + - typeof document === "undefined" ? null : rootIn(document) + typeof document === "undefined" ? null : outsideHost(document, OVER_ID) export type SettingsMenuProps = { readonly settings: Settings diff --git a/src/ui/Settle.tsx b/src/ui/Settle.tsx index 267f9ca3..44d579cf 100644 --- a/src/ui/Settle.tsx +++ b/src/ui/Settle.tsx @@ -8,7 +8,7 @@ import type { IssueState } from "../domain/issues" import { type ArtName, useArt } from "./art" import { Cap } from "./Cap" import { FIELD, FLOAT, PRESSABLE } from "./dress" -import { rootIn } from "./mount" +import { OVER_ID, outsideHost } from "./outside" import { useKeying, useLetters } from "./useLetters" import { onward } from "@/observability/report" @@ -175,7 +175,7 @@ export const Settle = ({ state, where, allowed, onSettle, onReopen }: SettleProp Close issue - + `, because the rest of the document is GitHub's page and our names on - * their root would repaint their chrome. So anything Radix portals to - * `document.body` is drawn with the stylesheet's defaults, which are the light - * pack — white panel, near-black text, on a dark page. `outside.ts` was written - * for that failure, having been paid for once by the bar. + * The colours are inline custom properties rather than rules on ``, because + * the rest of the document is GitHub's page and our names on their root would + * repaint their chrome. So anything Radix portals to a plain `document.body` is + * drawn with the stylesheet's defaults, which are the light pack — white panel, + * near-black text, on a dark page. `outside.ts` was written for that failure, + * having been paid for once by the bar. + * + * It used to say "inside `#gitquiet-root`", and that was one painted place rather + * than the property. The interface has since moved into a shadow root, where the + * root is out of reach of a lookup on `document`, and an overlay has to escape + * whatever its row is clipped by — which is the one thing a panel standing inside + * the root cannot do. So this asks what it always meant to ask: that the panel + * stands in a host of ours, which is a host `outside.ts` has painted and the gate + * rule spares. */ describe("the panel of knobs", () => { const ourRoot = (): HTMLElement => { @@ -43,7 +51,10 @@ describe("the panel of knobs", () => { const panel = await opened(root) - expect(root.contains(panel)).toBe(true) + // `closest` rather than a named id: what has to be true is that the panel is + // inside something of ours, and the mark is what says so — to the theme that + // paints it and to the gate rule that would otherwise hide it. + expect(panel.closest(`[${OUTSIDE}]`)).not.toBeNull() }) /** diff --git a/src/ui/shadowReach.test.ts b/src/ui/shadowReach.test.ts index 5b2bc516..984dfba8 100644 --- a/src/ui/shadowReach.test.ts +++ b/src/ui/shadowReach.test.ts @@ -17,7 +17,9 @@ import { keepRefraction } from "./refraction" * - `document.getElementById("gitquiet-root")` returns null now. Radix reads a * null portal `container` as "put it in `body`", so every dropdown in the * interface rendered into their document, where the gate rule hid it. The - * reader saw a menu that would not open. + * reader saw a menu that would not open. The answer is the host every other + * overlay already used — see below; the root was never the right target for a + * thing that has to escape whatever clips it. * - A selector led by `html` matches nothing inside a shadow tree, because a * shadow tree has no document element. That was the whole weight of the margin * reset, so every paragraph and heading in the interface wore the browser's @@ -97,6 +99,39 @@ describe("no rule reaches our root from the document element", () => { }) }) +describe("every overlay goes to the one host built for overlays", () => { + test("no menu portals into the screen root", () => { + /* + * The hover cards, the settings dialog and the toasts all portal to + * `outsideHost(document, OVER_ID)`: a child of `body` that carries the + * outside mark, so the gate rule spares it, and the theme tokens, so it is + * painted. The dropdown menus were the only overlays not using it — they + * named `#gitquiet-root` instead, which was a child of `body` too until the + * interface moved into a shadow root and the lookup started answering null. + * + * Radix then put them in `body` unmarked, where the gate rule hid them. A + * menu that opens and cannot be seen. + * + * The root is the wrong target regardless: an overlay exists to escape + * whatever its row is clipped by, which is the one thing standing inside the + * root cannot do. + */ + const guilty = [...sourcesIn(uiDir, ".tsx")] + .filter(([, text]) => /container=\{[^}]*\bROOT_ID\b|container=\{[^}]*\brootIn\(/.test(text)) + .map(([name]) => name) + + expect(guilty).toEqual([]) + }) + + test("and the host they do use is one their page cannot hide", () => { + const page = document.implementation.createHTMLDocument("github") + const host = outsideHost(page, "gitquiet-over") + + expect(host.parentElement).toBe(page.body) + expect(host.hasAttribute(OUTSIDE)).toBe(true) + }) +}) + describe("nothing of ours inherits from a page we have undressed", () => { test("our furniture in their document declares its own typography", () => { /* From 51c9c2f3615e9bbc5f20b322767672f4440e2915 Mon Sep 17 00:00:00 2001 From: flazouh Date: Wed, 16 Sep 2026 22:40:12 +0200 Subject: [PATCH 5/6] Leave the stylesheet's clock where the test seam writes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `millisOf` reads every motion duration off `#gitquiet-root`, and `tests/paced.ts` plants a root in `document.body` and writes the durations a test needs onto it. Both halves of that seam address the document, and pointing one of them at `rootIn` made them disagree: once a screen stands a root inside the shadow root, the lookup prefers that one and the planted durations are ignored. A dissolve paced to never finish finishes at once, React throws the element away and mounts a second one already faded out, and the test that asks for a transition with something to transition from gets two different elements. Green here and red on CI both times, because whether there is a root in the shadow at that moment depends on what else is in the worker. Moving the clock into the shadow root is worth doing — the CSS owns these numbers and the sheet is in there now — but it is a change to both halves of the seam and it is not this one. Co-Authored-By: Claude Opus 5 --- src/ui/motion.ts | 20 +++++++++++++++++--- src/ui/shadowReach.test.ts | 6 +++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/ui/motion.ts b/src/ui/motion.ts index cf437717..8906cd85 100644 --- a/src/ui/motion.ts +++ b/src/ui/motion.ts @@ -1,5 +1,3 @@ -import { rootIn } from "./mount" - /** * How long the stylesheet says a piece of motion takes. * @@ -10,7 +8,23 @@ import { rootIn } from "./mount" * tunes the first one. */ export const millisOf = (name: string, fallback: number): number => { - const root = rootIn(document) + /* + * `document` on purpose, and not our tree. + * + * This is one half of a seam whose other half is `tests/paced.ts`, which + * plants a `#gitquiet-root` in `document.body` and writes the durations a test + * needs onto it. Reading through `rootIn` instead makes the two halves + * disagree the moment a screen stands a root inside the shadow root: the + * planted durations are ignored, a dissolve paced to never finish finishes at + * once, and React throws the element away and mounts a second one — which is a + * transition with nothing to transition from. It cost two CI runs to find, + * being a fault that depends on what else is in the worker. + * + * Moving the clock into the shadow root is worth doing — the CSS owns these + * numbers and the sheet is in there now — but it is a change to both halves and + * it is not this one. + */ + const root = document.getElementById("gitquiet-root") if (root === null) return fallback const said = /^\s*([\d.]+)(ms|s)\s*$/.exec(getComputedStyle(root).getPropertyValue(name)) diff --git a/src/ui/shadowReach.test.ts b/src/ui/shadowReach.test.ts index 984dfba8..0ea34892 100644 --- a/src/ui/shadowReach.test.ts +++ b/src/ui/shadowReach.test.ts @@ -43,7 +43,11 @@ describe("nothing looks for our own elements in their document", () => { // The lookup that broke every dropdown. `rootIn` asks the shadow root first // and their document after, which is the only way that answers on both. const guilty = [...sourcesIn(uiDir, ".ts"), ...sourcesIn(uiDir, ".tsx")] - .filter(([name]) => name !== "mount.ts") + // `mount.ts` defines the helper. `motion.ts` reads the stylesheet's clock + // through `document` deliberately, because the test seam that writes those + // durations plants its root there — see the comment on `millisOf`, and the + // two CI runs it cost to establish that the two halves have to agree. + .filter(([name]) => name !== "mount.ts" && name !== "motion.ts") .filter(([, text]) => /document\.getElementById\(\s*(ROOT_ID|["'`]gitquiet-root)/.test(text) || /document\.querySelector\w*\(\s*["'`]#gitquiet-root/.test(text) From 9c218e79d264171bff65b669f5af8af7f0a83293 Mon Sep 17 00:00:00 2001 From: flazouh Date: Wed, 16 Sep 2026 23:28:57 +0200 Subject: [PATCH 6/6] Check a package's own path against the files the repository has MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A name borrowed from a package this repository holds itself resolved to a path with no ending on it. `@org/type-utils` held at `packages/type-utils` and imported as `@org/type-utils/result-monad` was read as `packages/type-utils/result-monad`, which is not a file — nothing on disk is called that, and the check that would have caught it was never made. What followed is worse than no answer. The path matched no file, so the answer fell through to the first Writing of that name anywhere in the repository: a different thing with the same spelling, offered to the reader as the place their name is written. That is the one mistake this whole feature exists to prevent, and it was being made silently. `endingsFor` is the ending list `couldBe` already had, split out, because a relative specifier is not the only way to arrive at a path that is missing one. `within` is `reaching` for a path that is already a path, and the package branch now goes through it — so a path built from a package's layout is checked against the repository's real files before it is believed, like every other answer here. Reported from a monorepo's pull request, where the import is `@openrouter-monorepo/type-utils/result-monad` and pressing `AsyncResult` did nothing at all. The console script alongside is what found it: the probes cannot reach a pull request — it is drawn from routes that answer to a session, and a fresh profile gets our signed-out card, measured on a public one — so it reports from the reader's own browser instead. It is also what proved a press on a diff opens the panel perfectly well, which is the opposite of what was reported and of what I expected. Still unresolved, and not this: a tsconfig `paths` alias. `@/routes/vault` is bare by the only test there is, so it goes to package resolution and finds no package. That needs the repository's `tsconfig.json` read and its patterns applied, which is a feature rather than a fix. Co-Authored-By: Claude Opus 5 --- scripts/following-in-the-console.js | 201 ++++++++++++++++++++++++++++ src/entrypoints/offscreen/ledger.ts | 21 ++- src/ledger/reaching.test.ts | 51 ++++++- src/ledger/reaching.ts | 38 +++++- 4 files changed, 304 insertions(+), 7 deletions(-) create mode 100644 scripts/following-in-the-console.js diff --git a/scripts/following-in-the-console.js b/scripts/following-in-the-console.js new file mode 100644 index 00000000..790d1227 --- /dev/null +++ b/scripts/following-in-the-console.js @@ -0,0 +1,201 @@ +/* + * Following, reported from the reader's own browser. + * + * Paste this into DevTools on a pull request's Files page, with the extension + * running. It presses names the way a reader does and says what happened. + * + * It exists because the probes cannot get here. A pull request is drawn from + * routes that answer to a session, so a fresh profile gets the signed-out card — + * measured on a public pull request, not assumed — and the machine this was + * written on has no display to sign one in with. The browser already looking at + * the page has both. + * + * The first run of this found that a press *does* open the panel on a diff, for + * a name written in the file being read — which is the opposite of what was + * reported. So this one watches what happens next: the panel closes on a press + * outside it and on any scroll that did not start inside it, and that second + * rule is listening in the capture phase on `window`, where it hears a scroll + * from anywhere on the page. A panel that opens and is taken away a moment + * later is, to a reader, a click that did nothing. + * + * Edit NAMES, paste, read. Nothing is sent anywhere; it prints to the console. + */ +;(async () => { + /** The names to press, as a reader would, and the line each is on. */ + const NAMES = [ + { word: "readBodyWithinLimit", line: 12 }, + { word: "BodyReadFailure", line: 5 } + ] + + /** How long to watch the panel after it opens, for it to be taken away. */ + const WATCH = 6000 + + const sleep = (ms) => new Promise((go) => setTimeout(go, ms)) + const panelNow = () => document.querySelector('[aria-label^="Uses of "]') + + const panes = () => { + const found = [] + const walk = (node) => { + for (const el of node.querySelectorAll("*")) { + if (!el.shadowRoot) continue + if (el.tagName.toLowerCase() === "diffs-container") found.push(el.shadowRoot) + else walk(el.shadowRoot) + } + } + walk(document) + return found + } + + const drawn = panes() + if (drawn.length === 0) { + console.log("%cno pane drew at all", "color:#f85149") + return + } + + /* + * Everything that could take the panel away, in the order it happens. + * + * The same two events the panel itself listens for, on the same targets and + * in the same phase, so what is logged here is what it heard. + */ + const heard = [] + const note = (what) => (event) => { + const target = event.target + const named = + target === document ? "document" + : target === window ? "window" + : target && target.nodeType === 1 + ? target.tagName.toLowerCase() + (target.id ? "#" + target.id : "") + + (typeof target.className === "string" && target.className ? "." + target.className.split(" ")[0] : "") + : String(target) + heard.push({ at: Math.round(performance.now()), what, from: named }) + } + const onScroll = note("scroll") + const onDown = note("pointerdown") + window.addEventListener("scroll", onScroll, true) + document.addEventListener("pointerdown", onDown, true) + + const edge = "[^A-Za-z0-9_$]" + + const press = async ({ word, line }) => { + const out = { name: `${word} @ L${line}` } + + let row = null, token = null, inside = -1 + for (const root of drawn) { + for (const one of root.querySelectorAll(`[data-line="${line}"]`)) { + for (const span of one.querySelectorAll("span")) { + if (span.querySelector("span")) continue + const text = span.textContent || "" + const at = text.search(new RegExp(`(^|${edge})${word}(${edge}|$)`)) + if (at === -1) continue + row = one; token = span; inside = text.indexOf(word, at) + break + } + if (token) break + } + if (token) break + } + if (!token) { + out.result = "the renderer drew no token holding that word on that line" + return out + } + + row.scrollIntoView({ block: "center", behavior: "instant" }) + await sleep(600) + + const box = token.getBoundingClientRect() + const text = token.textContent || "" + const wide = box.width / Math.max(1, text.length) + const where = { + bubbles: true, composed: true, cancelable: true, + clientX: box.left + (inside + word.length / 2) * wide, + clientY: box.top + box.height / 2, + metaKey: true, ctrlKey: true, button: 0, buttons: 1, + pointerId: 1, isPrimary: true, pointerType: "mouse" + } + + document.dispatchEvent(new KeyboardEvent("keydown", { key: "Meta", bubbles: true })) + token.dispatchEvent(new PointerEvent("pointerover", where)) + token.dispatchEvent(new PointerEvent("pointermove", where)) + + let underlined = false + for (let waited = 0; waited < 15000; waited += 50) { + const marked = (el) => (el.style.textDecoration || "") !== "" + if (marked(token) || [...token.querySelectorAll("span")].some(marked)) { underlined = true; break } + await sleep(50) + } + out.underlined = underlined + if (!underlined) { + out.result = "held the key and it never underlined" + return out + } + + /* + * The whole sequence a real mouse sends, not the three events a script + * usually bothers with. `mousedown` is the one a script leaves out and a + * hand never does, and it is what the drawing underneath acts on — a line + * marked, a selection begun, and whatever scrolling either causes. + */ + heard.length = 0 + const started = Math.round(performance.now()) + token.dispatchEvent(new PointerEvent("pointerdown", where)) + token.dispatchEvent(new MouseEvent("mousedown", where)) + token.dispatchEvent(new PointerEvent("pointerup", { ...where, buttons: 0 })) + token.dispatchEvent(new MouseEvent("mouseup", { ...where, buttons: 0 })) + token.dispatchEvent(new MouseEvent("click", { ...where, buttons: 0 })) + + let opened = null + for (let waited = 0; waited < 8000; waited += 50) { + const found = panelNow() + if (found) { opened = Math.round(performance.now()) - started; break } + await sleep(50) + } + if (opened === null) { + out.result = "no panel ever appeared" + out.heard = heard.slice(0, 6) + return out + } + + // It opened. Does it survive being looked at? + let gone = null + for (let waited = 0; waited < WATCH; waited += 50) { + if (!panelNow()) { gone = Math.round(performance.now()) - started; break } + await sleep(50) + } + + out.result = gone === null + ? `opened after ${opened}ms and stayed for ${WATCH}ms` + : `opened after ${opened}ms and was GONE by ${gone}ms` + out.heard = heard + .filter((one) => one.at - started >= 0 && (gone === null || one.at - started <= gone + 100)) + .map((one) => `${one.at - started}ms ${one.what} from ${one.from}`) + .slice(0, 10) + + document.dispatchEvent(new KeyboardEvent("keyup", { key: "Meta", bubbles: true })) + document.body.dispatchEvent( + new PointerEvent("pointerdown", { bubbles: true, composed: true, pointerId: 1, isPrimary: true, pointerType: "mouse" }) + ) + await sleep(400) + return out + } + + try { + const seen = [] + for (const one of NAMES) seen.push(await press(one)) + for (const one of seen) { + console.log(`%c${one.name}`, "font-weight:bold") + console.log(` underlined: ${one.underlined}`) + console.log(` result: ${one.result}`) + if (one.heard && one.heard.length > 0) { + console.log(` heard while it was up:`) + for (const line of one.heard) console.log(` ${line}`) + } else if (one.heard) { + console.log(` heard while it was up: nothing`) + } + } + console.log(seen) + } finally { + window.removeEventListener("scroll", onScroll, true) + document.removeEventListener("pointerdown", onDown, true) + } +})() diff --git a/src/entrypoints/offscreen/ledger.ts b/src/entrypoints/offscreen/ledger.ts index b5e331c9..a7ae9c51 100644 --- a/src/entrypoints/offscreen/ledger.ts +++ b/src/entrypoints/offscreen/ledger.ts @@ -19,6 +19,7 @@ import { withinPackage, type Held } from "@/ledger/packages" +import { within } from "@/ledger/reaching" import { heldIn, holdIn, holdingOf } from "@/ledger/holding" import { idbStore, noStore, type Store } from "@/ledger/store" import { usesAcross, type Asked } from "@/ledger/uses" @@ -604,8 +605,24 @@ const beyond = (work: LedgerBeyondWork): Effect.Effect => // Its own, which is most monorepos and costs no request at all. const own = ledger?.packages.get(packageOf(work.specifier)) if (own !== undefined) { - const within = withinPackage(work.specifier) - const path = within === null ? own.entry : `${own.at === "" ? "" : `${own.at}/`}${within}` + const deeper = withinPackage(work.specifier) + const named = + deeper === null ? own.entry : `${own.at === "" ? "" : `${own.at}/`}${deeper}` + /* + * And checked against the files the repository really has. + * + * A deep import names a path inside the package and not a file: + * `@org/type-utils/result-monad` is `packages/type-utils/result-monad`, + * with no ending on it and nothing of that name on disk. It was handed on + * as written, matched no file, and the answer fell through to the first + * Writing of that name anywhere in the repository — a different thing + * with the same spelling, offered to the reader as the place it is + * written. Which is the one mistake this whole feature exists to prevent. + */ + const path = + named === null || ledger === undefined + ? named + : within(named, new Set(ledger.files.keys())) ?? named if (path !== null) { const found = ledger === undefined ? [] : placesFor(ledger, work.name) const here = found.find((one) => one.path === path) ?? found[0] diff --git a/src/ledger/reaching.test.ts b/src/ledger/reaching.test.ts index e2d3e25e..14719a9e 100644 --- a/src/ledger/reaching.test.ts +++ b/src/ledger/reaching.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { couldBe, reaching } from "./reaching" +import { couldBe, reaching, within } from "./reaching" /** A repository laid out the way most of them are. */ const PATHS = new Set([ @@ -54,3 +54,52 @@ describe("which file a specifier names", () => { expect(reaching("src/two.ts", "./one", both)).toBe("src/one.ts") }) }) + +/** + * A path arrived at through a package's own folder, rather than through a + * specifier relative to a file. + * + * `@org/type-utils` held at `packages/type-utils` and imported as + * `@org/type-utils/result-monad` means `packages/type-utils/result-monad` — a + * path with no ending on it and nothing of that name on disk. It was handed on + * as written, so it matched no file, and the answer fell through to the first + * Writing of that name anywhere in the repository. A reader pressing + * `AsyncResult` was shown a different thing with the same spelling and told it + * was the place. + */ +describe("a path inside the repository", () => { + const paths = new Set([ + "packages/type-utils/result-monad.ts", + "packages/helpers/read-body-within-limit.ts", + "services/api/src/index.tsx", + "packages/deep/nested/index.ts" + ]) + + test("finds the file a folder and a name really mean", () => { + expect(within("packages/type-utils/result-monad", paths)).toBe( + "packages/type-utils/result-monad.ts" + ) + }) + + test("finds a folder's own index, which is how a package names itself", () => { + expect(within("packages/deep/nested", paths)).toBe("packages/deep/nested/index.ts") + }) + + test("takes a path that already names its ending as written", () => { + expect(within("services/api/src/index.tsx", paths)).toBe("services/api/src/index.tsx") + }) + + test("answers nothing where the repository holds no such file", () => { + // A path built from a package's layout is a claim about the repository, and + // it is checked like every other claim here rather than believed. + expect(within("packages/type-utils/nowhere", paths)).toBeNull() + expect(within("", paths)).toBeNull() + }) + + test("says `a/b/../c` plainly, and refuses a climb out of the repository", () => { + expect(within("packages/helpers/../type-utils/result-monad", paths)).toBe( + "packages/type-utils/result-monad.ts" + ) + expect(within("../../etc/passwd", paths)).toBeNull() + }) +}) diff --git a/src/ledger/reaching.ts b/src/ledger/reaching.ts index fdb1a80b..14956fb3 100644 --- a/src/ledger/reaching.ts +++ b/src/ledger/reaching.ts @@ -67,11 +67,29 @@ export const couldBe = (from: string, specifier: string): ReadonlyArray const folder = from.slice(0, Math.max(0, from.lastIndexOf("/"))) const asked = plainly(`${folder}/${specifier}`) - if (asked === null || asked === "") return [] + if (asked === null) return [] - // A specifier that already names its ending is taken as written. TypeScript's - // own `.js`-means-`.ts` rule is the one exception worth keeping, because - // every ES-module TypeScript repository is written that way. + return endingsFor(asked) +} + +/** + * The files a path inside the repository could be, before anything is checked. + * + * Split out from {@link couldBe} because a relative specifier is not the only + * way to arrive at a path with no ending on it. A package this repository holds + * itself resolves to a folder and a path inside it — `@org/type-utils` at + * `packages/type-utils`, imported as `@org/type-utils/result-monad`, means + * `packages/type-utils/result-monad` and whatever that file is really called. + * That path was being handed on with no ending at all, so it matched no file in + * the repository and the answer fell through to the first Writing of that name + * anywhere — a different thing with the same spelling, offered as the place. + */ +export const endingsFor = (asked: string): ReadonlyArray => { + if (asked === "") return [] + + // A path that already names its ending is taken as written. TypeScript's own + // `.js`-means-`.ts` rule is the one exception worth keeping, because every + // ES-module TypeScript repository is written that way. if (asked.endsWith(".ts") || asked.endsWith(".tsx")) return [asked] if (asked.endsWith(".js")) { return [`${asked.slice(0, -3)}.ts`, `${asked.slice(0, -3)}.tsx`, asked] @@ -92,3 +110,15 @@ export const reaching = ( specifier: string, paths: ReadonlySet ): string | null => couldBe(from, specifier).find((path) => paths.has(path)) ?? null + +/** + * The file a path inside the repository names, out of the paths that exist. + * + * {@link reaching} for a path that is already a path — arrived at through a + * package's own folder rather than through a specifier relative to a file. + * Nothing where the repository holds no such file, so a path built from a + * package's layout is checked before it is believed, like every other answer + * here. + */ +export const within = (path: string, paths: ReadonlySet): string | null => + endingsFor(plainly(path) ?? "").find((one) => paths.has(one)) ?? null