LHN — report with a failed DEW approval shows a GBR for every participant, not just the approver

[melvin-bot]

Problem

When a Dynamic External Workflow approval fails (for example, someone tries to approve their own report and the API returns You cannot approve your own reports.), Auth writes a DEWAPPROVEFAILED report action and pushes it to every participant on the report. The App then surfaces the report in the LHN with a green dot for all of them — including people who are neither the report owner, the manager, nor the actor who triggered the failed approval.

Reported case: a user with only auditor/comment permissions on an expense report saw it pinned in their LHN with a GBR, while their next step still read "waiting for <approver> to approve". They had no action to take.

Root cause

In getReasonAndReportActionThatRequiresAttention in src/libs/ReportUtils.ts, the first check returns HAS_DEW_APPROVE_FAILED whenever the report statusNum is SUBMITTED and any non-deleted DEW_APPROVE_FAILED action exists on it. There is no check on the current user's relationship to the report.

That sets requiresAttention, which in SidebarUtils.shouldDisplayReportInLHN overrides a hidden notification preference and pins the row with a GBR.

The neighbouring paths are gated correctly, which is what makes this look like an oversight:

• the DEW submit-failed branch requires isReportOwner
• the "Fix issue" next step requires isCurrentUserTheApprover

Expected behavior

A failed DEW approval should only require attention from the user who can actually resolve it — the current approver (and arguably the report owner). Other participants should not get a GBR or a pinned LHN row.

Actual behavior

Every participant on the report gets the GBR and the pinned LHN row, with no matching next step telling them what to do.

Steps to reproduce

1. Set up a workspace with a Dynamic External Workflow and several participants on an expense report.
2. Submit the report, then have the submitter attempt to approve it themselves so the approval fails.
3. Sign in as an unrelated participant (not owner, not manager, not the actor).
4. The report appears in that user's LHN with a GBR, while their next step still shows "waiting for <approver> to approve".

Proposed fix

Gate the HAS_DEW_APPROVE_FAILED branch on the current user being the approver (or the report owner), matching how the DEW submit-failed branch and the "Fix issue" next step are already gated.

Reported in Slack.

Issue Owner

Current Issue Owner: @Krishna2323

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Krishna2323 (External)

[melvin-bot]

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

[MelvinBot]

Proposal

What is the root cause of that problem?

src/libs/ReportUtils.ts:4607-4616 returns HAS_DEW_APPROVE_FAILED for any report with statusNum === SUBMITTED that has an active DEWAPPROVEFAILED action. It never looks at who the current user is. It is also the first check in getReasonAndReportActionThatRequiresAttention, so it short-circuits every later, correctly-gated check.

That reason becomes requiresAttention in the report attributes (ReportUtils.ts:13833-13845), and requiresAttention is what overrides a hidden notification preference (SidebarUtils.ts:201-215), pins the row into the GBR group, and puts the report in the To-do tab (SidebarUtils.ts:980-982). So every participant Auth pushed the action to gets a green dot.

The two neighbouring DEW paths are gated, which is why the GBR and the next step disagree:

Path	Gate
DEW submit failed	isReportOwner(report)
"Fix issue" next step	isCurrentUserTheApprover via useOptimisticNextStep.ts:78-83
DEW approve failed GBR	none

#79165 ("Add DEW support for approve action") added the approver gate to the next step but not to the GBR branch, so a non-approver gets the dot with no matching next step — exactly what the auditor reported.

What changes do you think we should make in order to solve the problem?

1. Gate the branch on the current user being the approver — only return HAS_DEW_APPROVE_FAILED when optionOrReport.managerID === currentUserAccountID, mirroring shouldShowDynamicExternalWorkflowApproveErrorNextStep.
2. Move the block below the archive guard at ReportUtils.ts:4625-4627. Today an archived report still keeps the GBR because the DEW check runs before that early return.
3. Skip deleted actions. Line 4608 scans raw Object.values(reportActions), while the submit-failed caller filters with !isDeletedAction first (ReportUtils.ts:10196). Match that.
4. Tests: the existing case at tests/unit/ReportUtilsTest.ts:11441-11474 builds its report with createRandomReport, so it needs an explicit managerID: currentUserAccountID to keep passing. Add a companion case asserting a participant who is neither manager nor owner gets no HAS_DEW_APPROVE_FAILED.

After this, the hasDEWApproveFailed && isCurrentUserTheApprover pair in useOptimisticNextStep is redundant but harmless — either leave it or drop the duplicate approver check.

One product call for the issue owner: should the report owner also get the GBR? The "Fix issue" copy reads "Waiting for <owner> to fix the issues" yet is only rendered for the approver. I'd ship approver-only (symmetric with the submit-failed path, which gates on the person who can retry the action) and add isReportOwner only if the intent is that the submitter fixes something.

What alternative solutions did you explore? (Optional)

• Filter in SidebarUtils.shouldDisplayReportInLHN. Wrong layer — requiresAttention is also consumed by the To-do tab and Search, so each consumer would need the same filter.
• Have Auth push DEWAPPROVEFAILED only to the approver. Backend change, and the action is legitimate report history everyone should see. The defect is the audience of the GBR, not the action.

Investigation details, confidence, and what I did not verify

Confidence: medium-high. The code path is confirmed end to end (ungated reason → requiresAttention → hidden-preference override + GBR pin + To-do), and the reported symptom pair (GBR present, next step still "waiting for <approver> to approve") is the exact signature of a reason that is ungated while the next step is gated.

Alternative explanations considered and ruled out:

• hasOutstandingChildRequest / IOU badge path — those live below the DEW check and take currentUserAccountID, so they cannot fire for an auditor with only comment rights.
• isUnreadWithMention — would need an actual mention, and it is also below the DEW check.
• hasUnresolvedCardFraudAlert — different action type.

Not verified: I did not reproduce this in a live session (a failed DEW approval needs a real Dynamic External Workflow policy plus a backend rejection) and I pulled no production logs. To confirm on the reported report, open it as the affected auditor with Troubleshoot → Debug mode; DebugUtils surfaces the reason (DebugUtils.ts:1575) and it should read hasDEWApproveFailed.

Possibly related: #100123 — "Card fraud alert green dot never clears when the fraud is cleared on the card, and shows to the wrong audience". Same function, the very next check (ReportUtils.ts:4618-4623), and also ungated on the current user and above the archive guard. Whoever fixes this one should look at both. No other open duplicates found.

💡 Suggested AI code review rule

Two independent occurrences of the same shape in the same function suggests this is worth catching mechanically rather than by review memory. Proposed for Expensify/App:

• Rule: CONSISTENCY-18 — Gate every requiresAttention reason on the current user
• Condition: flag any branch in getReasonAndReportActionThatRequiresAttention (or any other requiresAttention producer) that returns a reason without testing the current user's relationship to the report — managerID, ownerAccountID, participant role, or an explicit currentUserAccountID / currentUserLogin comparison. Also require the branch to sit below the isReportArchived guard.
• Why: requiresAttention overrides a hidden notification preference, pins the LHN row with a GBR, and adds the report to the To-do tab. An ungated reason nags every participant with a to-do they cannot action. Motivated by this issue and Add DEW support for approve action #79165.
• Search pattern: rg -n "reason: CONST.REQUIRES_ATTENTION_REASONS" src/libs/ReportUtils.ts, then check each enclosing condition references currentUserAccountID, currentUserLogin, managerID, or ownerAccountID.
• Offending code (❌): ReportUtils.ts:4607-4616
• Correctly gated analogue (✅): NextStepUtils.ts:374-381 and ReportUtils.ts:10214-10220

The full rule file, with the ❌/✅ code inline in App's coding-standards format, would live in Expensify/App itself. If it's worth adding, a maintainer can open it there as a draft PR.

---

Next Steps for Contributor+ team:
To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/100863) to create a draft PR.
To refine: @MelvinBot <your feedback>
To reject: Explain why you are rejecting Melvin's proposal.

[Krishna2323]

@MelvinBot Root cause is right and the approver gate is the correct fix, but a few corrections before moving forward:

1. Your line numbers are from a68e6f4. On main the branch is ReportUtils.ts:4492-4501 and the archive guard is at ReportUtils.ts:4510.

2. Use the existing isReportManager(optionOrReport, currentUserAccountID) (ReportUtils.ts:1559) instead of an inline managerID === currentUserAccountID comparison.

3. Point 3 is unnecessary. getMostRecentActiveDEWApproveFailedAction (ReportActionsUtils.ts:378) really has no isDeletedAction filter, but DEWAPPROVEFAILED is a backend-written system action no user can delete. Add it only for symmetry, not as part of the fix.

4. You missed automaticAction. The next step only renders for manual failures (NextStepUtils.ts:379-380). With approver-only gating an auto-approval block still gives the approver a GBR while the next step falls back to the normal workflow message, so the GBR/next-step mismatch does not go away, it just narrows from "wrong person" to "wrong failure type". Please state whether the GBR should also require !automaticAction.

5. You missed a side effect of the early return. The DEW check sits above getIOUReportActionWithBadge, so an approver who would otherwise get HAS_CHILD_REPORT_AWAITING_ACTION + ACTION_BADGE.APPROVE gets a bare green dot instead. Once the branch is approver-only that is the only population reaching it, so this needs a deliberate decision rather than being inherited by accident.

6. Your owner recommendation contradicts your own evidence. buildOptimisticFixIssueNextStep sets actorAccountID: ownerAccountID with WAITING_TO_FIX_ISSUES (NextStepUtils.ts:315-321), so the copy names the owner as the person who fixes. That points at owner-gets-the-GBR, approver-sees-the-next-step, not approver-only. Note your repro cannot distinguish the two: in a self-approval failure ownerAccountID === managerID, so it needs a three-account test where owner, approver and bystander are distinct.

Please update the proposal with the approver/owner question resolved against point 6, and with 4 and 5 answered explicitly.

[MelvinBot]

Proposal (updated)

Points 1–3 accepted. Point 6 I want to push back on — actorAccountID is a rendering field, not an audience gate, and it entered the DEW branch through a mechanical refactor. Approver-only stands. Point 4: no, don't add !automaticAction. Point 5: your conclusion is right but the branch is wrong — nothing is lost via getIOUReportActionWithBadge.

What is the root cause of that problem?

Unchanged: ReportUtils.ts:4492-4501 returns HAS_DEW_APPROVE_FAILED for any SUBMITTED report carrying an active DEWAPPROVEFAILED, with no test on the current user, and it runs first so it short-circuits every correctly-gated check below it.

What changes do you think we should make in order to solve the problem?

1. Gate on isReportManager(optionOrReport, currentUserAccountID) (ReportUtils.ts:1559) — accepted. No cast needed: ReportUtils.ts:4535 already passes optionOrReport to a OnyxEntry<Report> parameter.
2. Move the block below the archive guard at ReportUtils.ts:4510 — unchanged.
3. Dropped — agreed, DEWAPPROVEFAILED is backend-written and undeletable. Not part of this fix.
4. No !automaticAction on the GBR. See below.
5. Keep the DEW branch's precedence and its bare green dot, deliberately. See below.
6. Approver-only. The owner does not get the GBR. See below.

Tests: add a case where owner, approver and a bystander are three distinct accounts, asserting only the approver gets HAS_DEW_APPROVE_FAILED. Add a case pinning point 5: the fixture at ReportUtilsTest.ts:11428-11469 (hasParentAccess: false, managerID: currentUserAccountID, asserts ACTION_BADGE.APPROVE) plus a DEWAPPROVEFAILED action is the exact repro. The existing DEW test at ReportUtilsTest.ts:11471-11505 uses createRandomReport(70000), which leaves managerID unset and ownerAccountID: 70000 against currentUserAccountID = 5 — it needs managerID: currentUserAccountID to keep passing.

---

4. Should the GBR require !automaticAction? — No.

An auto-approval block is still actionable by the approver, so the green dot is correct there.

• canApproveIOU has no DEW_APPROVE_FAILED check — the Approve button stays enabled.
• approveMoneyRequest deliberately skips the optimistic status update for DEW policies (ReportWorkflow.ts:571-588), so the report sits in SUBMITTED waiting on a human.
• [Payment due @Eskalifer1] fix: don't show DEW approve error next step for auto-approval failures #96198's own test step 6 is "Verify the Approve button is still available so A can manually approve."

So the next step falling back to "Waiting for you to approve" is not a mismatch — GBR and next step agree: approve this. The defect in this issue is an actionless GBR; the auto-approval case is not actionless. Adding !automaticAction would strip the dot from a report that genuinely needs the approver, and for any approver with hasParentAccess set (the common case) the DEW branch is the only thing lighting that row at all — they'd get no signal whatsoever.

Nothing in App models a backend retry (isActionThatSupersedesDEWApproveFailure only reads superseding actions). Whether DEW re-runs auto-approval is backend behaviour I can't confirm from this repo — if it does, this answer flips.

5. The early-return side effect — right conclusion, wrong branch

HAS_CHILD_REPORT_AWAITING_ACTION + ACTION_BADGE.APPROVE is unreachable for the report object that hits the DEW branch:

• getIOUReportActionWithBadge finds its action by scanning for REPORTPREVIEW actions (ReportWorkflow.ts:406-410), which live on the chat report (ReportUtils.ts:8232-8253). The DEW branch requires statusNum === SUBMITTED, which a policy expense chat never is. Each report gets its own independent pass (reportAttributes.ts:604-615), so the chat row's Approve pill is computed separately and is untouched.

What is actually lost is IS_WAITING_FOR_ASSIGNEE_TO_COMPLETE_ACTION + assigneeBadge, and only when hasParentAccess === false (ReportUtils.ts:4429). For every other approver the function would otherwise return null.

Decision: keep DEW first, keep the bare dot. Reordering below the badge branches is not a cosmetic swap — useOptimisticNextStep.ts:78-82 reads gbrResult.reason === HAS_DEW_APPROVE_FAILED and gbrResult.reportAction, so demoting the branch silently kills the "fix the issues" next step. The DEW branch also supplies the DEWAPPROVEFAILED action as reportAction, so clicking the row deep-links to the failure message, which beats an "Approve" pill that will just fail again on a manual block. If you'd rather not lose the pill, the alternative is adding actionBadge: ACTION_BADGE.APPROVE to the DEW return rather than reordering — but I'd skip it: it reads as "this will work" on exactly the reports where approval was just rejected.

6. Approver vs owner — I don't think the evidence points at the owner

actorAccountID decides whose name appears in the sentence, not who sees it. buildNextStepMessage compares it against the viewer's currentUserAccountID to pick "you" vs a display name — it was added by #86503, "Fix next step showing 'you' to admin for scan-failed expenses", a display fix.

The audience gate is separate and explicit: isCurrentUserTheApprover (useOptimisticNextStep.ts:80), required by NextStepUtils.ts:375. The owner falls through to the backend nextStep.

buildOptimisticFixIssueNextStep is a generic shared builder with four call sites (useOptimisticNextStep.ts:75, :82, :89, and NextStepUtils.ts:349). It entered the DEW branch via #97202 "Remove deprecated nextstep format", which swapped a bespoke alert for the generic builder and left the approver gate untouched. So ownerAccountID there is inherited plumbing. Every PR that actually touched this audience scoped it to the approver — #96198: "The red error next step should only appear for manual approval failures when the current user is the approver."

The structural argument agrees. DEW submit failed is already owner-gated, and it is a different signal — an RBR error key, isReportOwner(report) && statusNum === OPEN (ReportUtils.ts:10104-10110). There is no dewApproveFailed error key anywhere in src/. The owner fixes what the owner submits; the approver retries what the approver approves.

Where I think you're right: the copy genuinely names the owner as the fixer, and the owner currently gets no signal at all once we ship this. I'd call that a copy/coverage gap rather than a reason to move the GBR. If product wants the owner involved, the right shape is the owner-side RBR analogue mirroring dewSubmitFailed — not handing the owner the approver's green dot. I'd file that separately rather than bundle it here.

Testing

Accepted — the self-approval repro can't distinguish, since ownerAccountID === managerID there. The three-account matrix (owner A, approver B, bystander C, all distinct) is the discriminating test, and the unit tests above pin it deterministically. I have not reproduced this in a live session: forcing a backend DEW approve rejection needs a real Dynamic External Workflow integration, which isn't something I can stand up in a test account.

What alternative solutions did you explore? (Optional)

Unchanged — filtering in SidebarUtils.shouldDisplayReportInLHN is the wrong layer (requiresAttention also feeds the To-do tab and Search), and narrowing the Auth push audience would hide legitimate report history.

Confidence and what I did not verify

High on points 1, 2, 3, 5, 6 — all statically traced on bb06aee, plus PR bodies for #79165, #86503, #96198, #97202.

Medium on point 4. It rests on the App showing no retry path and the Approve button staying enabled. Whether the DEW processor re-runs auto-approval server-side lives in Auth/Web-Expensify and would need a separate look; if it does retry, !automaticAction becomes the right call.

Not verified: what the backend writes into report.nextStep for a SUBMITTED + DEW-approve-failed report, and how common hasParentAccess === false is for DEW approvers (that decides whether point 5's regression is frequent or a corner case).

Still related: #100123 — same function, the very next branch (ReportUtils.ts:4503-4508), also ungated on the current user and also above the archive guard.

---

Next Steps for Contributor+ team:
To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/100863#issuecomment-5665754131) to create a draft PR.
To refine: @MelvinBot <your feedback>
To reject: Explain why you are rejecting Melvin's proposal.

---

view run · no recording available

[Krishna2323]

@MelvinBot, accepting 5 and 6, both confirmed.

On 4: I agree with the reasoning, but we need to confirm one thing first: what is managerID on a SUBMITTED DEW report when auto-approval fails? approveMoneyRequest doesn't optimistically set it for DEW, and the actual approver comes through DYNAMICEXTERNALWORKFLOWROUTED. If managerID isn't reliably the approver when automaticAction: true fails, isReportManager won't work as the gate. Please confirm this and add an automaticAction: true case to the three-account matrix.

Two smaller things:

• Specify whether the moved block goes before or after hasUnresolvedCardFraudAlert.
• Clarify what happens to isCurrentUserTheApprover in useOptimisticNextStep.ts:80. If the gate is inside the reason, it may be redundant.

[MelvinBot]

Proposal (updated)

Answering the three open items. Line anchors below are on c85bcf4 — the DEW block is now ReportUtils.ts:4501-4510, the fraud-alert check :4512-4517, the archive guard :4519.

managerID on a SUBMITTED DEW report — can't be confirmed from App, but the gate holds either way

Your premise about DYNAMICEXTERNALWORKFLOWROUTED is the part I'd correct: it can't be the source of the approver, because the App synthesizes it itself. withDEWRoutedActionsArray derives it client-side from the SUBMITTED/FORWARDED action (getDynamicExternalWorkflowRoutedAction:1635-1649), and its originalMessage is only {to: string, message?: string} (OriginalMessage.ts:1158-1165) — an email, no accountID, consumed only to render the sentence (ReportActionsUtils.ts:4722-4728).

So managerID is the only approver identity the App has, and it comes straight from the backend — you're right that the App never writes it for DEW (submitReport skips it, approveMoneyRequest skips it).

Two reasons the gate is still correct:

1. The App already bets on it. canApproveIOU:194-195 gates the Approve button on managerID === currentUserAccountID with no DEW exception, and getActionTypeForAssigneeToComplete:4438 uses isReportManager. If managerID weren't the routed approver on a SUBMITTED DEW report, no DEW approver would ever see an Approve button — which contradicts [Payment due @Eskalifer1] fix: don't show DEW approve error next step for auto-approval failures #96198's own test step "Verify the Approve button is still available so A can manually approve." Same for timing: the branch needs statusNum === SUBMITTED, which for DEW only ever arrives in a backend push, so the post-routing report state is already written by the time the branch can fire.
2. It fails closed. If managerID is ever unset at that moment, isReportManager is false for everyone and nobody gets the dot — a missed dot for the approver, not a wrong dot for a bystander. The approver still has the failure in report history and the settlement button's own failed state (AnimatedSettlementButton.tsx:41,65,83). That's strictly better than today's fail-open, and the Approve button already has the identical exposure.

Not confirmable in this repo — what the backend writes to report.managerID when routing and an auto-approval block land together is backend behaviour and needs an internal check. If it turns out the write is deferred past the auto-approval attempt, the consequence is narrow: the approver loses the dot in the automaticAction: true case only, and the fix is resolving the approver from the routed email — not un-gating the branch. I'd ship the gate and cover it with the managerID-unset row in the matrix below rather than block on it.

Where the block goes — I'd rather not move it

Leave it where it is (before hasUnresolvedCardFraudAlert) and add !isReportArchived to its own condition. Physically moving it below the archive guard forces it after the fraud check, which changes two things we just agreed to keep:

• fraud alert would outrank DEW on a report carrying both;
• because useOptimisticNextStep.ts:79-81 keys off gbrResult.reason === HAS_DEW_APPROVE_FAILED, such a report would silently lose the "fix the issues" next step for the approver — the same demotion hazard as point 5.

The inline !isReportArchived closes the archived leak with zero precedence change. If you'd still prefer the move, then: after hasUnresolvedCardFraudAlert, immediately below the guard — and the precedence flip plus next-step loss go in the PR description.

isCurrentUserTheApprover — redundant, remove it

It's literally moneyRequestReport?.managerID === accountID (useOptimisticNextStep.ts:80) — the same comparison as isReportManager(report, accountID) (ReportUtils.ts:1560). Once the gate is in the reason, hasDEWApproveFailed implies it.

Drop the param from shouldShowDynamicExternalWorkflowApproveErrorNextStep and the local at :80, and update the JSDoc to say the approver check now lives in the reason. The function keeps the automaticAction filter, which has no equivalent in the reason. Two independently-computed copies of the same gate is exactly how the GBR and the next step drifted apart.

Test fallout: NextStepUtilsTest.ts:841-871 — the four surviving cases lose their third argument, and :865-867 ("returns false when the current user is not the approver") is replaced by the bystander row below.

Three-account matrix

Owner A, routed approver B (managerID), bystander C — all distinct. Report SUBMITTED, one active DEWAPPROVEFAILED.

DEWAPPROVEFAILED	Viewer	reason	Next step
automaticAction absent/false	A	no DEW reason	backend nextStep
	B	HAS_DEW_APPROVE_FAILED	"Waiting for A to fix the issues"
	C	no DEW reason	backend nextStep
automaticAction: true	A	no DEW reason	backend nextStep
	B	HAS_DEW_APPROVE_FAILED (intentional — point 4)	backend nextStep ("waiting for you to approve")
	C	no DEW reason	backend nextStep
either, report archived	A / B / C	null	—
either, managerID unset	A / B / C	no DEW reason	backend nextStep

The automaticAction: true / B row is the one that pins point 4 — it must assert the reason is returned, so nobody later "fixes" it by adding !automaticAction. The managerID-unset row pins the fail-closed behaviour above.

Plus the two tests from the last round: the precedence test (hasParentAccess: false + managerID: currentUserAccountID + DEWAPPROVEFAILED → HAS_DEW_APPROVE_FAILED, not IS_WAITING_FOR_ASSIGNEE_TO_COMPLETE_ACTION), built from the fixture at ReportUtilsTest.ts:11428-11469; and the fixup to ReportUtilsTest.ts:11472-11505, which uses createRandomReport(70000) and needs an explicit managerID: currentUserAccountID to keep passing.

Everything else from the previous revision stands: gate on isReportManager, points 3 and 4 as answered, keep DEW's precedence and its bare green dot, approver-only.

Confidence

High on the routed-action finding, the redundancy of isCurrentUserTheApprover, the placement trade-off, and the test fallout — all statically traced on c85bcf4.

Medium on the managerID contract itself. The internal-consistency argument (Approve button + #96198's test step) is strong but indirect; the direct confirmation is backend-side and not visible from this repo. Also not verified: what the backend puts in report.nextStep for a SUBMITTED + DEW-approve-failed report, which is what the automaticAction: true / B row falls back to.

---

Next Steps for Contributor+ team:
To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/100863#issuecomment-5679987230) to create a draft PR.
To refine: @MelvinBot <your feedback>
To reject: Explain why you are rejecting Melvin's proposal.

---

view run · no recording available

[Krishna2323]

This proposal looks good to me!

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @puneetlath, see https://stackoverflow.com/c/expensify/questions/7972 for more details.