Skip to content

Resolve optimistic agent accountID via the replace pattern - #100068

Merged
NicolasBonet merged 14 commits into
mainfrom
nicolas-agent-replace-optimistic
Sep 10, 2026
Merged

NicolasBonet merged 14 commits into
mainfrom
nicolas-agent-replace-optimistic

Conversation

@NicolasBonet

@NicolasBonet NicolasBonet commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

The problem

  • When a user creates an agent, the app first stores it under a temporary ("optimistic") accountID. Only the server can create the real accountID, because the agent's login is based on it.
  • Before this change, the CreateAgent success response deleted the temporary agent data right away. If an agent settings screen was still open on the temporary accountID, it showed the "Hmm... it's not here" page.
  • Requests saved while offline (rename, prompt or avatar update, delete) still pointed to the temporary accountID, which the server does not know, so they failed with a 404.

The fix — the same replace pattern we already use for reports:

  • New Onyx key OPTIMISTIC_AGENT_ACCOUNT_ID_MAPPING: the CreateAgent success response writes a {optimisticAccountID: realAccountID} entry there.
  • New module replaceOptimisticAgentWithActualAgent (mirrors replaceOptimisticReportWithActualReport) listens to that key. For each entry it:
    • waits until navigation is ready, reports are loaded, and screen transitions have finished;
    • updates the accountID on every agent settings route in the navigation state, including routes hidden behind other screens, so the open screen keeps working;
    • moves the agent DM participants from the temporary accountID to the real one (does nothing if the server response already did this);
    • only after that deletes the temporary data and the used mapping entry. This cleanup no longer lives in createAgent()'s successData, so it can never run before the redirect.
  • The module also remembers used mappings in memory, and the agent update/delete actions translate their accountID through it. A write started from an old screen still reaches the real account.
  • New middleware ReplaceOptimisticAgentAccountID (mirrors HandleUnusedOptimisticID) rewrites the requests still waiting in the offline queue: it replaces the temporary accountID in the request data and in the stored Onyx updates (successData, failureData, finallyData, optimisticData, queueFlushedData), both as a string (inside Onyx keys like sharedNVP_agentPrompt_<accountID>) and as a number (the agentAccountID parameter). Mapping entries are validated first, so a broken entry can never damage the queue.

Fixed Issues

$ #93272
PROPOSAL: https://expensify.slack.com/archives/C03TQ48KC/p1788192129722359

Tests

  1. Go to Workspaces > [workspace] > Workflows and enable Approvals.
  2. Under Approvals, click Add Agent, then Create Agent, fill in the details, and click Save.
  3. Verify the flow completes without ever flashing the "Hmm... it's not here" page.
  4. Click Add Agent again.
  5. Verify the new-agent flow opens normally instead of the "Hmm... it's not here" page.
  6. Open the first agent's settings immediately after creating it and edit its name, prompt, and avatar.
  7. Verify each edit saves without errors and the settings screen stays on the agent (no not-found view).
  • Verify that no errors appear in the JS console

Offline tests

  1. Go to Workspaces > [workspace] > Workflows with Approvals enabled.
  2. Go offline.
  3. Click Add Agent, create an agent, and save.
  4. Still offline, edit the new agent's name and prompt from its settings screen.
  5. Go back online.
  6. Verify the agent is created, the name and prompt edits are applied (no error brick road), and the open agent settings screen never shows the "Hmm... it's not here" page.
  7. Verify the agent's DM in the LHN shows the agent with the correct name and avatar.

QA Steps

Same as tests.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

MacOS: Chrome / Safari
CleanShot.2026-09-07.at.2.02.48.PM.mp4

When CREATE_AGENT succeeds, the client's optimistic agent identity is
cleared and the real data lands under a server-assigned accountID, so
any open agent settings screen keyed on the optimistic ID fell through
to the not-found view.

Mirror replaceOptimisticReportWithActualReport: a global Onyx listener
consumes the optimisticAgentAccountIDMapping entry the backend sends,
redirects any focused agent settings screen to the real accountID via
Navigation.setParams, and only then clears the optimistic personal
detail, agent prompt, and the consumed mapping entry. createAgent no
longer clears those keys in successData, so the redirect can never race
the cleanup.

Mirror HandleUnusedOptimisticID for the offline queue: a new middleware
rewrites the ongoing and persisted requests when the mapping arrives,
replacing the optimistic accountID in request data and in the stored
Onyx-update arrays (successData, failureData, finallyData,
optimisticData, queueFlushedData). Strings are rewritten with
deepReplaceKeysAndValues; numeric agentAccountID params need a separate
strict-equality pass because that util only replaces strings.
… routes

The replace module now also: remaps the agent DM's participants from the
optimistic accountID to the real one (a no-op when the CreateAgent
response already swapped them), migrates the optimistic personal detail
and agent prompt onto the real Onyx keys instead of deleting pending
offline edits (dropping only the creation's ADD pendingAction), and
rewrites the accountID param on every agent settings route in the
navigation state rather than only the focused one.

Agent update and delete actions resolve their accountID through a
session-lifetime map of consumed mappings, so a write fired from a stale
screen after reconciliation still targets the real account.
NicolasBonet and others added 2 commits September 1, 2026 15:16
Classify optimisticAgentAccountIDMapping as a safe export key and update
the createAgent success-data test, since cleanup of the optimistic
entries now happens in replaceOptimisticAgentWithActualAgent instead of
successData.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract the shared request-rewriting logic in the middleware into a
rewriteRequest helper and rewrite comments as plain sentences per the
comment style guidelines.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/ONYXKEYS.ts 100.00% <ø> (ø)
src/SplashScreenStateContext.tsx 95.00% <100.00%> (+0.26%) ⬆️
src/libs/AgentAccountIDMapping.ts 100.00% <100.00%> (ø)
src/libs/ExportOnyxState/common.ts 80.95% <ø> (ø)
src/libs/Middleware/register.ts 100.00% <100.00%> (ø)
src/libs/actions/Agent.ts 71.73% <100.00%> (+1.28%) ⬆️
...s/actions/replaceOptimisticAgentWithActualAgent.ts 98.11% <98.11%> (ø)
...libs/Middleware/ReplaceOptimisticAgentAccountID.ts 97.01% <97.01%> (ø)
... and 191 files with indirect coverage changes

… onto the real agent

Skip malformed or identity mapping entries before rewriting queued requests or
migrating optimistic data, copy only pending/error state (and edits it staged)
onto the real personal detail and prompt so server-provided base fields win,
and trim comments to explain intent rather than restate the code.
…nd report hydration

- Write responses only reach Onyx once the sequential queue has drained, so every request queued
  against the optimistic agent has already been rewritten to the real accountID by the middleware
  and completed by the time the mapping is observable. Copying pending markers from the optimistic
  keys chained after that success data and left the real agent permanently pending or resurrected a
  deleted one, so the listener now only redirects, repairs DM participants and clears.
- Defer the redirect and cleanup until Navigation.isNavigationReady() so a persisted mapping consumed
  during app start cannot clear the data under an open agent screen before it is redirected.
- Wait for the report collection to hydrate before consuming the mapping so the participant repair
  does not run against an empty collection on app start.
- Leave the CreateAgent that produced the mapping untouched in the middleware and only re-persist
  the queued requests that actually reference the optimistic accountID.
…time

A queued write's response is only flushed to Onyx after the whole
sequential queue drains, so replaceOptimisticAgentWithActualAgent could
observe the mapping long after the middleware swept the persisted queue.
An agent edit fired in that window was enqueued with the optimistic
accountID after the sweep and reached the server as an unknown account.

Move the consumed-mapping map into its own AgentAccountIDMapping module
(the replace module is lazy-loaded post-splash, so the middleware cannot
import it directly without pulling navigation into the startup path) and
register validated mapping entries there as soon as the middleware sees
them, closing the window.
@NicolasBonet
NicolasBonet marked this pull request as ready for review September 7, 2026 19:04
@NicolasBonet
NicolasBonet requested review from a team as code owners September 7, 2026 19:04
@melvin-bot
melvin-bot Bot requested review from garrettmknight and marufsharifi and removed request for a team September 7, 2026 19:04
@melvin-bot

melvin-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

@marufsharifi 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]

@melvin-bot
melvin-bot Bot removed the request for review from a team September 7, 2026 19:04
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

This PR adds a new Onyx.connectWithoutView call, so I've requested a review from the Onyx performance reviewers (@tgolen, @mountiny, @luacmartins) — a review from any one of them is enough. Please add a link in your PR description to the Slack discussion where the @frontend-performance team approved using connectWithoutView here.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-07T19:19:42.867347Z 5135a53 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tgolen tgolen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve the connectWithoutView() references. I really like the direction of this PR. It's a much cleaner approach to correcting the optimistic IDs.

@NicolasBonet

Copy link
Copy Markdown
Contributor Author

@marufsharifi could you please approve so we can merge?

@garrettmknight
garrettmknight removed their request for review September 8, 2026 17:43
@marufsharifi

Copy link
Copy Markdown

@NicolasBonet, could you please confirm whether the following behavior is expected?

When an Agent is created while offline, the text containing the Agent’s temporary ID changes after reconnecting.

Thank you!

Screen.Recording.2026-09-08.at.8.57.52.PM.mp4

@NicolasBonet

Copy link
Copy Markdown
Contributor Author

@NicolasBonet, could you please confirm whether the following behavior is expected?

When an Agent is created while offline, the text containing the Agent’s temporary ID changes after reconnecting.

Thank you!

Screen.Recording.2026-09-08.at.8.57.52.PM.mp4

That is intended, people will highly likely be only doing that ID reference to refer to the agent so we need to use the latest one always.

@marufsharifi marufsharifi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@melvin-bot
melvin-bot Bot requested a review from carlosmiceli September 9, 2026 17:18
@melvin-bot

melvin-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

@carlosmiceli 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]

@NicolasBonet
NicolasBonet merged commit ab217c4 into main Sep 10, 2026
50 of 53 checks passed
@NicolasBonet
NicolasBonet deleted the nicolas-agent-replace-optimistic branch September 10, 2026 13:59
@github-actions

Copy link
Copy Markdown
Contributor

🚧 NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/NicolasBonet in version: 9.4.76-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

MelvinBot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

🤖 No help site changes are required — no docs PR created.

This PR is an internal state-management fix only. It changes how the app resolves an optimistic agent accountID to the real one (new Onyx key, a new middleware, a new replace module, and tests). Nothing in it changes a user-facing workflow, screen name, button label, or setting.

What I checked

Check Result
Files changed 14 files, all under src/libs/, src/ONYXKEYS.ts, src/SplashScreenStateContext.tsx, and tests/ — no src/languages/* and no UI components
New or renamed user-facing copy None
New or changed steps to create, edit, or delete an agent None — the flow (Workspaces > [workspace] > Workflows > Approvals > Add Agent) is unchanged
Existing agent articles describing the old broken behavior None. I searched docs/articles/new-expensify/ai-agents/ for the "Hmm... it's not here" state, offline caveats, and any 404/refresh workaround — the articles never documented the bug or a workaround, so there's nothing to remove

The user-visible effect is that a bug stops happening: the "Hmm... it's not here" page no longer flashes after creating an agent, and edits queued offline now apply once you're back online. Both match what Create-and-Use-Custom-Agents.md already tells users to expect, so the article is correct as written. Per HELP_AUTHORING_GUIDELINES.md, help articles document the intended workflow, not defect history, so a fix that restores documented behavior needs no article update.

Since no docs changes are needed, I didn't create a draft PR, apply the HelpDot label, or assign anything. There's no linked help site PR to review or mark Ready for review.

@NicolasBonet, if you think a user-facing behavior here should be documented that I've missed — for example agent creation while offline as an explicitly supported flow — reply with @MelvinBot and the detail, and I'll draft the article.


view run · no recording available

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.76-1 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants