Repository navigation
test(web): guard every upstream name the fork stylesheets borrow - #108
Conversation
github-actions
Bot
added
vouch:trusted
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
REQUEST CHANGES. The new guard recreates the failure mode it claims to catch, for every data attribute and slot the fork stylesheets select.
sourceCorpus walks all of apps/web/src for ts/tsx/css/html, excluding only tests and __fork_guards__/. That walk includes theme.custom.css, theme.custom.palettes.css, and custom/**/*.css — the same files collect() harvests references from. The attribute check then asks whether the attribute name appears as a token in that blob, and whether the value appears as a quoted literal. Both strings already live in the selector that produced the reference.
The attach-button rules in this PR are the next silent miss. Today ChatComposer.tsx still stamps data-chat-composer-actions="right", so the test is green for two reasons at once. After an upstream rename, the stylesheet still contains [data-chat-composer-actions="right"], sourceCorpus still matches, and CI stays green. Same class of bug as --topbar-scroll-fade-height in #107.
Split the corpora by role, not by ownership:
- References — already
forkCss. Keep. - Declarations (variables) — all CSS, including fork sheets. A borrowed
var(--old)with noname:declaration still fails. That path actually works; it is why the fade rename is a real catch. - Stamps (attributes, slots, classes) —
ts/tsx/htmlonly. Dropcssfrom thesourceCorpusfilter. Do not denylistFORK_STYLESHEETSout of a mixed blob. CSS selectors are not stamps.
Variable and class checks are fine (a leading . does not satisfy the class token regex; that is how .chat-composer-glass was found). The CSS presentation changes belong in these files and the sibling guards pin their outcomes. One structural fix, then this is the right guard.
Sent by Cursor Automation: Thermo nuke 4.6
| const sourceCorpus = (() => { | ||
| const files = walk(webSrc).filter( | ||
| (file) => | ||
| /\.(?:tsx?|css|html)$/u.test(file) && | ||
| !/\.test\.[cm]?tsx?$/u.test(file) && | ||
| !file.includes(`${NodePath.sep}__fork_guards__${NodePath.sep}`), | ||
| ); | ||
| files.push(NodePath.join(webRoot, "index.html")); | ||
| return files.map((file) => stripComments(NodeFS.readFileSync(file, "utf8"))).join("\n"); | ||
| })(); |
There was a problem hiding this comment.
This is a structural miss, not a coverage nit. css in this filter puts theme.custom.css, theme.custom.palettes.css, and custom/**/*.css into sourceCorpus. Those are the files collect() reads references from.
The attribute check later is a token / "value" search against that blob. [data-chat-composer-actions="right"] in this PR's own stylesheet satisfies both, so an upstream rename that removes the stamp from ChatComposer.tsx leaves CI green. That is the #107 failure mode this guard exists to catch.
Do not denylist the fork sheets. CSS selectors are not stamps. Drop css from this filter and keep declarationCorpus for var() declarations (including fork-owned --fork-*). Stamps are ts/tsx/html.
There was a problem hiding this comment.
Fixed in ac6aec9. sourceCorpus (stamps) is now .ts/.tsx plus index.html only; declarationCorpus keeps every stylesheet, fork sheets included, for var() targets. Probed both ways: renaming the composer-host slot value in ComposerSurface fails the guard with [data-slot="composer-host"] (theme.custom.css), and renaming the fork-timeline-cutoff class in MessagesTimeline fails it with .fork-timeline-cutoff (theme.custom.css). Also rebased onto the current #107 head, which now stamps the attach button by name.
NoahHendrickson
left a comment
NoahHendrickson
left a comment
There was a problem hiding this comment.
Blocking issue: sourceCorpus includes CSS, then the data-attribute test searches sourceCorpus for each referenced attribute name and quoted value. The selector being validated therefore supplies both strings itself. For example, after removing data-chat-composer-actions="right" from ChatComposer, theme.custom.css still contains [data-chat-composer-actions="right"], so the guard remains green—the same self-validating failure mode this PR is intended to eliminate. Split the corpora by role: keep CSS in declarationCorpus for variables, but build the attribute/slot stamp corpus from TS/TSX/HTML only.
Every fork stylesheet reaches into upstream by name — a CSS variable read with var(), a data attribute or data-slot value it selects, a class it targets — and none of those is a typed contract. An upstream rename compiles clean and simply un-matches the fork rule; an unresolved var() is worse, since it invalidates the whole declaration around it at computed-value time. That is how the timeline cutoff vanished when pingdotgg#8799 renamed --topbar-scroll-fade-height while the guard for it stayed green by pinning the fork's own text. forkUpstreamNames.test.ts resolves each referenced name against the web source (tests and comments excluded, so a stale name quoted in a comment cannot count as a declaration), Tailwind's default theme, and a short allowlist of names Base UI stamps at runtime. Verified it fails on the pingdotgg#8799 rename with the old name swapped back in. Its first run also found .chat-composer-glass in the Cool Darker glass block: upstream pingdotgg#8734 dropped that class for arbitrary utilities on ComposerSurface, and the composer's filters are already cleared by the composer-box rules, so the dead arm is removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review on #108: the stamp corpus walked CSS too, so every data attribute and slot value the fork stylesheets select was satisfied by the selector that produced the reference — the self-validating failure mode the guard exists to catch. Declarations (var() targets) still come from every stylesheet, fork sheets included, since a borrowed variable with no declaration anywhere is the real catch. Stamps come from .ts/.tsx and index.html only. Probed by renaming the composer-host slot value in ComposerSurface and the fork-timeline-cutoff class in MessagesTimeline: both now fail the guard naming the stale selector. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5a09bc4 to
ac6aec9
Compare
|
Addressed the blocking issue in ac6aec9: the corpora are split by role. Stamps (attributes, slots, classes) are resolved against TS/TSX/HTML only, so a selector can never satisfy its own check; variable declarations still come from every stylesheet. Verified the guard now fails when a slot value or class stamp is renamed in source. Rebased onto #107's latest head. |
2 participants
Problem
Every fork stylesheet reaches into upstream by name: a CSS variable it reads with
var(), adata-slotor data attribute it selects, a utility class it targets. None of those is a typed contract, so an upstream sync can rename one and nothing fails to compile. The fork rule simply stops matching, or worse, an unresolvedvar()invalidates the whole declaration around it at computed-value time.That is exactly how the timeline cutoff broke in #107: upstream pingdotgg#8799 renamed
--topbar-scroll-fade-height, the fork's mask rule kept reading the old name,mask-sizewent invalid, and the transcript painted straight through the composer. The guard for that customization stayed green because it pinned the fork's own text rather than the name upstream exports.Fix
A new guard,
forkUpstreamNames.test.ts, that resolves every name the fork stylesheets borrow against the web app's own source:var()must be declared in some stylesheet, as a Tailwind arbitrary property in a className, or stamped from TypeScript. Tailwind's default theme is included for--color-*.@utility, with Tailwind escaping undone.Tests and comments are excluded from the corpus, so a stale name that survives only in an assertion or a comment cannot count as a declaration. A short allowlist covers names Base UI stamps at runtime (positioner variables, the tooltip-trigger attribute), each with a note saying who sets it.
It asserts existence only, never meaning. A token whose value changed semantics still needs the guard that owns that customization.
Its first run also found
.chat-composer-glassin the Cool Darker glass block: upstream pingdotgg#8734 dropped that class for arbitrary utilities onComposerSurface, and the composer's filters are already cleared by the composer-box rules, so the dead arm is removed and the vibrancy manifest entry no longer lists it.Manifest entry
fork-upstream-namesadded, tier 1, watchingindex.css.Verification
theme.custom.cssand ran the guard: it fails naming--topbar-scroll-fade-height (theme.custom.css). Restored, it passes.vp test run src/__fork_guards__→ 46 files, 385 tests passing.vp run typecheckinapps/webclean.Stacked on #107: the fade fix there is what makes this guard pass on the current stylesheet. Merge #107 first, or this PR's guard will report the same rename against
custom, which is the correct answer.No runtime code changes beyond removing one dead selector arm. Web only.
Claude Fable 5.1 via Claude Code.
🤖 Generated with Claude Code