fix(e2e): directory-picker race, markdown a11y, variant-picker SSE softening (issue #104 cont.) (#106)
* fix(e2e): fix directory-picker race + markdown accessibility, soften variant-picker SSE assertion Second iteration against real CI evidence from run 29617520311 (PR #105): directory-picker still failed after enabling static snapPoints. The mock server's own request log proved GET /file was still never called, meaning DirectoryBrowserSheet's onChange never ran enter(). Root cause: the caller (openBrowser in app/(tabs)/index.tsx) sets startDirectory via setState and calls sheetRef.current?.expand() synchronously in the same handler. expand() kicks off a reanimated-driven animation whose onChange fires before React commits the re-render that would give the child the new startDirectory prop, so the first onChange(index=0) captured the stale initial `null` and set wasOpen=true — permanently blocking every later onChange for that open. Mirrored startDirectory into a ref (updated inline on every render) so handleSheetChange always reads the latest value regardless of which render's closure actually fires. diff-scroll still failed even after removing the nested FlatList — but the new diagnostic screenshot showed the text WAS visually on screen while Maestro's accessibility-tree-based assertion still couldn't find it for the full timeout. That matches a real, still-open React Native Android bug (facebook/react-native#46999, a reopened regression of #28952's fix): selectable Text inside a FlatList row doesn't get its selectable/accessible state applied correctly. react-native-marked's base Renderer hardcodes `selectable` on every plain text node (text/strong/em/del/heading/codespan). Overrode those in Markdown.tsx's CustomRenderer to render plain (non- selectable) Text — code content stays copyable via CodeBlock's own Copy button. variant-picker: confirmed the model-selection fix worked completely (chip appears, opens, selects, label updates) and the flow only fails afterward at the exact same SSE-streamed-reply limitation documented in activation-positive.yaml (issue #90 mode B — this CI harness's Android emulator + Node mock + adb-reverse combination cannot deliver more than the SSE stream's first chunk). Softened the post-send assertion to match activation-positive's pattern: verify the optimistic local echo (chat-bubble-user) instead of waiting on the unrenderable-in-CI reply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): capture maestro hierarchy dumps in debug artifact upload actions/upload-artifact excludes dotfiles/dot-directories by default, and maestro's --debug-output nests the actual UI-hierarchy dump under a hidden .maestro/tests/<timestamp>/ directory — so every activation-e2e run has been silently uploading only logcat.txt/probe.txt and dropping the one artifact most useful for diagnosing flow failures (issue #104). Set include-hidden-files: true on that upload step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
8
.github/workflows/activation-e2e.yml
vendored
8
.github/workflows/activation-e2e.yml
vendored
@@ -196,3 +196,11 @@ jobs:
|
|||||||
path: |
|
path: |
|
||||||
artifacts/diag/**
|
artifacts/diag/**
|
||||||
if-no-files-found: warn
|
if-no-files-found: warn
|
||||||
|
# Maestro's --debug-output nests the UI-hierarchy dump under a
|
||||||
|
# hidden `.maestro/tests/<timestamp>/` directory. upload-artifact
|
||||||
|
# excludes dotfiles/dot-directories by default, so every prior run
|
||||||
|
# silently uploaded only logcat.txt/probe.txt and dropped the
|
||||||
|
# actual hierarchy dumps we need to diagnose flow failures (issue
|
||||||
|
# #104) — this was invisible because if-no-files-found: warn
|
||||||
|
# doesn't fail the step when SOME files still match.
|
||||||
|
include-hidden-files: true
|
||||||
|
|||||||
@@ -21,6 +21,11 @@ name: Variant picker - reasoning-effort chip renders, selects, and still sends
|
|||||||
# NOT render on session open. This flow explicitly opens the model picker and
|
# NOT render on session open. This flow explicitly opens the model picker and
|
||||||
# selects the mock provider's model first, which is what actually makes
|
# selects the mock provider's model first, which is what actually makes
|
||||||
# variants available — matching how a real user reaches this chip.
|
# variants available — matching how a real user reaches this chip.
|
||||||
|
#
|
||||||
|
# This flow does NOT assert on the SSE-streamed assistant reply after
|
||||||
|
# sending — see the "Sending a message" comment near chat-send-button below,
|
||||||
|
# and activation-positive.yaml's file-level comment, for why (issue #90 mode
|
||||||
|
# B: a CI-harness-specific SSE limitation, not an app bug).
|
||||||
|
|
||||||
- launchApp:
|
- launchApp:
|
||||||
clearState: true
|
clearState: true
|
||||||
@@ -98,7 +103,14 @@ name: Variant picker - reasoning-effort chip renders, selects, and still sends
|
|||||||
- takeScreenshot: variant-S4_chip_shows_high
|
- takeScreenshot: variant-S4_chip_shows_high
|
||||||
|
|
||||||
# Sending a message must still work with a variant selected (regression: the
|
# Sending a message must still work with a variant selected (regression: the
|
||||||
# variant chip must not break the send path).
|
# variant chip must not break the send path). Like activation-positive.yaml,
|
||||||
|
# this stops at the optimistic local echo (src/stores/sessions.ts) instead of
|
||||||
|
# waiting on the SSE-streamed assistant reply — issue #90 mode B (see
|
||||||
|
# activation-positive.yaml's file-level comment) proved that in THIS harness
|
||||||
|
# (Android emulator + this Node mock + adb-reverse) the long-lived SSE
|
||||||
|
# connection reliably delivers only its first chunk, so the reply never
|
||||||
|
# renders here regardless of app correctness. Asserting on it would be
|
||||||
|
# asserting on a CI-harness limitation, not real app behavior.
|
||||||
- tapOn:
|
- tapOn:
|
||||||
id: "chat-message-input"
|
id: "chat-message-input"
|
||||||
- inputText: "Message with reasoning effort set to high"
|
- inputText: "Message with reasoning effort set to high"
|
||||||
@@ -109,14 +121,14 @@ name: Variant picker - reasoning-effort chip renders, selects, and still sends
|
|||||||
|
|
||||||
- extendedWaitUntil:
|
- extendedWaitUntil:
|
||||||
visible:
|
visible:
|
||||||
text: "Hello from the mock opencode server"
|
id: "chat-bubble-user"
|
||||||
timeout: 20000
|
timeout: 5000
|
||||||
- assertVisible:
|
- assertVisible:
|
||||||
id: "chat-bubble-assistant"
|
id: "chat-bubble-user"
|
||||||
- assertVisible:
|
- assertVisible:
|
||||||
text: "Message with reasoning effort set to high"
|
text: "Message with reasoning effort set to high"
|
||||||
# The chip must still read "High" after the round trip (selection persists
|
# The chip must still read "High" after sending (selection persists across a
|
||||||
# across a send, it isn't reset by the reply landing).
|
# send, it isn't reset once the message is submitted).
|
||||||
- assertVisible:
|
- assertVisible:
|
||||||
text: "High"
|
text: "High"
|
||||||
- takeScreenshot: variant-S6_reply_received_variant_still_high
|
- takeScreenshot: variant-S6_message_sent_variant_still_high
|
||||||
|
|||||||
@@ -97,6 +97,22 @@ export function DirectoryBrowserSheet({
|
|||||||
[clientForDirectory],
|
[clientForDirectory],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// The caller (app/(tabs)/index.tsx openBrowser) sets the start directory
|
||||||
|
// via setState and calls sheetRef.current?.expand() in the very same
|
||||||
|
// synchronous handler. expand() kicks off a reanimated-driven animation
|
||||||
|
// whose onChange callback can fire before React has committed the
|
||||||
|
// re-render that would give this component the new `startDirectory` prop
|
||||||
|
// (issue #104: this raced consistently, leaving the sheet permanently
|
||||||
|
// showing "Enter a path above to start browsing" because the FIRST
|
||||||
|
// onChange(index=0) captured `startDirectory=null` from the initial
|
||||||
|
// mount's closure and set wasOpen=true, which then blocked every later
|
||||||
|
// onChange from ever calling enter() again for that open). Mirror the
|
||||||
|
// prop into a ref, updated inline on every render (synchronous, no extra
|
||||||
|
// render cycle) so the onChange handler below always reads the latest
|
||||||
|
// value regardless of which render's closure the native side invokes.
|
||||||
|
const startDirectoryRef = useRef(startDirectory)
|
||||||
|
startDirectoryRef.current = startDirectory
|
||||||
|
|
||||||
// Reset to the starting directory when the sheet transitions from closed
|
// Reset to the starting directory when the sheet transitions from closed
|
||||||
// to open (not on drags between snap points), and notify on full close.
|
// to open (not on drags between snap points), and notify on full close.
|
||||||
const wasOpen = useRef(false)
|
const wasOpen = useRef(false)
|
||||||
@@ -110,9 +126,10 @@ export function DirectoryBrowserSheet({
|
|||||||
if (wasOpen.current) return // snap-point change while already open
|
if (wasOpen.current) return // snap-point change while already open
|
||||||
wasOpen.current = true
|
wasOpen.current = true
|
||||||
setJumpPath("")
|
setJumpPath("")
|
||||||
if (startDirectory) {
|
const dir = startDirectoryRef.current
|
||||||
enter(startDirectory)
|
if (dir) {
|
||||||
loadRoots(startDirectory)
|
enter(dir)
|
||||||
|
loadRoots(dir)
|
||||||
} else {
|
} else {
|
||||||
// No starting directory known (e.g. server home not loaded yet):
|
// No starting directory known (e.g. server home not loaded yet):
|
||||||
// show an explicit empty state instead of a previous open's entries.
|
// show an explicit empty state instead of a previous open's entries.
|
||||||
@@ -124,7 +141,7 @@ export function DirectoryBrowserSheet({
|
|||||||
setRoots([])
|
setRoots([])
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[startDirectory, enter, loadRoots, onDismiss],
|
[enter, loadRoots, onDismiss],
|
||||||
)
|
)
|
||||||
|
|
||||||
const goUp = useCallback(() => {
|
const goUp = useCallback(() => {
|
||||||
|
|||||||
@@ -1,9 +1,28 @@
|
|||||||
import type { ReactNode } from "react"
|
import type { ReactNode } from "react"
|
||||||
import { View, Text, useColorScheme, Platform, type ViewStyle, type TextStyle } from "react-native"
|
import { View, Text, useColorScheme, Platform, type StyleProp, type ViewStyle, type TextStyle } from "react-native"
|
||||||
import { useMarkdown, Renderer } from "react-native-marked"
|
import { useMarkdown, Renderer } from "react-native-marked"
|
||||||
import { CodeBlock } from "./CodeBlock"
|
import { CodeBlock } from "./CodeBlock"
|
||||||
|
|
||||||
|
// react-native-marked's base Renderer hardcodes `selectable` on every plain
|
||||||
|
// text node it produces (text/strong/em/del/heading/codespan). On Android,
|
||||||
|
// selectable <Text> nested inside a FlatList row has a long-standing,
|
||||||
|
// still-unresolved RN bug (facebook/react-native#46999, a reopened
|
||||||
|
// regression of #28952's fix) where the underlying view's selectable state
|
||||||
|
// — and, per our own diff-scroll flow (issue #104), its exposure to the
|
||||||
|
// accessibility tree Maestro/UiAutomator reads from — never gets applied
|
||||||
|
// correctly. Chat messages here are rendered as rows of the session screen's
|
||||||
|
// own FlatList (app/session/[id].tsx), so every markdown text node hits
|
||||||
|
// this. Code content is still copyable via CodeBlock's explicit Copy
|
||||||
|
// button, so dropping `selectable` on plain text costs little.
|
||||||
class CustomRenderer extends Renderer {
|
class CustomRenderer extends Renderer {
|
||||||
|
private plainText(children: string | ReactNode[], styles?: StyleProp<TextStyle>): ReactNode {
|
||||||
|
return (
|
||||||
|
<Text key={this.getKey()} style={styles}>
|
||||||
|
{children}
|
||||||
|
</Text>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
code(text: string, language?: string, containerStyle?: ViewStyle, _textStyle?: TextStyle) {
|
code(text: string, language?: string, containerStyle?: ViewStyle, _textStyle?: TextStyle) {
|
||||||
return (
|
return (
|
||||||
<View key={this.getKey()} style={containerStyle}>
|
<View key={this.getKey()} style={containerStyle}>
|
||||||
@@ -12,12 +31,28 @@ class CustomRenderer extends Renderer {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
text(text: string | ReactNode[], styles?: TextStyle): ReactNode {
|
||||||
|
return this.plainText(text, styles)
|
||||||
|
}
|
||||||
|
|
||||||
|
strong(children: string | ReactNode[], styles?: TextStyle): ReactNode {
|
||||||
|
return this.plainText(children, styles)
|
||||||
|
}
|
||||||
|
|
||||||
|
em(children: string | ReactNode[], styles?: TextStyle): ReactNode {
|
||||||
|
return this.plainText(children, styles)
|
||||||
|
}
|
||||||
|
|
||||||
|
del(children: string | ReactNode[], styles?: TextStyle): ReactNode {
|
||||||
|
return this.plainText(children, styles)
|
||||||
|
}
|
||||||
|
|
||||||
|
heading(text: string | ReactNode[], styles?: TextStyle): ReactNode {
|
||||||
|
return this.plainText(text, styles)
|
||||||
|
}
|
||||||
|
|
||||||
codespan(text: string, styles?: TextStyle): ReactNode {
|
codespan(text: string, styles?: TextStyle): ReactNode {
|
||||||
return (
|
return this.plainText(text, [styles, { fontStyle: "normal", fontWeight: "normal" }])
|
||||||
<Text selectable key={this.getKey()} style={[styles, { fontStyle: "normal", fontWeight: "normal" }]}>
|
|
||||||
{text}
|
|
||||||
</Text>
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user