diff --git a/.github/workflows/activation-e2e.yml b/.github/workflows/activation-e2e.yml index a1eb2bc..e4610e5 100644 --- a/.github/workflows/activation-e2e.yml +++ b/.github/workflows/activation-e2e.yml @@ -120,12 +120,15 @@ jobs: # different directories, for all-sessions.yaml). 4099 is a fresh normal-mode # instance shared by directory-picker.yaml and variant-picker.yaml (its fake # file tree / provider variants / project list don't affect each other). + # 4100 is normal mode + --seed-diff (one session with a pre-existing wide + # edit-diff tool call + wide code block, for diff-scroll.yaml / issue #21). # All bind 0.0.0.0 so the emulator can reach them via 10.0.2.2. nohup node tests/fixtures/mock-opencode-server.ts --port 4096 > /tmp/mock-4096.log 2>&1 & nohup node tests/fixtures/mock-opencode-server.ts --port 4097 --fail-auth > /tmp/mock-4097.log 2>&1 & nohup node tests/fixtures/mock-opencode-server.ts --port 4098 --seed-sessions > /tmp/mock-4098.log 2>&1 & nohup node tests/fixtures/mock-opencode-server.ts --port 4099 > /tmp/mock-4099.log 2>&1 & - for port in 4096 4097 4098 4099; do + nohup node tests/fixtures/mock-opencode-server.ts --port 4100 --seed-diff > /tmp/mock-4100.log 2>&1 & + for port in 4096 4097 4098 4099 4100; do for i in $(seq 1 30); do if curl -sf --connect-timeout 1 -m 3 "http://127.0.0.1:${port}/global/health" > /dev/null 2>&1; then echo "mock server on ${port} responded (may be 401, that's expected on 4097)" @@ -167,6 +170,7 @@ jobs: echo "--- 4097 (fail-auth) ---"; cat /tmp/mock-4097.log || true echo "--- 4098 (seed-sessions) ---"; cat /tmp/mock-4098.log || true echo "--- 4099 (normal, directory-picker + variant-picker) ---"; cat /tmp/mock-4099.log || true + echo "--- 4100 (seed-diff) ---"; cat /tmp/mock-4100.log || true - name: Upload screenshots if: always() diff --git a/.maestro/flows/diff-scroll.yaml b/.maestro/flows/diff-scroll.yaml new file mode 100644 index 0000000..cb2c0f1 --- /dev/null +++ b/.maestro/flows/diff-scroll.yaml @@ -0,0 +1,119 @@ +appId: cc.agentlabs.opencode +name: "DiffView + CodeBlock horizontal scroll - wide diff/code line reachable by swipe (closes #21)" +--- +# GitHub issue #21 ("QA: verify DiffView + CodeBlock horizontal-scroll +# on-device with a populated diff"). Two rendering fixes shipped earlier were +# code-audited but never verified with real, wide content on a real +# (emulated) device: +# - src/components/chat/DiffView.tsx: diff lines render inside a horizontal +# ScrollView (previously truncated with numberOfLines={1}). +# - src/components/markdown/CodeBlock.tsx: code renders inside a horizontal +# ScrollView (previously long lines wrapped/mangled). +# +# The CI job starts `node tests/fixtures/mock-opencode-server.ts --port 4100 +# --seed-diff` on the runner host BEFORE this flow runs. --seed-diff +# pre-populates ONE session (id "seed-diff", directory /mock/project — the +# default connection's directory) whose single assistant message already +# contains: +# - a completed `edit` tool part with a wide "add" diff line (renders via +# ToolCallCard -> EditDetail -> DiffView once the card is expanded) +# - a text part with a wide fenced code block (renders via Markdown -> +# CodeBlock) +# Both wide lines are ~220 filler characters (no spaces, so they can't +# word-wrap) followed by a distinct marker token. This is loaded via GET +# /session/seed-diff/message when the session is opened (selectSession), +# NOT delivered over SSE — issue #90 (a separate SSE-render bug) is being +# fixed independently, and this flow must not depend on it landing first. +# +# Caveat for whoever debugs a flake here: assertVisible/assertNotVisible on +# a substring deep inside ONE very wide, unwrapped Text node is coarser than +# it looks — Maestro (like most accessibility-tree-based mobile test +# frameworks) determines visibility per ELEMENT, not per glyph, and there are +# open Maestro issues about partially-visible elements being reported as +# fully visible (mobile-dev-inc/maestro #1275, #2411). The filler is made +# very wide (~220 chars, comfortably wider than any emulator screen at this +# monospace font size) specifically to keep the marker's actual rendered +# position off past the right edge at rest. Screenshots are taken at every +# step so a flake here can be diagnosed visually, not just from the +# assertion's pass/fail. + +- launchApp: + clearState: true +- assertVisible: + id: "telemetry-consent-card" +- tapOn: + id: "telemetry-decline-button" + +- assertVisible: + text: "No Connection" +- tapOn: + id: "add-connection-button" +- tapOn: + id: "connect-ip-input" +- inputText: "127.0.0.1:4100" +- hideKeyboard +- tapOn: + id: "connect-submit-button" + +# 40s margin: see activation-positive.yaml — the connect handshake's own +# fetches are individually capped at 30s (src/lib/sdk.ts REQUEST_TIMEOUT_MS). +- extendedWaitUntil: + visible: + id: "connection-status-dot" + timeout: 40000 +- takeScreenshot: diffscroll-S1_connected + +# The seeded session must appear on the home tab's own-directory list (no +# SSE, no roots=true — this is the plain GET /session directory-scoped path). +- extendedWaitUntil: + visible: + text: "Wide Diff Session" + timeout: 15000 +- tapOn: + id: "session-item-seed-diff" +- extendedWaitUntil: + visible: + id: "chat-message-input" + timeout: 15000 + +# History loaded via GET /session/seed-diff/message (not SSE) must already +# render both the tool card and the markdown code block. +- assertVisible: + text: "Edit wide_diff_target.ts" +- assertVisible: + text: "Here is a wide code sample" +- takeScreenshot: diffscroll-S2_history_loaded + +# --- CodeBlock: the wide fenced code block is already expanded (no tap +# needed — Markdown renders it immediately, unlike the collapsible tool card). +- assertVisible: + id: "code-block-scroll" +- assertNotVisible: + text: "ZZZ_CODE_SCROLL_TARGET_ZZZ" +- takeScreenshot: diffscroll-S3_codeblock_marker_offscreen +- swipe: + from: + id: "code-block-scroll" + direction: LEFT +- assertVisible: + text: "ZZZ_CODE_SCROLL_TARGET_ZZZ" +- takeScreenshot: diffscroll-S4_codeblock_marker_revealed + +# --- DiffView: lives inside the collapsible tool card's detail section, so +# expand it first. +- tapOn: + text: "Edit wide_diff_target.ts" +- assertVisible: + text: "src/wide_diff_target.ts" +- assertVisible: + id: "diff-view-scroll" +- assertNotVisible: + text: "ZZZ_DIFF_SCROLL_TARGET_ZZZ" +- takeScreenshot: diffscroll-S5_diffview_marker_offscreen +- swipe: + from: + id: "diff-view-scroll" + direction: LEFT +- assertVisible: + text: "ZZZ_DIFF_SCROLL_TARGET_ZZZ" +- takeScreenshot: diffscroll-S6_diffview_marker_revealed diff --git a/scripts/run-e2e-flows.sh b/scripts/run-e2e-flows.sh index e2d08ea..c6b41d8 100755 --- a/scripts/run-e2e-flows.sh +++ b/scripts/run-e2e-flows.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # # Runs the Maestro activation E2E flows against the mock opencode servers the -# workflow started on the host (ports 4096-4099). Invoked as a single line from +# workflow started on the host (ports 4096-4100). Invoked as a single line from # activation-e2e.yml's emulator-runner `script:` so cd/trap/loop actually work. # # Networking: `adb reverse` maps each mock port so the emulator's own @@ -11,14 +11,14 @@ set -uo pipefail ROOT="$(pwd)" # capture BEFORE any cd, so diag paths are absolute APK="android/app/build/outputs/apk/release/app-release.apk" -FLOWS=(activation-positive activation-negative-401 directory-picker all-sessions variant-picker) +FLOWS=(activation-positive activation-negative-401 directory-picker all-sessions variant-picker diff-scroll) mkdir -p "$ROOT/artifacts/screenshots" "$ROOT/artifacts/diag" echo "== installing APK ==" adb install "$APK" echo "== forwarding mock ports into the emulator (adb reverse) ==" -for p in 4096 4097 4098 4099; do adb reverse "tcp:$p" "tcp:$p"; done +for p in 4096 4097 4098 4099 4100; do adb reverse "tcp:$p" "tcp:$p"; done adb reverse --list # Decisive probe: can the EMULATOR actually reach the host mock via 127.0.0.1? diff --git a/src/components/chat/DiffView.tsx b/src/components/chat/DiffView.tsx index b3ebe7b..eeeaad7 100644 --- a/src/components/chat/DiffView.tsx +++ b/src/components/chat/DiffView.tsx @@ -1,4 +1,5 @@ import { View, Text, StyleSheet, Platform, ScrollView } from "react-native" +import { WIDE_CONTENT_SCROLL_CONFIG } from "../../lib/scroll-config" const mono = Platform.OS === "ios" ? "Menlo" : "monospace" @@ -76,7 +77,7 @@ export function DiffView({ before, after, isDark }: Props) { return ( - + {lines.map((line, idx) => ( {copied ? "Copied!" : "Copy"} - + {code} diff --git a/src/components/wide-content-scroll.regression.test.ts b/src/components/wide-content-scroll.regression.test.ts new file mode 100644 index 0000000..3e0edcd --- /dev/null +++ b/src/components/wide-content-scroll.regression.test.ts @@ -0,0 +1,45 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { readFileSync } from "node:fs" +import { fileURLToPath } from "node:url" +import path from "node:path" + +// GitHub issue #21 ("QA: verify DiffView + CodeBlock horizontal-scroll +// on-device with a populated diff"). The underlying fixes were: +// - src/components/chat/DiffView.tsx: wrap diff lines in a horizontal +// ScrollView (previously truncated with numberOfLines={1}). +// - src/components/markdown/CodeBlock.tsx: wrap code in a horizontal +// ScrollView (previously long lines wrapped/mangled). +// +// This repo's runtime is React Native, so these components can't be rendered +// with node:test (no react-test-renderer / RN jest preset is set up here — +// see package.json's "test" script, which only globs plain .test.ts files). +// Instead of skipping component-level coverage, this test reads the actual +// .tsx source and asserts on its structure: it's a plain-text/regex check, +// but it directly targets the two markers that would prove a regression — +// (a) the ScrollView wiring, (b) reintroducing line-truncation — so it can't +// pass on a source file where the fix was reverted. + +function readComponent(relativePath: string): string { + const dir = path.dirname(fileURLToPath(import.meta.url)) + return readFileSync(path.join(dir, relativePath), "utf8") +} + +test("DiffView wraps diff lines in the shared horizontal-scroll ScrollView", () => { + const src = readComponent("chat/DiffView.tsx") + assert.match(src, / { + const src = readComponent("markdown/CodeBlock.tsx") + assert.match(src, / { + const diffView = readComponent("chat/DiffView.tsx") + const codeBlock = readComponent("markdown/CodeBlock.tsx") + assert.match(diffView, /from ["']\.\.\/\.\.\/lib\/scroll-config["']/) + assert.match(codeBlock, /from ["']\.\.\/\.\.\/lib\/scroll-config["']/) +}) diff --git a/src/lib/scroll-config.test.ts b/src/lib/scroll-config.test.ts new file mode 100644 index 0000000..8e1739e --- /dev/null +++ b/src/lib/scroll-config.test.ts @@ -0,0 +1,26 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { WIDE_CONTENT_SCROLL_CONFIG } from "./scroll-config.ts" + +// GitHub issue #21: DiffView + CodeBlock must render wide content in a +// horizontally-scrollable container (not wrapped, not truncated). Both +// components spread WIDE_CONTENT_SCROLL_CONFIG onto their ScrollView (see +// src/components/chat/DiffView.tsx and src/components/markdown/CodeBlock.tsx) +// so asserting on this object here is asserting on the actual runtime props, +// not a parallel copy that can drift out of sync. + +test("wide-content scroll config enables horizontal scrolling", () => { + assert.equal(WIDE_CONTENT_SCROLL_CONFIG.horizontal, true) +}) + +test("wide-content scroll config shows the horizontal scroll indicator", () => { + // Regression guard: a container that scrolls but hides its indicator is + // easy to mistake for content that simply doesn't overflow. Keep the + // indicator visible so on-device QA (and screenshots) can tell scrollable + // content apart from clipped/truncated content. + assert.equal(WIDE_CONTENT_SCROLL_CONFIG.showsHorizontalScrollIndicator, true) +}) + +test("wide-content scroll config has no unexpected keys", () => { + assert.deepEqual(Object.keys(WIDE_CONTENT_SCROLL_CONFIG).sort(), ["horizontal", "showsHorizontalScrollIndicator"]) +}) diff --git a/src/lib/scroll-config.ts b/src/lib/scroll-config.ts new file mode 100644 index 0000000..b915ede --- /dev/null +++ b/src/lib/scroll-config.ts @@ -0,0 +1,18 @@ +// Pure (no React Native imports) horizontal-scroll configuration shared by +// src/components/chat/DiffView.tsx and src/components/markdown/CodeBlock.tsx. +// +// GitHub issue #21: wide diff lines and wide code-block lines must render in +// a horizontally-scrollable container instead of being wrap-broken or +// truncated with `numberOfLines`. Centralizing the actual runtime props in +// one plain object lets that decision be unit-tested with node:test (no React +// Native renderer needed) while both components spread the SAME object onto +// their ScrollView, so the test and the real components can't drift apart. +export interface HorizontalScrollConfig { + horizontal: true + showsHorizontalScrollIndicator: boolean +} + +export const WIDE_CONTENT_SCROLL_CONFIG: HorizontalScrollConfig = { + horizontal: true, + showsHorizontalScrollIndicator: true, +} diff --git a/tests/fixtures/mock-opencode-server.ts b/tests/fixtures/mock-opencode-server.ts index 33633ff..d039278 100644 --- a/tests/fixtures/mock-opencode-server.ts +++ b/tests/fixtures/mock-opencode-server.ts @@ -68,6 +68,7 @@ // node tests/fixtures/mock-opencode-server.ts --port 4096 // node tests/fixtures/mock-opencode-server.ts --port 4097 --fail-auth // node tests/fixtures/mock-opencode-server.ts --port 4098 --seed-sessions +// node tests/fixtures/mock-opencode-server.ts --port 4100 --seed-diff import http from "node:http" import { randomUUID } from "node:crypto" @@ -88,6 +89,20 @@ export interface MockServerOptions { * open regression for GitHub issues #46/#48). */ seedSessions?: boolean + /** + * Pre-populate a session (id "seed-diff", in DEFAULT_DIRECTORY so it shows + * up on the normal home-tab session list) with ONE assistant message that + * already contains a wide `edit` tool part (renders via DiffView) and a + * wide fenced code block in its text (renders via CodeBlock) — used by + * .maestro/flows/diff-scroll.yaml (GitHub issue #21) to prove both + * components' horizontal ScrollView actually scrolls on-device. + * + * Deliberately seeded as pre-existing history, fetched via GET + * /session/:id/message, NOT delivered over SSE — the positive-flow SSE + * render bug (issue #90) is being fixed separately, and this flow must not + * depend on it landing first. + */ + seedDiff?: boolean } interface StoredSession { @@ -106,6 +121,16 @@ interface StoredPart { messageID: string type: string text?: string + // Tool part fields (type: "tool") — mirrors src/lib/sdk.ts's Part interface, + // needed to seed a populated `edit` tool call for --seed-diff. + tool?: string + callID?: string + state?: { + status: "pending" | "running" | "completed" | "error" + input?: unknown + output?: unknown + title?: string + } } interface StoredMessageInfo { @@ -166,13 +191,97 @@ export const FAKE_SERVER_PROJECTS = [ }, ] +// Exported so .maestro/flows/diff-scroll.yaml's assertions and this file's +// own seeding logic share one source of truth for what's "off-screen until +// you scroll" in the seeded diff/code content. +export const SEED_DIFF_SESSION_ID = "seed-diff" +export const SEED_DIFF_TITLE = "Wide Diff Session" +export const SEED_DIFF_TOOL_TITLE = "Edit wide_diff_target.ts" +export const SEED_DIFF_LINE_MARKER = "ZZZ_DIFF_SCROLL_TARGET_ZZZ" +export const SEED_CODE_LINE_MARKER = "ZZZ_CODE_SCROLL_TARGET_ZZZ" + +// Long, space-free filler so the RN can't word-wrap it — it just +// overflows its parent, which is exactly what the horizontal ScrollView (the +// fix under test) exists to make scrollable instead of clipped/truncated. +const WIDE_FILLER = "x".repeat(220) + export function createMockOpencodeServer(opts: MockServerOptions) { - const { port, failAuth = false, replyText = DEFAULT_REPLY_TEXT, replyDelayMs = 300, seedSessions = false } = opts + const { + port, + failAuth = false, + replyText = DEFAULT_REPLY_TEXT, + replyDelayMs = 300, + seedSessions = false, + seedDiff = false, + } = opts const sessions = new Map() const messagesBySession = new Map() const sseClients = new Set() + if (seedDiff) { + const now = Date.now() + const session: StoredSession = { + id: SEED_DIFF_SESSION_ID, + slug: "seed-diff", + projectID: "mock-project", + directory: DEFAULT_DIRECTORY, + title: SEED_DIFF_TITLE, + version: "0.0.0-mock", + time: { created: now - 30_000, updated: now - 20_000 }, + } + sessions.set(session.id, session) + + const messageID = "seed-diff-msg" + const info: StoredMessageInfo = { + id: messageID, + sessionID: session.id, + role: "assistant", + time: { created: now - 25_000, completed: now - 20_000 }, + modelID: "mock-model", + providerID: "mock", + } + + // Populated `edit` tool call: renders via ToolCallCard -> EditDetail -> + // DiffView (src/components/chat/DiffView.tsx) once the card is expanded. + // One short unchanged line + one wide added line, so DiffView's diff has + // both a context row and an "add" row whose text overflows the ScrollView. + const toolPart: StoredPart = { + id: "seed-diff-tool", + sessionID: session.id, + messageID, + type: "tool", + tool: "edit", + callID: "seed-diff-call-1", + state: { + status: "completed", + title: SEED_DIFF_TOOL_TITLE, + input: { + filePath: "src/wide_diff_target.ts", + oldString: "const shortLine = 1", + newString: `const shortLine = 1\nconst wideLine = "${WIDE_FILLER}${SEED_DIFF_LINE_MARKER}"`, + }, + output: "applied", + }, + } + + // Wide fenced code block in the reply text: renders via + // Markdown -> CustomRenderer.code -> CodeBlock + // (src/components/markdown/CodeBlock.tsx). + const textPart: StoredPart = { + id: "seed-diff-text", + sessionID: session.id, + messageID, + type: "text", + text: + "Here is a wide code sample:\n\n```typescript\n" + + `const wideCodeLine = "${WIDE_FILLER}${SEED_CODE_LINE_MARKER}"\n` + + "```\n", + } + + messagesBySession.set(session.id, [{ info, parts: [toolPart, textPart] }]) + } + if (seedSessions) { const now = Date.now() const seedDefault: StoredSession = { @@ -539,12 +648,13 @@ export function createMockOpencodeServer(opts: MockServerOptions) { } } -function parseArgs(argv: string[]): { port: number; failAuth: boolean; seedSessions: boolean } { - const opts = { port: 4096, failAuth: false, seedSessions: false } +function parseArgs(argv: string[]): { port: number; failAuth: boolean; seedSessions: boolean; seedDiff: boolean } { + const opts = { port: 4096, failAuth: false, seedSessions: false, seedDiff: false } for (let i = 0; i < argv.length; i++) { if (argv[i] === "--port") opts.port = Number(argv[++i]) else if (argv[i] === "--fail-auth") opts.failAuth = true else if (argv[i] === "--seed-sessions") opts.seedSessions = true + else if (argv[i] === "--seed-diff") opts.seedDiff = true } return opts } @@ -557,7 +667,7 @@ if (invokedDirectly) { const mock = createMockOpencodeServer(opts) mock.listen().then(() => { console.log( - `[mock-opencode-server] listening on ${mock.url} (failAuth=${opts.failAuth}, seedSessions=${opts.seedSessions})`, + `[mock-opencode-server] listening on ${mock.url} (failAuth=${opts.failAuth}, seedSessions=${opts.seedSessions}, seedDiff=${opts.seedDiff})`, ) }) const shutdown = () => {