From 7ca5d2eb19b7934e018a907d5934fc93e0a99b41 Mon Sep 17 00:00:00 2001 From: engineer Date: Thu, 16 Jul 2026 17:19:46 -0700 Subject: [PATCH] test(activation): address code-review findings on E2E flows, mock, CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes (REQUEST_CHANGES round 1): 1. HIGH activation-negative-401.yaml: after dismissing the "Connection Failed" alert the app stays on the Add-Connection modal (handleQuickConnect's failure branch never calls router.back()), so the old `text: "No Connection"` assertion (Sessions-tab empty state) could never pass. Now asserts connect-submit-button is still visible instead. 2. MEDIUM mock-opencode-server.ts: prompt_async now parses the request body, persists the USER's message, and broadcasts it (message.updated + message.part.updated) BEFORE the canned assistant reply — matching real server behavior. Without this, the app's handleEvent strips the optimistic temp- user message when the assistant's message.updated arrives and the sent message vanishes from the transcript. activation-positive.yaml now also asserts chat-bubble-user and the user's message text are visible after the reply lands, so that regression class is actually covered. 3. MEDIUM activation-e2e.yml: timeout-minutes 15 -> 60. The job runs the same npm install + prebuild + assembleRelease + emulator pipeline that cua-smoke.yml budgets 60 min for (emulator-boot-timeout alone is 10 min). 4. MEDIUM activation-e2e.yml: replicated cua-smoke.yml's "Purge stale generated sources" step — the Gradle cache key/restore-keys are shared with that workflow, so the stale-autolinking-tree failure mode (compileReleaseJavaWithJavac against the old package id) applies here too. Verified locally: tsc --noEmit clean; npm test 81/81 pass; all three touched YAML files parse valid; mock server exercised standalone — full prompt cycle confirms GET /session/:id/message returns BOTH user and assistant messages, SSE order is message.updated(user) -> message.part.updated(user) -> busy -> message.updated(assistant) -> message.part.updated(assistant) -> idle, user events carry the sessionID/messageID fields handleEvent filters on, and --fail-auth mode returns 401. Still no emulator run in this environment. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NJKAQ6HAikWGQK7PGZ5Y4E --- .github/workflows/activation-e2e.yml | 15 ++++++- .maestro/flows/activation-negative-401.yaml | 10 +++-- .maestro/flows/activation-positive.yaml | 8 ++++ tests/fixtures/mock-opencode-server.ts | 49 +++++++++++++++++++++ 4 files changed, 77 insertions(+), 5 deletions(-) diff --git a/.github/workflows/activation-e2e.yml b/.github/workflows/activation-e2e.yml index 81c32e1..d222c59 100644 --- a/.github/workflows/activation-e2e.yml +++ b/.github/workflows/activation-e2e.yml @@ -37,7 +37,11 @@ on: jobs: activation-e2e: runs-on: ubuntu-latest - timeout-minutes: 15 + # Same npm install + expo prebuild + assembleRelease + emulator pipeline as + # cua-smoke.yml, which budgets 60 min (emulator-boot-timeout alone is 10 min). + # Typical runs finish well under this; the ceiling just avoids flaky kills + # on cold Gradle caches. + timeout-minutes: 60 steps: - uses: actions/checkout@v6 @@ -74,6 +78,15 @@ jobs: restore-keys: | ${{ runner.os }}-gradle- + - name: Purge stale generated sources + # Same mitigation as cua-smoke.yml (whose Gradle cache entries this job + # shares — the cache key/restore-keys are identical): the restore-keys + # prefix fallback can restore a generated autolinking tree from a + # previous package id, making compileReleaseJavaWithJavac fail against + # the old package (ai.opencode.mobile vs cc.agentlabs.opencode). Delete + # generated sources so prebuild + Gradle regenerate them. + run: rm -rf android/app/build/generated android/build/generated android/app/build/intermediates + - name: Install Maestro CLI run: | curl -Ls "https://get.maestro.mobile.dev" | bash diff --git a/.maestro/flows/activation-negative-401.yaml b/.maestro/flows/activation-negative-401.yaml index f50efed..397de1b 100644 --- a/.maestro/flows/activation-negative-401.yaml +++ b/.maestro/flows/activation-negative-401.yaml @@ -60,9 +60,11 @@ name: Activation - negative path (connect-time 401 must surface a visible error) - takeScreenshot: negative-S4_visible_error_alert # The connection must NOT have been silently saved: dismiss the alert and -# confirm we are still on the empty/no-connection state, not a fake -# "connected" screen. +# confirm we are still on the add-connection screen, not a fake "connected" +# screen. (handleQuickConnect's failure branch never calls router.back(), so +# the app stays on this modal — the Sessions-tab "No Connection" empty state +# is NOT visible here.) - tapOn: "OK" - assertVisible: - text: "No Connection" -- takeScreenshot: negative-S5_still_disconnected_after_dismiss + id: "connect-submit-button" +- takeScreenshot: negative-S5_still_on_add_connection_after_dismiss diff --git a/.maestro/flows/activation-positive.yaml b/.maestro/flows/activation-positive.yaml index 1937406..c328462 100644 --- a/.maestro/flows/activation-positive.yaml +++ b/.maestro/flows/activation-positive.yaml @@ -71,4 +71,12 @@ name: Activation - positive path (consent -> connect -> send -> reply) id: "chat-bubble-assistant" - assertVisible: text: "Hello from the mock opencode server" +# The USER's message must still be in the transcript after the reply lands. +# Regression guard: any message.updated event strips optimistic temp- +# messages (src/stores/sessions.ts handleEvent), so if the server never +# persisted/broadcast the user's message, it would vanish here. +- assertVisible: + id: "chat-bubble-user" +- assertVisible: + text: "Hello from the activation e2e test" - takeScreenshot: positive-S8_reply_received diff --git a/tests/fixtures/mock-opencode-server.ts b/tests/fixtures/mock-opencode-server.ts index 21d5892..5004d2d 100644 --- a/tests/fixtures/mock-opencode-server.ts +++ b/tests/fixtures/mock-opencode-server.ts @@ -16,6 +16,14 @@ // returns immediately; the actual reply is delivered as // `message.updated` + `message.part.updated` + `session.status` (idle) // events on the SSE stream (src/stores/events.ts). +// - The real server ALSO persists and broadcasts the USER's message. The +// app relies on this: any `message.updated` event strips optimistic +// `temp-` messages (src/stores/sessions.ts handleEvent), so if the mock +// only broadcast the assistant reply, the user's sent message would +// vanish from the transcript. The mock therefore stores the user message +// from the prompt_async body and broadcasts it (message.updated + +// message.part.updated) before the canned assistant reply, and returns +// it from GET /session/:id/message. // // Two modes: // - Normal mode: implements the endpoints above so the app can connect, @@ -110,6 +118,39 @@ export function createMockOpencodeServer(opts: MockServerOptions) { }) } + // Persist the user's message (parsed from the prompt_async body) and + // broadcast it over SSE, mirroring the real server. This is what lets the + // app replace its optimistic `temp-` user message with the real one instead + // of losing it when the assistant's message.updated arrives. + function storeUserMessage(sessionID: string, promptParts: Array<{ type?: string; text?: string }>) { + const list = messagesBySession.get(sessionID) + if (!list) return + + const now = Date.now() + const messageID = randomUUID() + const info: StoredMessageInfo = { + id: messageID, + sessionID, + role: "user", + time: { created: now, completed: now }, + } + const parts: StoredPart[] = promptParts + .filter((p) => p.type === "text" && typeof p.text === "string") + .map((p) => ({ + id: randomUUID(), + sessionID, + messageID, + type: "text", + text: p.text, + })) + + list.push({ info, parts }) + broadcast("message.updated", { info }) + for (const part of parts) { + broadcast("message.part.updated", { part }) + } + } + function scheduleReply(sessionID: string) { const list = messagesBySession.get(sessionID) if (!list) return @@ -262,7 +303,15 @@ export function createMockOpencodeServer(opts: MockServerOptions) { if (!sessions.has(sid)) { return json(res, 404, { error: `unknown session ${sid}` }) } + let promptParts: Array<{ type?: string; text?: string }> = [] + try { + const parsed = JSON.parse(body || "{}") + if (Array.isArray(parsed.parts)) promptParts = parsed.parts + } catch { + // malformed body — still ack like a fire-and-forget endpoint would + } json(res, 200, { ok: true }) + storeUserMessage(sid, promptParts) scheduleReply(sid) }) return