fix(mcp/mobile): learn-tap-module Branch-B scoping + adb-restart pre-step ordering#355
Merged
Merged
Conversation
…step ordering Two orthogonal bugs surfaced on malaria-itn-fgd/20260515-1645 Phase 6 attempt 12. Both unrelated to PR #354's Learn-Deliver chain scope. Bug A — learn-tap-module.yaml Branch-B over-fires when form-name != module-name: The same-name-suppressed-auto-skip branch guarded only on `screen_suite_menu_list visible AND nav_btn_next NOT visible`, then re-tapped `text:${MODULE_NAME}`. On J1 (module "Briefing Acknowledgement" → form "Acknowledge Readiness"), the form-list screen's toolbar still reads ${MODULE_NAME}, so Branch-B fired and re-tapped the non-tappable toolbar TextView instead of the form row. Fix: nested runFlow with inner guard `visible: text: ${MODULE_NAME}, below: id: screen_suite_menu_list` so the text-match is scoped to the list body. When form-name != module-name no body row matches, Branch-B skips, caller's next learn-tap-module(FORM_NAME) drills the form row by its own label. Bug B — sweepStaleEmulatorState ordering (PR #349 follow-up): PR #349 wired up the orphan-qemu sweep and adb-server restart but (a) used a conservative `qemuPids.length >= liveCount + 2` kill threshold that left the attempt-12 signature (2 orphan qemu + 1 stale adb-devices entry, 2 NOT >= 1+2) below the kill bar, and (b) ran the adb-restart immediately after the kill without waiting for the kernel to release the emulator-NNNN TCP sockets — letting the freshly-restarted daemon adopt the wedged-port state. Result: 2 of the next 3 ensureAvdRunning calls still failed with "package service did not bind" until a second manual adb kill-server/start-server fired inside the dispatch. Fix: 1. Loosen the orphan kill to fire whenever qemuPids.length > liveCount. 2. Add a 500ms socket-release wait between the last orphan SIGKILL and adb kill-server. Tests: - static-recipe-invariants.test.ts asserts Branch-B's inner runFlow has `visible: text: ${MODULE_NAME}, below: id: screen_suite_menu_list`. - avd.test.ts asserts orphan-qemu kill precedes adb kill-server AND there's a ≥400ms gap between them (we wait 500ms, 100ms scheduling slack). Second test asserts the loosened threshold actually fires kills on the attempt-12 2-PID + 1-live-device signature. Live reproducer: /Users/jjackson/.maestro/tests/2026-05-19_151650/ screenshot-❌-1779218294468-(chunk-0.yaml).png. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
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
Two orthogonal bugs surfaced on
malaria-itn-fgd/20260515-1645Phase 6 attempt 12. Both unrelated to PR #354's Learn-Deliver chain scope.Bug A —
learn-tap-module.yamlBranch-B over-fires when form-name ≠ module-nameBranch-B (CommCare's same-name-suppressed-auto-skip case) guarded only on
screen_suite_menu_list visible AND nav_btn_next NOT visible, then re-tappedtext:${MODULE_NAME}. On J1 (module"Briefing Acknowledgement"→ form"Acknowledge Readiness"), the form-list screen's toolbar still reads${MODULE_NAME}, so Branch-B fired and re-tapped the non-tappable toolbar TextView instead of the form row. The downstreamextendedWaitUntil nav_btn_next visiblethen expired against the unchanged form-list.Fix: nested
runFlowwith inner guardvisible: text: ${MODULE_NAME}, below: id: screen_suite_menu_listso the text-match is scoped to the list body. When form-name ≠ module-name no body row matches, Branch-B skips, and the caller's nextlearn-tap-module(FORM_NAME)drills the form row by its own label.Bug B —
sweepStaleEmulatorStateordering (PR #349 follow-up)PR #349 wired up the orphan-qemu sweep and
adb-serverrestart but:qemuPids.length >= liveCount + 2kill threshold that left the attempt-12 signature (2 orphan qemu + 1 stale adb-devices entry:2 NOT >= 1+2) below the kill bar;emulator-NNNNTCP sockets — letting the freshly-restarted daemon adopt the wedged-port state.Result: 2 of the next 3
ensureAvdRunningcalls still failed withpackage service did not binduntil a second manualadb kill-server/start-serverfired inside the dispatch.Fix:
qemuPids.length > liveCount.SIGKILLandadb kill-server.Tests
test/mcp/mobile/static-recipe-invariants.test.tsasserts Branch-B's innerrunFlowhasvisible: text: ${MODULE_NAME}, below: id: screen_suite_menu_list.test/mcp/mobile/avd.test.tsasserts orphan-qemu kill precedesadb kill-serverAND there's a ≥400ms gap between them (we wait 500ms; 100ms scheduling slack). A second test asserts the loosened threshold fires kills on the attempt-12 2-PID + 1-live-device signature.Live reproducer:
~/.maestro/tests/2026-05-19_151650/screenshot-❌-1779218294468-(chunk-0.yaml).png.Test plan
npm test— 1238 passed, 43 skipped (incl. new regression tests above)🤖 Generated with Claude Code