fix(hooks): prevent placing an order with stale appData after adding a hook by tenderdeve · Pull Request #7908 · cowprotocol/cowswap

tenderdeve · 2026-07-27T07:48:34Z

Summary

When a pre/post-hook is added, appData is rebuilt asynchronously (AppDataInfoUpdater → buildAppData). If the user was quick to open the confirm modal and place the order, the trade flow captured the previous appData doc — built before the hook existed — so the placed order was missing the hook.

Fix

Track the hooks the current appDataInfo was built from in a new appDataBuiltWithHooksAtom, set alongside every appDataInfo update.
Add useIsAppDataHooksInSync() comparing the latest hooks against the ones the current appData was built from.
Gate useTradeFlowContext on that check, so the order can't be placed until appData reflects the added hook. The window is just the async rebuild (a few ms), after which placement re-enables normally.

Testing

Enable hooks, add a pre/post-hook, immediately open confirm and place — the hook is now always included.
Normal (no-hook) flow unaffected: hooks are undefined on both sides, so the check is always in sync.

Summary by CodeRabbit

Bug Fixes
- Improved trade flow reliability by waiting for app configuration to finish synchronizing.
- Ensured trade actions use the latest configuration after updates.
- Prevented outdated background updates from replacing newer configuration.
- Improved behavior when app data is unavailable, rebuilding, or encounters an error.
- Added safeguards for synchronization checks when configuration is missing or still updating.

…a hook When a pre/post-hook is added, appData is rebuilt asynchronously. Placing the order before that rebuild finished used a stale appData doc, so the hook could be missing from the order. Track the hooks the current appData was built from and block the trade flow context until they match the latest hooks, so placement always includes the added hook. Fixes cowprotocol#7872

vercel · 2026-07-27T07:48:41Z

@tenderdeve is attempting to deploy a commit to the cow-dev Team on Vercel.

A member of the Team first needs to authorize it.

coderabbitai · 2026-07-27T07:49:06Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b5cac723-492e-438e-99d8-c603adb1aa8f

📥 Commits

Reviewing files that changed from the base of the PR and between cd56bbc and fbaefaa.

📒 Files selected for processing (1)

apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts

🚧 Files skipped from review as they are similar to previous changes (1)

apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts

Walkthrough

The change tracks the hooks used to build current app data, exposes a synchronization check, and blocks trade-flow activation until current hooks match the latest app-data build.

Changes

App-data synchronization

Layer / File(s)	Summary
Track hooks used for app-data builds `apps/cowswap-frontend/src/modules/appData/state/atoms.ts`, `apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts`, `apps/cowswap-frontend/src/modules/appData/hooks.ts`, `apps/cowswap-frontend/src/modules/appData/index.ts`	Stores the hooks used during asynchronous app-data builds. Prevents stale builds from updating state. Exports `useIsAppDataHooksInSync`, which compares the built hooks with the current hooks.
Gate trade-flow activation `apps/cowswap-frontend/src/modules/tradeFlow/hooks/useTradeFlowContext.ts`	Requires synchronized app-data hooks before enabling the trade-flow context.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AppDataInfoUpdater
  participant AppDataAtoms
  participant TradeFlowContext
  AppDataInfoUpdater->>AppDataAtoms: record hooks used for current app-data build
  TradeFlowContext->>AppDataAtoms: compare current hooks with built hooks
  AppDataAtoms-->>TradeFlowContext: return synchronization status
  TradeFlowContext->>TradeFlowContext: activate only when synchronized

Suggested reviewers: kernelwhisperer, shoom3301

Poem

A rabbit tracks hooks through each app-data run,
It waits for the build before orders are done.
When current and built hooks match in a row,
The trade flow is ready to go. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes preventing orders from using stale appData after a hook is added.
Description check	✅ Passed	The description includes the issue, implementation summary, and focused testing steps, although optional background and self-check sections are omitted.
Linked Issues check	✅ Passed	The changes address issue `#7872` by blocking order placement until appData includes the latest pre- or post-hooks.
Out of Scope Changes check	✅ Passed	The changes remain within scope and support reliable appData synchronization for hook-enabled order placement.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts`:
- Around line 84-88: Update the buildAppData success/failure handling in
AppDataInfoUpdater so setAppDataBuiltWithHooks(typedHooks) runs only after a
successful build. When buildAppData throws and appDataInfo falls back to
getAppData(), reset the built-hooks marker to undefined instead of recording
typedHooks.
- Around line 84-88: Update the async updater around buildAppData to capture the
current execution/request ID before awaiting it, then only perform
setAppDataInfo and setAppDataBuiltWithHooks when that ID is still current. Apply
the same guard in the finally path before recording typedHooks, reusing an
existing request ID rather than creating a timestamp.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 31449099-6998-4f52-8d99-8b98edd5822f

📥 Commits

Reviewing files that changed from the base of the PR and between 37e9cff and 4900648.

📒 Files selected for processing (5)

apps/cowswap-frontend/src/modules/appData/hooks.ts
apps/cowswap-frontend/src/modules/appData/index.ts
apps/cowswap-frontend/src/modules/appData/state/atoms.ts
apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts
apps/cowswap-frontend/src/modules/tradeFlow/hooks/useTradeFlowContext.ts

vercel · 2026-07-27T12:55:28Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
swap-dev	Ready	Preview	Jul 31, 2026 12:57pm

elena-zh

Hey @tenderdeve , the app preview crashes on opening

deepEqual calls Object.keys(), which throws on the undefined appData hooks atoms present on first render, crashing the app on load. Treat both-undefined (or same reference) as in sync and one-sided undefined as out of sync before falling back to the deep comparison.

tenderdeve · 2026-07-31T10:13:56Z

@elena-zh good catch — the crash was in the new sync check. useIsAppDataHooksInSync compared the two hook atoms with deepEqual, which calls Object.keys(), and on first render both atoms are undefined → Object.keys(undefined) threw and took the page down.

Fixed in 0ea0d89: it now treats both-undefined (no hooks) as in sync and only deep-compares when both sides are present, so there's nothing to throw on load. Order placement is still blocked during the stale window when hooks were actually changed.

elena-zh

It works

shoom3301 · 2026-08-07T09:05:26Z

@tenderdeve I kindly ask you to check the coderabbit comments:
#7908 (comment)

Move the built-with-hooks marker out of the finally block: record typedHooks only after a successful build, and clear it on the fallback path since the default document may omit the newly added hooks. Also guard both writes with a run id so a superseded, slower build can't overwrite fresher appData.

tenderdeve · 2026-08-10T09:04:28Z

@shoom3301 addressed both CodeRabbit comments in cd56bbc.

The built-with-hooks marker was in a finally, so it recorded typedHooks even when buildAppData threw and we fell back to getAppData() — that fallback doc can omit the just-added hook, yet the marker would report in-sync and re-enable order placement. Moved the marker into the success path and clear it to undefined on the fallback.

Also guarded both writes with a run id (latestRunIdRef): since useAsyncEffect doesn't cancel an in-flight build, a slower superseded run could overwrite appDataInfo after a newer run started. Now stale runs skip the writes.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts`:
- Around line 91-94: In the catch block of the app-data updater, replace catch
(e: any) with catch (err: unknown), assign const error = normalizeError(err)
before logging, and pass the normalized error to console.error. Remove the TODO
and eslint suppression, reusing the existing normalizeError utility.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 34c1ac16-18ce-44f3-8852-9052a146db27

📥 Commits

Reviewing files that changed from the base of the PR and between 9d5efa1 and cd56bbc.

📒 Files selected for processing (1)

apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts

tenderdeve · 2026-08-12T06:31:51Z

Addressed the remaining CodeRabbit comment in fbaefaa7d: the catch (e: any) in AppDataInfoUpdater now uses catch (err: unknown) + normalizeError(err), dropping the any suppression and TODO.

fairlighteth · 2026-08-31T14:11:11Z

⚠️

AI Review (Codex GPT-5): cover stale-appData lifecycle edges

Finding: [BLOCKING] The race fix needs current-base validation and regression coverage

This is a valuable fix for the stale-appData race. Before merging, please update from develop and add focused coverage for a failed build, a superseded async build, and rapid add/remove-hook races. After that it needs a fresh technical review plus QA confirmation, since the prior approval predates the latest commits.

Suggested next step

Rebase and add focused tests for failed, superseded, and rapid add/remove update sequences before refreshing technical and QA review.

🤖 Prompt for AI agents

Verify the stale-appData race fix on the current head. Add focused tests for a failed build, a superseded async build, and rapid add/remove-hook transitions; stale work must not overwrite the latest appData.

Generated using the pr-review skill from the CoW Protocol skills repo.

shoom3301 · 2026-09-01T08:32:45Z

@tenderdeve thank you for the contribution!
I think this issue is very important and should be addressed by one of core contributors since the changes are affecting core trading flow.

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

Comment thread apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts Outdated

elena-zh assigned tenderdeve Jul 27, 2026

Merge branch 'develop' into fix/7872-hook-not-added

523a46c

elena-zh requested changes Jul 27, 2026

View reviewed changes

elena-zh and others added 2 commits July 28, 2026 18:06

Merge branch 'develop' into fix/7872-hook-not-added

19e4135

Merge branch 'develop' into fix/7872-hook-not-added

9d5efa1

elena-zh requested a review from shoom3301 July 31, 2026 13:43

elena-zh approved these changes Jul 31, 2026

View reviewed changes

elena-zh requested a review from kernelwhisperer August 5, 2026 15:53

Merge branch 'develop' into fix/7872-hook-not-added

48bf4bd

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread apps/cowswap-frontend/src/modules/appData/updater/AppDataInfoUpdater.ts Outdated

refactor(app-data): normalize caught error in appData updater

fbaefaa

shoom3301 closed this Sep 1, 2026

github-actions Bot locked and limited conversation to collaborators Sep 1, 2026

Conversation

tenderdeve commented Jul 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Testing

Summary by CodeRabbit

Uh oh!

vercel Bot commented Jul 27, 2026

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vercel Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elena-zh left a comment

Choose a reason for hiding this comment

Uh oh!

tenderdeve commented Jul 31, 2026

Uh oh!

elena-zh left a comment

Choose a reason for hiding this comment

Uh oh!

shoom3301 commented Aug 7, 2026

Uh oh!

tenderdeve commented Aug 10, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tenderdeve commented Aug 12, 2026

Uh oh!

fairlighteth commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shoom3301 commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tenderdeve commented Jul 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

vercel Bot commented Jul 27, 2026 •

edited

Loading

fairlighteth commented Aug 31, 2026 •

edited

Loading