Open the agent DM via an optimistic reportID - #95631
Conversation
…identity Generate the owner<->agent DM's reportID client-side, the same way any other new chat's reportID is generated, and write the DM to Onyx optimistically alongside the agent itself. CreateAgent (see Expensify/Auth#22772 and Expensify/Web-Expensify#54367) creates the DM under that exact reportID, so the client can navigate to it immediately after submitting, online or offline, without waiting for the response or reconstructing the agent's identity from Onyx collection diffs. This replaces the previous approach of polling the agent-prompt collection to detect the newly created agent, which produced multiple race-condition bugs across review rounds.
dismissModal() and the delayed navigate() were scheduled independently via setTimeout + isNavigationReady(), which only checks the nav ref exists and doesn't wait for the dismiss transition to settle. Under load (e.g. creating several agents in quick succession) the dismissal's own delayed navigation could land after our navigate() and send the user back to the previous screen instead of the report they just opened. Sequence the navigate through dismissModal's afterTransition callback instead, matching the existing dismissModalWithReport pattern.
getDisplayNameForParticipant short-circuited to the formatted login for any optimistic personal detail, on the assumption that a login is the only thing known about it (true for invite-by-email flows). Agent creation is different: the display name the user chose is already known, but login is intentionally omitted (it's server-assigned), so the DM title resolved to an empty string. Since CREATE_AGENT's response usually lands within a second online, this was an imperceptible flicker there, but offline the write stays queued indefinitely and the blank title persists. Only take the login-based shortcut when no display name is already known, which existing optimistic-personal-detail callers already set to the same value as the login anyway, so their behavior is unchanged.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Matches the Auth/Web-Expensify rename: CreateAgent's primary subject is the agent account, not a report, so the owner<->agent DM's client-supplied ID follows the optimisticReportID naming SharePolicy and MoveIOUReportToExistingPolicy already use for the same kind of auxiliary, side-effect report ID.
dismissModalWithReport is the established pattern for "dismiss the modal and open a report" (used by WorkspaceNewRoomPage, NewChatPage, Split, TeachersUnite, etc.) and already sequences the navigate through dismissModal's afterTransition correctly, so it isn't affected by the race that motivated exporting and patching navigateToReport. That patch is no longer needed, so revert navigateToReport back to its prior implementation and drop its export.
createAgent() already builds an optimistic CREATED action for the DM report (reportActions_<optimisticReportID>), but left its ID random and never forwarded it, so the server generated a different, duplicate CREATED action instead of reconciling onto the client's optimistic one. Generate the ID up front and pass it as createdReportActionID, the same way openReport() forwards its optimistic CREATED action's ID.
Preserve the deferred write and post-create navigation changes while retaining the upstream 1-to-2 transaction highlighting logic so creating a distance expense continues to navigate and highlight the new transaction correctly.
…eportid # Conflicts: # src/libs/ReportUtils.ts # tests/unit/ReportUtilsTest.ts
|
@eVoloshchak 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2019d79ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (shouldHandleNavigation) { | ||
| const navigationActiveReportID = backToReport ?? activeReportID; | ||
| highlightTransactionOnSearchRouteIfNeeded(isFromGlobalCreate, parameters.transactionID, CONST.SEARCH.DATA_TYPES.EXPENSE); | ||
| handleNavigateAfterExpenseCreate({ | ||
| activeReportID: navigationActiveReportID, |
There was a problem hiding this comment.
The current distance submit callers still own post-submit navigation: for example, useExpenseSubmission.ts calls createDistanceRequestIOUActions(...) and then immediately calls performPostBatchCleanup(...), which delegates to cleanupAndNavigateAfterExpenseCreate. Since none of the callers pass the new shouldHandleNavigation/backToReport fields, this default-true block now runs handleNavigateAfterExpenseCreate first and the caller navigates again afterward, which can double-dismiss/race the modal flow and ignore the caller's backToReport target for distance expenses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Weird conflict resolution and some unwanted changes slipped in
mjasikowski
changed the title
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppscreen-20260803-002656-1785709587636.mp4Android: mWeb ChromeScreen.Recording.2026-08-03.at.00.09.11.moviOS: HybridAppScreen.Recording.2026-08-03.at.00.05.37.moviOS: mWeb SafariScreen.Recording.2026-08-03.at.00.07.22.movMacOS: Chrome / SafariScreen.Recording.2026-08-03.at.00.10.00.movScreen.Recording.2026-08-03.at.00.09.35.mov |
eVoloshchak
left a comment
eVoloshchak
left a comment
There was a problem hiding this comment.
The code looks good and this works well!
I think the animation could be improved a bit, though. Currently, the following happens
- You create a new agent (Settings > Agents > Create Agent)
- The create agent page/model is closed, you're navigated back to agents page
- After a small delay you're navigated to the agent DM
This isn't critical, but we could navigate directly to the agent Dm witout closing the create agent page. That's how it works when creating a workspace, you're navigated directly to the workspace settings after pressing create (video below)
Screen.Recording.2026-07-22.at.22.27.58.mov
Reveal the DM route under the AddAgentPage modal before dismissing, instead of dismissing to the agents list first and navigating afterward. Matches the pattern used for workspace creation and expense submission navigation.
|
Hmm cc @Expensify/design @Expensify/product for eyes on this - I don't feel strongly but it does feel slightly odd that you are navigated away from Account as soon as you create an agent. Maybe it makes more sense to open up the agent DM in the RHP next to Account? |
Resolved AddAgentPage.tsx / AddAgentPageTest.tsx conflicts between this PR's optimistic-reportID navigation and main's Onyx-based avatar draft management, keeping both.
|
There's a ad-hoc web build cache problem, so it still uses old files. I'll find someone to fix it |
|
@MelvinBot merge main and resolve conflicts |
…eportid Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com> # Conflicts: # src/pages/settings/Agents/AddAgentPage.tsx
|
🤖 Merged One conflicting file: The two sides were orthogonal, so I kept both:
Resolved linesconst {isKeyboardActive} = useKeyboardState();
const isInLandscapeMode = isInLandscapeModeUtil(windowWidth, windowHeight);
const shouldUseScrollableLayout = isInLandscapeMode || (isMobile() && windowWidth > windowHeight);
const shouldShrinkPromptInput = shouldUseScrollableLayout && isKeyboardActive;
const {accountID: ownerAccountID, login: ownerLogin, displayName} = useCurrentUserPersonalDetails();Imports kept both Please re-verify the |
|
🚧 mjasikowski has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@dubielzyk-expensify @dannymcclain @shawnborton check out the latest web ad hoc here and let me know how this works for you 
|
|
I think that feels okay? It's so abrupt (which I guess is good). Maybe we can highlight the row being added to the table in the background? We already do that pattern when adding expenses to a table, etc. |
|
Would we consider making the Chat buttons also open it in the RHP or do we want that to go to Inbox still? |
|
@MelvinBot merge main and resolve conflict |
…eportid Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com> # Conflicts: # src/pages/settings/Agents/AddAgentPage.tsx # tests/unit/pages/settings/AddAgentPageTest.tsx
|
🤖 Merged Two files conflicted — both because
Note on verificationI couldn't execute the Jest runner in this environment, so the merged unit test wasn't run locally — CI will validate it. I did confirm the tree is free of conflict markers, all referenced symbols exist post-merge, and the test assertions match the merged |
I could go either way. For all other profile views (the Details view in RHP), I think the "Message User" row goes to the Inbox. No strong feelings here. |
Since the RHP approach keeps the Agents table visible underneath the new agent's DM, forward a ref from AgentsTable and use it to highlight (and scroll to) the new row once it appears, mirroring the same pattern already used for newly-invited workspace members in WorkspaceMembersPage.
|
🚧 mjasikowski has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Cool. One thing though, aren't we supposed to have a back arrow here and not an X in the top right. I thought the top right X was only reserved for Concierge RHPs as that's something you can kinda pin to the side. I believe we want a regular back arrow here. @Expensify/design for gut check 
|
|
per @shawnborton:
X makes more sense IMO, because the agent is already created at that point and the back arrow usually means "one step back" - and that would be the agent description prompt |
|
Sorry for missing that. Fair enough 👍 |
Settings uses a split navigator on wide layouts, so AgentsPage is the central pane and stays mounted/visible while the new agent's DM opens in the RHP alongside it, but react-navigation reports it as unfocused in that state (unlike WorkspaceMembersPage, which returns to being the focused screen after its invite modal is dismissed). Gating on useIsFocused silently skipped the highlight every time.
|
🚧 mjasikowski has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Failing test is unrelated. |
|
@chuckdries 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] |
| getRoute: (accountID: number) => `settings/agents/${accountID}/edit/avatar` as const, | ||
| }, | ||
| AGENT_REPORT: { | ||
| route: 'settings/agents/report/:reportID', |
There was a problem hiding this comment.
this is definitely a NIT but can we make the route settings/agents/r/:reportID? Just for consistency with the main inbox report route
| // (e.g. an agent pending creation, or an imported device contact) fall through to the normal name | ||
| // resolution below instead, so that name is shown rather than an empty/placeholder login. | ||
| if (personalDetails.isOptimisticPersonalDetail === true && !personalDetails.displayName) { | ||
| return formattedLogin; |
There was a problem hiding this comment.
This seems like it probably has a wide blast radius, is it strictly necessary? If I'm reading the comment right, pending agents don't take this codepath, so why are we changing the return value here
7 participants
Explanation of Change
Enables opening agent DM immediately after creation
Depends on
Fixed Issues
$ #94395
Tests
Offline tests
QA Steps
Same as Tests. Requires the two backend PRs above to be deployed to staging/production first.
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