[No QA] Settle agent-device drive actions and preserve sign-in replay diagnostics - #98094
Merged
rlinoz merged 3 commits intoAug 11, 2026
Merged
Conversation
press_label, the onboarding name fill, and the sign-in fallback now pass
--settle, so agent-device waits for the UI to go quiet before returning instead
of the drive loop sleeping blindly and snapshotting a half-rendered transition.
--settle returns the settled diff on stdout, and this library must not write
there — callers may parse stdout as a machine protocol. agent-device already
leaked action results that way ("Tapped (x, y)", "Filled N chars"), so every
internal drive command now routes stdout to an artifacts drive log.
Replay keeps its stderr in a dedicated log: on failure it names the diverging
step, the selector it could not match, and a repair hint, which the previous
generic fallback message discarded.
Contributor
Author
|
It's a counterpart PR for https://github.com/Expensify/melvin/pull/333 that needs to be merged after original improvement. |
|
@linhvovan29546 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
wait_for_login_field waited on SEL_LOGIN_FIELD, which carries no editable predicate, so it returned as soon as the input had rendered. sign-in.ad then fills that field requiring editable=true, so the replay could start against an element that existed but was not yet interactive and fail at step 1: Replay failed at step 1 (fill "id=\"username\" || role=\"textfield\" label=\"Phone or email\" editable=true || ..."): Selector did not match The selector is not stale - id="username" is still on the login form at src/pages/signin/LoginForm/BaseLoginForm.tsx:256, and the direct-fill fallback matched the same element moments later using a selector without the predicate. The wait and the fill were testing different conditions. The wait now mirrors the macro's selector. SEL_LOGIN_FIELD keeps its existing form for the fallback fill, which does not require the element to report editable.
rlinoz
requested review from
Julesssss and
rlinoz
and removed request for
linhvovan29546
August 10, 2026 12:33
Julesssss
previously approved these changes
Aug 10, 2026
Julesssss
left a comment
Julesssss
left a comment
Contributor
There was a problem hiding this comment.
Verbose comments could be simplified, but NAB
Contributor
Author
|
@Julesssss Done ✅ Ready for merge. |
rlinoz
approved these changes
Aug 11, 2026
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Contributor
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Contributor
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.4.53-0 🚀
|
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
4 participants
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.
Explanation of Change
Three related fixes to
.claude/skills/agent-device/flows/lib/sign-in-drive.sh. No production App code is touched — this is agent tooling under.claude/.1. Settle drive actions instead of sleeping blindly.
press_label, the onboarding name fill, and the sign-in fallback fill/press now pass--settle, soagent-devicewaits for the UI to go quiet (500ms quiet window, 10s deadline) and returns the settled diff. Previously the drive loop pressed, slept a fixed 1s, then snapshotted — which samples the accessibility tree mid-transition. The observed symptom is the onboarding ladder loggingonboarding clear — nothing actionableseveral times in a row while the next screen is still rendering, then giving up on a screen it could have handled.2. Keep drive-command output off stdout. This library documents that it never emits protocol lines, because callers may parse its stdout as a machine protocol.
agent-devicewas already breaking that from insidepress_labeland the name fill — it echoesTapped (x, y)andFilled N charsto stdout — and--settlemakes it much worse by returning a full settled diff there. Every internal drive command now routes stdout toartifacts/melvin-drive-<session>.log.3. Keep replay's diagnosis.
agent-device replaypreviously had neither stream redirected, so its failure output was discarded into the caller's stdout. On failure it names the diverging step, the exact selector it could not match, and a repair hint. That now lands inartifacts/melvin-signin-replay-<session>.log, and the last lines are surfaced throughhuman(). Same treatment for the fill/press fallback path. Previously a failed sign-in reported only a generic "direct fill failed" with no way to tell a selector drift from a timeout.The same three fixes land in parallel in the Melvin staging mirror of this library (Expensify/melvin#333), which this file's header already describes; they should stay in sync until that mirror is retired.
Artifact paths use the existing
${GITHUB_WORKSPACE:-/tmp}/artifactsconvention already in this file, so the standalone (non-CI) mode writes under/tmpand still works.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/667790
PROPOSAL:
Tests
bash -n .claude/skills/agent-device/flows/lib/sign-in-drive.sh— parses clean.shellcheck -S warning .claude/skills/agent-device/flows/lib/sign-in-drive.sh— no findings.agent-device open "https://dev.new.expensify.com:8082/" --platform web --session drive-test, then run.claude/skills/agent-device/flows/lib/sign-in-drive.sh --platform web --session drive-test --email <fresh test alias>— verify it signs in and exits 0, and that${GITHUB_WORKSPACE:-/tmp}/artifacts/melvin-drive-drive-test.lognow contains theTapped/Filled/ settled-diff output that previously went to stdout.agent-device replayfails — verifymelvin-signin-replay-drive-test.logcontains a realREPLAY_DIVERGENCEnaming the failed step and selector, and that the same detail is echoed throughhuman()rather than the old generic message.Offline tests
N/A — agent tooling, not reachable from the app UI and not network-state dependent.
QA Steps
N/A —
[No QA], no user-facing surface.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
N/A — no UI surface. This changes shell tooling under
.claude/.