fix(ui): deep-QA batch — transcript data loss, notification reachability, dock upgrade trap, chat-first sidebar, credential-preserving saves - #10994
Merged
Conversation
…ity, dock upgrade trap, chat-first sidebar, credential-preserving saves Confirmed by an adversarially-verified QA sweep of the chat/launcher/home surfaces on develop (20 confirmed findings; this lands the highest-value, lowest-risk nine): - shell-state: keep attachment-only and secret-request-only turns in the transcript window — an image-only send (or a generated-image reply) used to silently vanish when the turn settled (CRITICAL: perceived data loss). New unit tests cover both roles, every phase, and the empty-array case. - HomeScreen pull-zone: stop double-counting safe-area-top (the strip deadened ~70px of home content on notched iPhones — taps dead, scroll refused); the strip now spans only the residual tucked band + 30px. It is also a real button now: click/tap and Enter/Space open the notification center, so desktop fine-pointer and keyboard/AT users have a path to notifications (was pointer-drag-only, home-only). - launcher-layout: re-seed the default dock for pre-#10800 payloads (favorites [] without intent marker) — those users were locked out of the dock forever with no in-product recovery. An explicitly committed empty dock now stamps dockCleared and stays respected. Corrupt payloads fall back to the seeded default. Migration + marker regression tests added. - ConversationsSidebar: 'New chat' clears the active terminal session (the fresh conversation was created but never shown); the collapsed-rail '+' is now chat-first (it always spawned a TERMINAL via an unreachable-branch bug; New terminal stays in the expanded terminal section). - conversation guards: stamp scope 'general' at creation and reject reserved legacy page-chat titles on rename — renaming a chat to 'wallet'/'settings'/… made it vanish from every list with no recovery. - plugin/connector config saves: handlePluginConfigSave now reports success; ConnectorsSection + PluginsView keep the user's typed draft when a save fails (pasted tokens/keys were silently wiped on network errors). - e2e harness de-flake (verified root causes, not product bugs): chat-sheet keyboard block uses structural flicks instead of a knife's-edge velocity race; launcher fixture overflows the seeded dock (27 specs) with a loud bundle-time guard, restoring the Page-2/page-swipe assertions. Focused suites green: shell-state (13), launcher-layout (35 incl. property), HomeScreen (7), conversations (21), settings+plugins state (202); run-launcher-e2e 15/15; run-chat-sheet-e2e PASSED (51 screenshots). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…, reachable background redo Three more adversarially-confirmed QA findings: - NotificationCenter row dismiss (X) was opacity-0 with only a hover reveal — invisible on touch (the pull-down sheet is the primary mobile surface) yet still hit-testable, so near-edge taps silently deleted notifications; keyboard focus landed on an invisible button. Now dimmed visible at rest, full opacity on hover and focus-visible. - The home Notifications tile fed producer/LLM-influenceable deepLink strings straight into the navigate:view rail, bypassing the scheme-checked navigateDeepLink single source of truth: https deep links were dead taps (cross-origin pushState throw swallowed), unvetted schemes were forwarded, and the tile never marked the notification read (surfaces diverged). Now mirrors NotificationRow.handleOpen exactly. - Background redo (#10771) was unreachable by any user path: the UI channel handles op 'redo' but the BACKGROUND action's op union/resolver/schema had no redo and no UI control existed. Added the redo op end-to-end (regex intent, explicit op, handler broadcast, similes/examples/schema) and a Redo2 button next to Undo gated on canRedoBackground. 4 new tests; plugin suite 21/21, UI suites 15/15. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
… opens visible Two adversarially-confirmed z-order findings: - The chat's document-level outside-tap swallower ate the FIRST tap on any surface painted above the chat glass (notification pull-down sheet at z-9501, tutorial card at Z_TUTORIAL, any Radix dialog) and collapsed the chat under it. The swallower now cedes taps whose target sits inside [data-above-shell-overlay] or [role=dialog]; the notification sheet, its backdrop, and the tutorial spotlight are tagged. - /commands opened the CommandPalette dialog (Z_DIALOG=170) BEHIND the open chat sheet (Z_SHELL_OVERLAY=9000) — an invisible focus-trapped dialog; Escape then closed the palette AND collapsed the chat at once. The composer's palette route now collapses the sheet first so the palette opens visible over the pill (raising all dialogs above the shell overlay was rejected: tooltips at Z_TOOLTIP=300 inside dialogs would stack under them), and the chat's Escape handler defers to an open dialog (scoped to [data-state=open] dialogs + the notification sheet — broad role=dialog would match always-mounted shell surfaces and permanently disable Escape-collapse). Regression tests: taps inside a role=dialog layer no longer collapse the sheet; Escape with an open dialog leaves the chat open and collapses it once the dialog is gone. 4/4 targeted + existing collapse tests green; run-chatux-gesture-e2e ALL PASSED post-change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…on on the chat action rail Two adversarially-confirmed QA findings: - A blocked/errored coding-agent session permanently hijacked the chat surface: ChatView auto-focused any error/blocked PTY session whenever activeTerminalSessionId was null, and 'blocked' is a routine long-lived waiting-for-input state — closing the panel or selecting a conversation bounced straight back to the terminal, locking the desktop chat window out of every conversation. Auto-focus now fires at most once per transition into a problem state (pickProblemSessionToAutoFocus + a handled-set ref; viewing counts as handled; recovery evicts so a new transition focuses again). 7 new tests reproduce the loop through React's real effect scheduling. - Flick-scrolling the ChatView transcript on touch toggled the action rail (copy/edit/play/delete) on whichever message the finger started on: handleTapReveal had no move-slop or selection guard, unlike the shell ThreadLine. Touchstart position is now recorded and >10px travel (either axis) or a non-collapsed selection suppresses the toggle, mirroring ThreadLine's COPY_MOVE_CANCEL_PX pattern. 3 new tests (clean tap toggles; vertical/horizontal scrolls don't; selection suppresses). Focused suites: 17 files / 85 tests green; packages/ui typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…redential form for mode-less connectors
Two adversarially-confirmed QA findings:
- BackgroundSettingsSection was orphaned dead code: the only surface with
BackgroundSettingsControls is the 'background' view (viewKind: preview —
hidden on every default build) and the section was never registered, so a
default user could not change, undo, or preview the wallpaper from any
visible UI (chat-only via the BACKGROUND action). Registered as a real
system-group Settings section (Wallpaper icon, en label; other locales
fall back to the default label).
- Settings → Connectors dead-ended every connector without a hardcoded
mode list (farcaster, bluesky, matrix, nostr, msteams, mattermost,
twitch, instagram, …) with the factually-wrong '{name} uses its own
setup surface.' — no link, no form, even with declared parameters.
Mirrors the canonical /connectors fallback: no declared modes + declared
parameters → the credential form IS the setup surface
(setupPluginId ?? plugin.id). local-setup/plugin-managed/cloud-managed
modes keep their dedicated panels (regression suite still green).
Settings sweep: 18 files / 208 tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…ork=0) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
25 tasks
…inter paging control at any width Two adversarially-confirmed QA findings (one root cause): - The home↔launcher rail and the launcher grid pager both attach pointer handlers on the same bubble path: one horizontal drag was tracked — and painted — by BOTH pagers (touch: ~1.35x finger motion, background gap at the screen edge, spring-back jerk on release), and for mouse/pen the outer handler's later setPointerCapture stole the pointer mid-drag, freezing the inner pager (dragRef never saw pointerup, ResizeObserver resync skipped) — mouse drag-paging on launcher page 0 could never reach the Developer page. Violated the shell-surface store invariant that a swipe claimed by two machines must be structurally impossible. useHorizontalPager now runs a shared pointer-claim registry: trackers register in bubble order (innermost first); the first pager that commits a horizontal axis AND canMove in the drag direction claims the pointer exclusively and synchronously EVICTS the rest (push, not poll — losers may never see another event once the winner holds capture). Eviction abandons the drag immediately and settles to the resting page. An unowned dead-end drag rubber-bands on the innermost pager only. finish() claims before advancing so two pagers can never both commit one pointerup. Also fixes a latent scheduleOffset bug where a synchronous rAF left the frame permanently pending, swallowing later drag paints. - PagerEdgeButtons dropped its min-width:1024px gate (hover+fine-pointer kept): with page dots off in production, a fine-pointer window under 1024px had NO control that reached launcher page 2. Also updates the springboard-migration component tests to the #10800-QA dock re-seed semantics (dock-less migrated layouts dock chat+settings; manual page order still preserved for non-dock ids) — the old expectation was invalidated by the launcher-layout upgrade-path fix in this PR. Tests: 4 new nested-harness arbitration tests + 3 composed tests on the real bubbling path (tile-origin drags) + real-browser regression in run-home-screen-e2e (mouse drags page the inner grid both directions; the rail never hijacks) — HOME-SCREEN and LAUNCHER e2e PASSED; focused suites 88/88; packages/ui typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Resolve 3 conflicts: - plugins/plugin-app-control/src/actions/background.ts + .test.ts: take develop's #10960 BACKGROUND redo action (more thorough: re-apply|go forward + RESET_RE exclusion). PR's unique settings-UI redo button in BackgroundSettingsControls.tsx (non-conflicting) preserved. - packages/ui/src/components/shell/__e2e__/run-chat-sheet-e2e.mjs: take develop's #10972 real-touch comment; gesture code identical on both sides and already drives touchDragHold real-touch path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in set #10434 (99cab28) removed the `companion` view from the builtin navigation set (BuiltinTab / TAB_PATHS) — and `voice` is likewise no longer a builtin tab — but the view-lifecycle matrix test still classified both, so "covers exactly the builtin tab set" failed on develop tip (classified 36 vs TAB_PATHS 34). TAB_PATHS/BuiltinTab is the authoritative builtin set and is correct; the test was stale. Drop the two stale entries; assertion stays an exact deep-equal against TAB_PATHS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Member
Author
Synced onto
|
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
lalalune
pushed a commit
that referenced
this pull request
Jul 3, 2026
…r gate, 5 coverage-ratchet drifts, deterministic #11030 deadlock guard packages/ui no-backdrop-blur-gate (#9141 battery): #11829's new NotificationCenter reintroduced backdrop-blur-2xl/backdrop-saturate-150 on the sheet/panel shell. Drop the GPU backdrop filter and carry readability with a near-opaque base (bg-neutral-950/[0.87]) instead; refresh the committed home-screen e2e artifacts (sheet + panel verified readable over the live launcher in headless chromium, 10 screenshots + walkthrough video). packages/app coverage/ratchet gates (5 reds, all real drift, no baselines loosened): - core-view-action-surface + view-switching matrices: track the automations core view (added by b07f3d9 without a surface owner) and the background settings subsection (#10994 registered it in SETTINGS_SECTION_META only) — AutomationsFeed + BackgroundSettings* are real agent-surface owners. - route-coverage: plugin-birdclaw (#11385) declares a production collapsed gui/xr/tui view that was never added to the manifest ratchet. Wire it fully: manifest list, XR ratchet, runtime-plugin boot classification, gui+tui visual-matrix cases, tracked visual-review rows, manager-visible tile case, HMR lockstep probe, GUI interaction owners (its real plugin.test.ts + BirdclawView.test.tsx), and the ui-smoke stub (view registration + honest zero-key /api/birdclaw/status "not installed" state). Both birdclaw Playwright visual cases pass live (real BirdclawView bundle renders the setup card; audit JSON + screenshots reviewed). - ui-smoke-coverage: scenario-pr.yml still hand-named the 10 pre-#11442 voice-workbench spec filenames; point the slice at the renamed specs. - view-interaction-coverage: the launcher owner declaration still claimed edit-mode/drag-to-reorder coverage that #11523's read-only launcher removed; declare what run-launcher-e2e.mjs actually proves now (no-edit long-press, tap-launch telemetry). ios-local-agent-transport #11030 deadlock guard (both ui + app-core copies, ~1-in-4 flaky under a loaded suite): the 10s wall-clock Promise.race lost to a CPU-starved event loop. Replace it with a deterministic detector — promise assimilation invoking the hostile proxy's fabricated `then` proves the raw Capacitor proxy crossed an await (the exact #11030 regression) and rejects instantly with the descriptive error. Verified: reintroducing the raw-proxy bug fails the test immediately with the #11030 message; fixed transport never touches `then` (asserted). Suites: packages/ui 544 files / 5532 pass; packages/app 37 files / 316 pass; app-core transport suite 30 pass; typecheck green (ui, app, app-core); biome clean on touched files.
lalalune
pushed a commit
that referenced
this pull request
Jul 3, 2026
…r gate, 5 coverage-ratchet drifts, deterministic #11030 deadlock guard (#11898) packages/ui no-backdrop-blur-gate (#9141 battery): #11829's new NotificationCenter reintroduced backdrop-blur-2xl/backdrop-saturate-150 on the sheet/panel shell. Drop the GPU backdrop filter and carry readability with a near-opaque base (bg-neutral-950/[0.87]) instead; refresh the committed home-screen e2e artifacts (sheet + panel verified readable over the live launcher in headless chromium, 10 screenshots + walkthrough video). packages/app coverage/ratchet gates (5 reds, all real drift, no baselines loosened): - core-view-action-surface + view-switching matrices: track the automations core view (added by b07f3d9 without a surface owner) and the background settings subsection (#10994 registered it in SETTINGS_SECTION_META only) — AutomationsFeed + BackgroundSettings* are real agent-surface owners. - route-coverage: plugin-birdclaw (#11385) declares a production collapsed gui/xr/tui view that was never added to the manifest ratchet. Wire it fully: manifest list, XR ratchet, runtime-plugin boot classification, gui+tui visual-matrix cases, tracked visual-review rows, manager-visible tile case, HMR lockstep probe, GUI interaction owners (its real plugin.test.ts + BirdclawView.test.tsx), and the ui-smoke stub (view registration + honest zero-key /api/birdclaw/status "not installed" state). Both birdclaw Playwright visual cases pass live (real BirdclawView bundle renders the setup card; audit JSON + screenshots reviewed). - ui-smoke-coverage: scenario-pr.yml still hand-named the 10 pre-#11442 voice-workbench spec filenames; point the slice at the renamed specs. - view-interaction-coverage: the launcher owner declaration still claimed edit-mode/drag-to-reorder coverage that #11523's read-only launcher removed; declare what run-launcher-e2e.mjs actually proves now (no-edit long-press, tap-launch telemetry). ios-local-agent-transport #11030 deadlock guard (both ui + app-core copies, ~1-in-4 flaky under a loaded suite): the 10s wall-clock Promise.race lost to a CPU-starved event loop. Replace it with a deterministic detector — promise assimilation invoking the hostile proxy's fabricated `then` proves the raw Capacitor proxy crossed an await (the exact #11030 regression) and rejects instantly with the descriptive error. Verified: reintroducing the raw-proxy bug fails the test immediately with the #11030 message; fixed transport never touches `then` (asserted). Suites: packages/ui 544 files / 5532 pass; packages/app 37 files / 316 pass; app-core transport suite 30 pass; typecheck green (ui, app, app-core); biome clean on touched files.
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.
Summary
Deep adversarial QA sweep of the chat / launcher / home / notification / state surfaces on
develop(6 parallel deep reviewers + per-finding refutation agents → 20 confirmed findings, plus the fullpackages/uivitest battery and every boot-free real-Chromium e2e runner). This PR lands the highest-value, lowest-risk nine fixes; the remaining confirmed findings are being posted to their issues with file:line detail.Product fixes
selectVisibleShellMessagesdropped attachment-only turns (user image-only sends AND settled generated-image replies). Also covered secret-request-only turns (same class).shell-state.ts+ 5 new unit tests.HomeScreen.tsx) — the notification pull strip addedsafe-area-topon top of the shell root's already-reserved padding, deadening ~70 px of home content on notched iPhones (dead taps, refused scroll). Now spans only the residual tucked band + 30 px.launcher-layout.ts) — any pre-fix(ui): restore default launcher dock #10800 stored layout (favorites: [], the entire no-dock era) suppressed the dock forever with no in-product recovery. Unmarked empty docks now re-seed chat+settings; an explicitly committed empty dock stampsdockClearedand stays respected; corrupt payloads fall back to the seeded default. Migration tests added.ConversationsSidebar.tsx) — created the conversation server-side but kept rendering the terminal. Now clearsactiveTerminalSessionId, mirroring row selection.showNewChatAction/showNewTerminalActionwere the identical expression and the ternary preferred the terminal, so the New-chat branch was unreachable dead code. The collapsed "+" is now chat-first; New terminal stays in the expanded terminal section.scope: "general"at creation; renames to reserved titles are rejected with a notice.handlePluginConfigSaveswallowed errors and both callers cleared the draft unconditionally; sensitive fields never echo back, so the token was gone. Saves now report success and drafts survive failures (ConnectorsSection,PluginsView).Harness fixes (root-caused as stale harnesses, not product bugs)
run-chat-sheet-e2ekeyboard block: structural flicks instead of a knife's-edge 0.5 px/ms velocity race (its old pass was accidental timing; arithmetically impossible via slow drags).run-launcher-e2efixture: the fix(ui): restore default launcher dock #10800 seeded dock absorbed chat+settings, collapsing 25 tiles to one page and killing the Page-2/page-swipe assertions. Fixture now overflows the dock (27 specs) with a loud bundle-time guard.Evidence (all real, captured this run)
.github/issue-evidence/10722-uiux-deep-qa-sweep/— post-fix screenshots (desktop + mobile home, seeded dock, chat-sheet keyboard FULL) + 2 webm real-input walkthroughs (launcher swipe telemetry 0→2; home→launcher rail).packages/uivitest battery: 1908 tests green (initial 12 failures reproduced as machine-load timeouts only; 127/127 isolated).Refs #10722 #10706 #10800 #10231 (QA findings being posted per-issue).
🤖 Generated with Claude Code