fix: dialogs rendered behind their own scrim; CDP scripts killed the user's Chrome - #244
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Scaffold Handoff — For Your Coding Agent
Immutability CheckMode: advisory ✅ No immutable files were modified in this branch. Changed Files
Needs Adversarial Review — Do NOT Apply YetThese proposals passed the reject check but have not been approved by
Safety Boundary
Immutable files (never modify):
Scaffold files (safe to modify):
|
Radix portals dialog overlay and content as SIBLINGS. The legacy scrims
(.r-modal-backdrop grid place-items:center, .r-room-modal-scrim) were written
to center a CHILD, and FocusTrapDialog passes `unstyled`, which drops the
Tailwind fixed/translate classes from DialogContent. Every dialog on this path
therefore rendered position:static at the end of <body>, BEHIND its z-95 blur
overlay — including the create-room dialog that asks the product's one
governance question ('How should NodeAgent edits land?'). Tailwind's sr-only
is also absent from this bundle, so the fallback DialogTitle rendered the word
'Dialog' visibly at the top of every modal.
Found by reviewing walkthrough footage frame by frame: DOM text extraction read
the dialog fine the whole time. Fix: center [data-slot=dialog-content] itself
above its scrim, and define .sr-only.
Also included, same session:
- boot.ts: the private-route boot shell had no failure exit — a rejected or
hung chunk import shimmered 'Opening room' forever. Rejection handler, 20s
timeout backstop, data-boot-state in the DOM, failed state stops the shimmer
and offers Reload. 13/13 scenario checks incl. reduced-motion.
- scripts/: trust-surface gate (core + selftest probed in BOTH directions +
live runner), motion inventory (painted-only, reduced-motion sweep), CDP
YouTube upload/retitle with ffprobe-derived titles, capture/verify tooling.
- design-dna/: 28 cited facts + 6 rules from a live Greptile inspection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ries chrome-cdp-up.ps1 is idempotent and encodes the three flags that each cost a debugging cycle: --user-data-dir must be explicit and UNQUOTED (a quoted path with a space fails the launch silently, and Chrome ignores the debugging port without it), and --disable-extensions is required because a real profile loads ~13 extension service workers as CDP targets that stall connectOverCDP. yt-upload gains the three narrated videos and derives every title duration from ffprobe rather than a typed constant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two independent defects, both of which made the debugging port look flaky when the scripts were the cause: 1. browser.close() on a connectOverCDP connection CLOSES THE REAL BROWSER. Playwright reads it as 'close the browser', not 'drop the socket'. Every script ended with it, so each run killed the Chrome the next run depended on — which is why the port 'kept dying' and got relaunched all session instead of diagnosed. Swept the class, not the instance: 9 pure-CDP scripts drop the call; motion-inventory.mjs connects OR falls back to launch(), so it now tracks weLaunchedIt and closes only a browser it owns. 2. chrome-cdp-up.ps1 ran Stop-Process -Force on every chrome process. A force kill gives Chrome no chance to write its session file, which is what produced the 'Chrome didn't shut down correctly / Restore pages?' banner and lost the user's open tabs. Now CloseMainWindow() (equivalent to clicking X), 15s grace, force only for a window that refuses, and --restore-last-session on relaunch. Also: yt-privatize.mjs — set superseded uploads to Private over CDP. Private, never delete: it hides a video from everyone including link-holders and is reversible. Guards by video ID against a keeper allowlist because two superseded clips share a title prefix with a keeper, so any title match would eventually hide the wrong one; the guard was tested in the failing direction first. README: the three NodeRoom clips as a roster with ffprobe-verified lengths, the fresh-user GIF inline, and an honest coverage note — 3 of 6 NodeRoom journeys, 6/21 elements, R5 declined by the owner rather than missed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
yt-verify.mjs hardcoded YUpSMEkkK4Q and q1CL1hCO_0Q — both superseded and set
Private earlier today. It also matched titles on 'review every agent change',
a phrase the superseded clip and its REPLACEMENT both carry, so it could have
passed against the wrong video and reported health. yt-save-probe.mjs
navigated to the same dead id.
Root cause is not the stale ids, it is that the roster existed twice: once in
the verifier and once in the privatize guard, free to disagree. Introduced
scripts/yt-roster.mjs as the single source both import, with strings
that DISTINGUISH rather than merely match (both narrated titles contain 'the
full walkthrough, narrated', so each carries its product), and an import-time
throw if an id ever appears in both published and superseded.
Rewrote yt-verify.mjs around two changes:
- no browser. It used connectOverCDP, so a verifier could only run when
Chrome was up with a debugging port, and before today's fix could take the
user's browser down with it. oembed answers 'is this public, and what is
its title' over plain HTTP with no session.
- it checks BOTH directions. Confirming the good ids resolve proves nothing
about the ones that should be gone; the superseded half is what would
catch a privatize that silently did nothing.
Verified: 6 published PASS (HTTP 200, titles match), 4 superseded PASS (HTTP
403). Guard re-tested in the failing direction against the imported list.
Reported by a subagent reviewing files outside its assigned scope.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HomenShum
force-pushed
the
claude/fresh-user-dialog-fix
branch
from
July 29, 2026 02:52
cdd374f to
64714f5
Compare
skills/motion-proof previously lived only in ~/.claude, which is not version controlled — so the one artifact this session built from scratch could not be handed to anyone, backed up, or reviewed. It now sits beside skills/liveflow, skills/probe-first and the rest, which was already the convention. It ships the primary instrument that was missing: an audit found the skill INVERTED in practice — SKILL.md correctly names Element.getAnimations() primary and the video judge advisory, but getAnimations() had zero executable callers while six Gemini video-judge scripts ran. motion-probe.mjs is that instrument, with seven adversarial fixtures and an honest control. The control must pass and every deception must be caught; the first run found two false positives in the probe itself (opacity is not a paint test, because enter animations start at opacity 0; and transform:none equals the identity matrix). Also resolves an unresolved stash conflict in .qa/memory/findings.jsonl. Both sides were distinct valid records in an append-only log, so the union was kept — 29 records, all parsing — rather than picking a winner.
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.
Three defects found while filming product walkthroughs, each invisible to the checks that were supposed to catch it.
1. Every
FocusTrapDialogmodal rendered behind its own blur scrimRadix portals dialog overlay and content as siblings. The legacy scrims (
.r-modal-backdrop,.r-room-modal-scrim) were written to center a child, andFocusTrapDialogpassesunstyled, which drops the Tailwind fixed/translate classes. Every dialog on this path therefore renderedposition: staticat the end of<body>, under its own z-95 blur — including the create-room dialog that asks the product's one governance question, "How should NodeAgent edits land?"Tailwind's
sr-onlyis also absent from this bundle, so the fallbackDialogTitlerendered the word "Dialog" visibly atop every modal.DOM text extraction read these dialogs correctly the entire time. Only looking at rendered frames caught it.
2. CDP scripts were force-killing the real browser
browser.close()on aconnectOverCDPconnection closes the actual Chrome, not the socket. Every script ended with it, so each run killed the browser the next run needed — the port looked flaky for hours. Swept the class: 9 pure-CDP scripts drop the call;motion-inventory.mjsconnects or falls back tolaunch(), so it now tracksweLaunchedItand closes only what it owns.chrome-cdp-up.ps1ranStop-Process -Force, giving Chrome no chance to write its session file — the "Chrome didn't shut down correctly" banner and lost tabs. Now gracefulCloseMainWindow(), 15s grace, force only as last resort.3. The verifier asserted videos that no longer exist
yt-verify.mjshardcoded two ids that are now Private, and matched titles on a phrase the superseded clip and its replacement both carry — so it could pass against the wrong video and report health. Root cause: the roster existed twice, free to disagree. Now oneyt-roster.mjsboth the verifier and the privatize guard import, with distinguishingexpectstrings and an import-time throw if an id appears in both lists.The verifier now checks both directions — 6 published resolve (HTTP 200, titles match), 4 superseded refused (403). The second half is what catches a cleanup that silently did nothing.
Also included
boot.ts: the private-route boot shell had no failure exit — a rejected chunk import shimmered "Opening room" forever. Rejection handler, 20s timeout,data-boot-statein the DOM, failed state stops the shimmer and offers Reload.ffprobe-verified lengths, the fresh-user GIF inline, and coverage stated honestly — 3 of 6 NodeRoom journeys, 6/21 elements, R5 declined by the owner rather than missed.🤖 Generated with Claude Code