Skip to content

chore(deps): upgrade all dependencies (2026-08-10) - #136

Open
devin-ai-integration[bot] wants to merge 39 commits into
developfrom
deps/upgrade-all-2026-08-10
Open

devin-ai-integration[bot] wants to merge 39 commits into
developfrom
deps/upgrade-all-2026-08-10

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 10, 2026

Copy link
Copy Markdown

Summary

Upgrades every outdated npm dependency in the repo that can be upgraded without a breaking-API migration: 80 direct dependencies (18 patch, 26 minor, 36 major), plus a lockfile-only bump of webpack 5.97.1 → 5.109.2. Work is split into one commit per batch (patch/minor) and one commit per major package so any single upgrade can be reverted in isolation.

Notable non-mechanical parts of the diff:

  • axios 0.28.1 → 1.19.0 clears the largest cluster of advisories in the tree (SSRF, prototype-pollution and DoS chains, ~25 GHSAs against 0.x).
  • react / react-dom 18.2.0 → 19.2.8 (with matching @types/react* 19) — verified in the browser: sign-in, XState-driven feed, react-virtualized list and navigation still work.
  • http-proxy-middleware 0.19.1 → 3.0.7 needs the CI proxy server's call site updated, since v3 has no default export and replaced the (context, options) signature:
    - const createProxyMiddleware = require("http-proxy-middleware");
    - createProxyMiddleware(["/login", ..., "graphql"], { target, changeOrigin, logLevel: "debug" })
    + const { createProxyMiddleware } = require("http-proxy-middleware");
    + createProxyMiddleware({ pathFilter: ["/login", ..., "/graphql"], target, changeOrigin })
    Note the old "graphql" entry had no leading slash, so it never matched; "/graphql" is now proxied by scripts/testServer.ts. The frontend calls GraphQL at an absolute backend URL, so nothing depended on the old behaviour.
  • history was attempted at 5.3.0 and reverted: react-router 5 subscribes with history.listen((location, action) => ...) while history 5 passes a single { action, location } update object, so the router stored a non-location and every programmatic navigation (the post-login history.push("/"), every tab click) rendered a blank route. Caught in the browser walkthrough, not by lint/build/tests.
  • react-virtualized 9.22.5 → 9.22.6 lets us delete patches/react-virtualized+9.22.5.patch; the broken WindowScroller ESM import that patch commented out is fixed upstream.
  • react-number-format 4 → 5 needs a call-site change, since v5 drops the default export:
    - import NumberFormat from "react-number-format";
    + import { NumericFormat } from "react-number-format";
    -   <NumberFormat ... isNumericString />
    +   <NumericFormat ... valueIsNumericString />
  • @types/lodash 4.14 → 4.17 types the lodash/fp intersectionWith comparator's second argument as T1 | T2, so the destructured comparators in scripts/seedDataUtils.ts no longer type-check. Both call sites were plain "keep transactions that appear in X" filters, so they are now expressed directly:
    - intersectionWith(({ id: transactionId }, { transactionId: likeTransactionId }) => ..., transactions, seedLikes)
    + transactions.filter(({ id }) => seedLikes.some(({ transactionId }) => isEqual(id, transactionId)))
  • passport 0.5 → 0.7 breaks POST /logout: 0.7's req.logout is asynchronous and regenerates the session, but the route destroyed the session synchronously alongside it, so passport then dereferenced a destroyed session and killed the backend process with TypeError: Cannot read properties of undefined (reading 'regenerate') (it also raced two res.redirects):
    - req.logout(() => res.redirect("/"));
    - req.session!.destroy((err) => res.redirect("/"));
    + req.logout(() => {
    +   req.session!.destroy(() => res.redirect("/"));
    + });
  • express 4 → 5 makes req.query a lazy getter, so middleware that writes into it silently no-ops — express-paginate could no longer install its page/limit defaults (a parameterless GET /transactions/public returned all 38 rows with totalPages: null instead of a page of 10) and the express-validator query sanitizers in backend/validators.ts had no effect. Snapshotting the parsed query as a writable own property restores Express 4 semantics for both:
    + app.use((req, _res, next) => {
    +   Object.defineProperty(req, "query", { value: req.query, writable: true, configurable: true, enumerable: true });
    +   next();
    + });
      app.use(paginate.middleware(+process.env.PAGINATION_PAGE_SIZE!));
  • husky 7 → 9: postinstall now calls husky instead of the deprecated husky install.

Version-range style is preserved per package (exact stays exact, ^ stays ^).

Security fixes

Advisories resolved on direct dependencies (transitive fixes come along via the refreshed lockfile):

Package Advisories
axios 0.28.1 → 1.19.0 GHSA-jr5f-v2jv-69x6 (CVE-2025-27152), GHSA-4hjh-wcwx-xvwj (CVE-2025-58754), GHSA-3p68-rc4w-qgx5 (CVE-2025-62718), GHSA-43fc-jf86-j433 (CVE-2026-25639), GHSA-fvcv-3m26-pcqx (CVE-2026-40175), plus the CVE-2026-42033/42035/42038/42039/42041/42043 prototype-pollution and SSRF chain and GHSA-mmx7-hfxf-jppx / GHSA-7q8q-rj6j-mhjq
passport 0.5.0 → 0.7.0 GHSA-v923-w3x8-wh69 (CVE-2022-25896) session fixation
http-proxy-middleware 0.19.1 → 3.0.7 GHSA-c7qv-q95q-8v27 (CVE-2024-21536), GHSA-64mm-vxmg-q3vj (CVE-2026-55602)
@types/jsonwebtoken 8 → 9 (aligned with jsonwebtoken 9 typings) tracks GHSA-8cf7-32gw-wr33 / GHSA-hjrf-2m68-5959 / GHSA-qwph-4952-7xr6 fixes
morgan 1.10.0 → 1.11.0 GHSA-4vj7-5mj6-jm8m (CVE-2026-5078) log forging
uuid 8 → 14 GHSA-w5hq-g745-h8pq (CVE-2026-41907)
webpack (lockfile) 5.97.1 → 5.109.2 GHSA-8fgc-7cc6-rx7x, GHSA-38r7-794h-5758

npm audit fix --force was not used; every change is an explicit version bump.

Upgrade summary

Patch (18)

Package From To Scope
@mui/lab ^5.0.0-alpha.167 ^5.0.0-alpha.177 prod
@percy/cypress 3.1.6 3.1.9 dev
@types/bcryptjs 2.4.2 2.4.6 dev
@types/bluebird 3.5.36 3.5.42 dev
@types/connect-flash 0.0.37 0.0.40 dev
@types/cors 2.8.12 2.8.19 dev
@types/dinero.js 1.9.0 1.9.4 dev
@types/json-server 0.14.4 0.14.8 dev
@types/lowdb 1.0.11 1.0.15 dev
@types/morgan 1.9.3 1.9.10 dev
@types/passport 1.0.16 1.0.17 dev
@types/react-router 5.1.18 5.1.20 dev
@types/yup 0.29.13 0.29.14 dev
cors 2.8.5 2.8.6 dev
formik 2.4.6 2.4.9 prod
react-calendar ^6.0.0 ^6.0.1 prod
react-virtualized 9.22.5 9.22.6 prod
shortid 2.2.16 2.2.17 prod

Minor (26)

Package From To Scope
@auth0/auth0-react 2.2.4 2.23.0 prod
@babel/core ^7.28.0 ^7.29.7 prod
@babel/plugin-syntax-flow ^7.14.5 ^7.29.7 prod
@babel/plugin-transform-react-jsx ^7.14.9 ^7.29.7 prod
@babel/preset-env ^7.28.0 ^7.29.7 dev
@emotion/styled ^11.11.0 ^11.14.1 prod
@mui/icons-material ^5.15.12 ^5.18.0 prod
@mui/material ^5.15.12 ^5.18.0 prod
@okta/okta-react ^6.7.0 ^6.11.0 prod
@percy/cli ^1.27.4 ^1.32.6 dev
@types/connect-history-api-fallback 1.3.5 1.5.4 dev
@types/express-session 1.18.0 1.19.0 dev
@types/lodash 4.14.181 4.17.25 dev
@types/react-virtualized 9.21.21 9.22.3 dev
@types/validator 13.7.2 13.15.10 dev
@types/webpack-env 1.16.4 1.18.8 dev
aws-amplify ^6.0.16 ^6.20.0 prod
babel-loader ^10.0.0 ^10.1.1 dev
cypress 15.0.0 15.20.0 dev
date-fns 4.1.0 4.4.0 prod
express-session 1.18.0 1.19.0 dev
graphql-http ^1.22.0 ^1.23.0 dev
morgan 1.10.0 1.11.0 dev
passport 0.5.0 0.7.0 dev
prettier ^3.0.0 ^3.9.6 dev
typescript-eslint ^8.46.2 ^8.66.0 dev

Major (36)

Package From To Scope
@cypress/code-coverage ^3.14.5 ^4.0.3 dev
@graphql-tools/graphql-file-loader 7.5.17 8.1.18 prod
@graphql-tools/load 7.8.14 8.1.15 prod
@okta/jwt-verifier ^3.0.1 ^4.0.2 prod
@okta/okta-auth-js ^7.3.0 ^8.0.1 prod
@types/jsonwebtoken 8.5.8 9.0.10 dev
@types/node ^20.11.25 ^26.2.0 dev
@types/react ^18.2.14 ^19.2.18 dev
@types/react-dom ^18.2.6 ^19.2.4 dev
@types/shortid 0.0.29 2.2.0 dev
@types/uuid 8.3.4 10.0.0 dev
axios 0.28.1 1.19.0 prod
bcryptjs 2.4.3 3.0.3 dev
clsx 1.2.1 2.1.1 prod
concurrently 9.1.2 10.0.4 dev
connect-history-api-fallback 1.6.0 2.0.0 dev
cross-env 7.0.3 10.1.0 dev
dotenv 16.0.0 17.4.2 dev
express 4.20.0 5.2.1 dev
fuse.js 6.5.3 7.5.0 dev
graphql 16.8.1 17.0.2 dev
graphql-tools 8.2.7 9.0.33 dev
http-proxy-middleware 0.19.1 3.0.7 dev
husky 7.0.4 9.1.7 dev
jsdom ^22.1.0 ^29.1.1 dev
nodemon 2.0.22 3.1.14 dev
npm ^9.8.0 ^11.19.0 dev
nyc 15.1.0 18.0.0 dev
patch-package ^7.0.0 ^8.0.1 dev
react 18.2.0 19.2.8 prod
react-dom 18.2.0 19.2.8 prod
react-number-format 4.9.4 5.4.5 prod
start-server-and-test 1.14.0 3.0.12 dev
uuid 8.3.2 14.0.1 prod
wait-on ^8.0.3 ^9.1.0 dev
yup 0.32.11 1.7.1 prod

Skipped packages

Each was attempted on its own commit, validated, and reverted when the fix was more than a mechanical change:

Package Target Reason
@mui/material, @mui/icons-material, @mui/lab 9.x MUI ≥6 removes the Grid item / xs API used in 98 places across 19 components; it builds but logs Received true for a non-boolean attribute item at runtime and the layout silently degrades. Kept on latest 5.x (5.18.0) instead.
history 5.x react-router 5.3.4 pins history@^4.9.0 and its Router listener contract; history 5's single-argument listen callback breaks all client-side navigation (verified in the browser). Needs react-router 6+.
react-router, react-router-dom 7.x useRouteMatch (and the v5 route-render API) are gone; TransactionNavTabs/routing need a data-router migration.
xstate, @xstate/react 5.x / 6.x Machine, .withConfig, and the actor API used by all 9 machines were removed in XState 5.
lowdb 7.x ESM-only, no default export; backend/database.ts and the whole db.get(...).value() chain style would have to be rewritten.
dinero.js 2.x v2 has no default export and a completely different (functional) API.
typescript 7.x typescript-eslint refuses to run: "typescript-eslint does not support TS 7.0".
@faker-js/faker 10.x v6 → v10 renames nearly every generator used by the seed scripts (faker.random.*, faker.name.*, internet.userName, phone.phoneNumberFormat, helpers.randomize), which would rewrite most of scripts/seedDataUtils.ts and change committed seed data.
express-validator 7.x Drops sanitizeQuery used in backend/validators.ts.
express-jwt 8.x v8 changes the export shape (expressjwt), so backend/helpers.ts no longer compiles.
jwks-rsa 4.x Bundles no express-jwt typings and requires express-jwt 8 (see above).
@types/express, @types/express-serve-static-core 5.x Express 5 typings break the current route handler signatures in backend/user-routes.ts. Runtime express itself was upgraded to 5.2.1.
eslint, @eslint/js, eslint-plugin-cypress 10.x / 7.x ESLint 10's new preserve-caught-error rule fails on existing catch blocks — a lint-debt cleanup, not a dependency bump.
@testing-library/jest-dom, @testing-library/react 7.x / 16.x Requires an explicit @testing-library/dom dependency and React 19 testing setup changes; unit suite fails to resolve it.
vite, @vitejs/plugin-react, vite-plugin-istanbul 8.x / 6.x / 9.x vite-plugin-istanbul 9 is ESM-only and cannot be required from the CJS vite.config.ts/cypress.config.ts setup.
vitest 4.x Requires Vite 8 (see above); yarn install cannot link it against Vite 7.
detect-port 2.x v2 ships ESM that imports node util.debuglog, which breaks the browser bundle (it is imported from front-end code).
graphql 17 Upgraded (with graphql-tools 9 and @graphql-tools/* 8) — listed here only to note the GraphQL stack moved as one unit.

Source impact

  • Production / runtime: axios, react/react-dom, react-number-format, react-virtualized, uuid, clsx, date-fns, formik, yup 1, @mui/* 5.18, aws-amplify, @okta/*, @auth0/auth0-react, @graphql-tools/*. Backend runtime: express 5, passport 0.7, morgan, express-session, http-proxy-middleware, connect-history-api-fallback, bcryptjs 3, fuse.js 7, graphql 17.
  • Dev-only / tooling (no shipped-code impact): all @types/*, cypress, @cypress/code-coverage 4, @percy/*, prettier, typescript-eslint, nyc, jsdom, nodemon, concurrently, cross-env, dotenv, wait-on, start-server-and-test, husky, patch-package, npm, babel-loader, @babel/*.
  • Only six source files changed: src/setupProxy.js (http-proxy-middleware v3), scripts/seedDataUtils.ts (lodash typings), src/components/TransactionCreateStepTwo.tsx (react-number-format v5), backend/auth.ts (passport 0.7 logout), backend/app.ts (Express 5 req.query), and prettier 3.9 reformatting in src/machines/* and src/models/notification.ts.

Validation

  • yarn lint (eslint + prettier --check): pass
  • yarn types / yarn build (tsc --noEmit then vite build): pass
  • yarn test:unit:ci (vitest): 44 passed, 10 skipped
  • yarn db:seed regenerates seed data successfully with the upgraded faker/lodash stack
  • Baseline on develop was verified green before any upgrade, and each batch/major was validated independently before it was committed

Frontend verification

Recorded browser walkthrough against yarn dev (Vite on :3000, Express API on :3001) with the seeded database, as the seeded user Heath93:

walkthrough

  • Sign-in → redirect renders a populated transaction feed; EVERYONE / FRIENDS / MINE tabs all navigate and re-render
  • New payment of $25: yup 1 validation blocks an empty note, NumericFormat renders $25, submit → Transaction Submitted!, balance $1,509.53$1,484.53, and the payment appears first in the MINE feed
  • Logout returns to the sign-in page and re-login succeeds, proving the passport 0.7 crash above is gone (backend PID stayed up across every logout, no regenerate error, no nodemon restart)
  • Feed loads exactly 10 rows and paginates to ~28 on scroll; amount and date-range filters return correctly-filtered, non-empty sets with no 422s, cross-checked against the API responses
  • yarn start:ci starts cleanly on http-proxy-middleware 3; with BACKEND_PORT=3001, /login, /checkAuth and /graphql proxy to the backend while / and /personal fall back to the SPA

Full results, per-check evidence and the verification recording for the backend fixes are in this comment.

Pre-existing console noise unaffected by this PR: React "Invalid DOM property stop-color" warnings from src/components/SvgCypressLogo.tsx, xstate empty-string-transition deprecation warnings, and HTTP 410s from the retired avatars.dicebear.com avatar URLs used by the seed data.

Pre-existing issue, not fixed here: src/setupProxy.js reads process.env.BACKEND_PORT, which is defined nowhere in the repo (.env only has VITE_BACKEND_PORT), so yarn start:ci proxies to localhost:undefined and returns 504 unless the variable is supplied. develop has the same bug; worth a separate fix.

Link to Devin session: https://app.devin.ai/sessions/5146de38ebb147cfbda93ad807a68c4d
Requested by: @dr-phil


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)
Open in Devin Review

devin-ai-integration Bot and others added 30 commits August 10, 2026 14:07
Drops the obsolete react-virtualized 9.22.5 patch; the broken WindowScroller import is fixed upstream in 9.22.6.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>
Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>
Reformats a few files to satisfy prettier 3.9.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>
@types/lodash 4.17 types the fp intersectionWith comparator's second argument as a union of both list element types, so the destructured comparator no longer type-checks.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>
devin-ai-integration Bot and others added 7 commits August 10, 2026 14:40
…ls/load@8.1.15 @graphql-tools/graphql-file-loader@8.1.18
MUI v9 drops the Grid item/xs API used in 98 places across 19 components; migrating is a large refactor out of scope for a dependency bump.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>
v5 exports NumericFormat instead of a default export and renames isNumericString to valueIsNumericString. Also switches the husky postinstall to the husky 9 entrypoint.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 5 potential issues.

Open in Devin Review

Comment thread package.json Outdated
Comment thread package.json
"http-proxy-middleware": "0.19.1",
"husky": "7.0.4",
"graphql-tools": "9.0.33",
"http-proxy-middleware": "3.0.7",

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: Proxy path filter now actually matches GraphQL requests

The old context list contained "graphql" without a leading slash; http-proxy-middleware matches string filters as a prefix of the pathname (pathname.indexOf(context) === 0), so /graphql never matched and was never proxied by scripts/testServer.ts. The new "/graphql" entry changes that in CI (start:react:proxy-server / start:ci). In practice the frontend calls GraphQL with an absolute http://localhost:${backendPort}/graphql URL (src/machines/bankAccountsMachine.ts:53), so nothing should regress — but this is a genuine behavior change of the CI proxy server, not a pure rename.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 6a4c6a6src/setupProxy.js now destructures createProxyMiddleware and uses pathFilter, and logLevel is dropped. On the "graphql""/graphql" change: agreed it is a behavior change rather than a rename, and it is the intended pattern (the old entry silently matched nothing). The frontend calls GraphQL at an absolute backend URL, so nothing depends on the proxy for it. yarn start:ci / scripts/testServer.ts is being verified now.

Comment thread package.json
"eslint": "^9.38.0",
"eslint-plugin-cypress": "^5.2.0",
"express": "4.20.0",
"express": "5.2.1",

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Runtime express upgraded to 5 while @types/express stays on 4.x

express moves to 5.2.1 but @types/express (4.17.21) and @types/express-serve-static-core (4.17.2) remain pinned to v4 (the description says v5 typings break the handler signatures). This means the compiler validates the backend against Express 4 semantics while the app runs Express 5 — so genuinely breaking API differences (removed req.param, res.sendfile, path-to-regexp v8 route syntax, error-handling of rejected promises) will not be caught by yarn types. I scanned backend/*.ts and found no removed APIs or */optional-param route patterns in the app's own routes, but note backend/helpers.ts:94-96 uses unless({ path: ["/testData/*"] }) in the auth-provider code paths, whose matching semantics under the newer stack are untested by CI.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Being verified against a running server now (pagination on the feeds plus the sanitizeQuery path). If either turns out to be broken I will revert express to 4.20.0 rather than pull express-validator 7 / replace express-paginate in a dependency-bump PR.

Comment thread scripts/seedDataUtils.ts
Comment on lines +444 to 446
transactions.filter(({ id }) =>
seedLikes.some(({ transactionId }) => isEqual(id, transactionId))
);

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: intersectionWith → filter/some rewrite is behaviour-equivalent here

lodash/fp intersectionWith(comparator, transactions, seedLikes) returns elements of the first array that match any element of the second, with de-duplication performed via the same comparator. Because the comparator reads transactionId off the second argument, the internal "already seen" check always compares against a value with transactionId === undefined and never matches, so no de-duplication actually occurred. The new transactions.filter(({ id }) => seedLikes.some(...)) therefore produces the same array (same order, same duplicates) for both the likes and comments call sites.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, and intentional — the inputs are shortid-keyed and unique, so the de-dup is a no-op here. Kept the simpler form rather than reaching for uniqBy.

import NumberFormat from "react-number-format";
import { NumericFormat } from "react-number-format";
import { Formik, Form, Field, FieldProps } from "formik";
import { string, object, number } from "yup";

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 yup 0.32 → 1.x affects five form schemas, only one file touched

Only src/components/TransactionCreateStepTwo.tsx is in the diff, but yup 1 changes behavior for schemas in BankAccountForm.tsx, CommentForm.tsx, UserSettingsForm.tsx, SignUpForm.tsx and SignInForm.tsx too (mixed<T>().oneOf, ref() cross-field comparison, empty-string→NaN typeError precedence for number().required()). Nothing in those schemas is obviously broken under v1, and Formik 2.4 supports yup 1, but validation-message assertions in the Cypress e2e suite are the only real coverage for these paths.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Being checked in the browser now, including the empty-amount message on TransactionCreateStepTwo under validateOnMount. Will report back here with the result.

…iddleware 3

react-router 5 subscribes with history.listen((location, action) => ...) while history 5 passes a single { action, location } update, so the router stored a non-location and every programmatic navigation rendered a blank route. http-proxy-middleware 3 drops the default export and the (context, options) signature in favour of pathFilter.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

Open in Devin Review

Comment thread package.json
Comment on lines +42 to +45
"react": "19.2.8",
"react-calendar": "^6.0.1",
"react-dom": "19.2.8",
"react-number-format": "5.4.5",

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: react-virtualized under React 19 — findDOMNode removal risk

React 19 removes ReactDOM.findDOMNode, which several react-virtualized modules (notably WindowScroller) still call. src/components/TransactionInfiniteList.tsx only imports InfiniteLoader/List, which do not call it, so the transaction feed should be unaffected; the failure would only appear if WindowScroller were ever used. Flagging so the constraint is recorded, since 9.22.6 is not a React-19-aware release.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed on the peer ranges. InfiniteLoader/List are the only react-virtualized imports and neither touches findDOMNode; verified in the browser that the virtualized feed renders and scrolls under React 19. MUI stays on 5.x because MUI ≥6 drops the Grid item/xs API this app uses in ~98 places.

Comment thread package.json
Comment on lines +89 to +94
"@types/react-virtualized": "9.22.3",
"@types/shortid": "2.2.0",
"@types/uuid": "10.0.0",
"@types/validator": "13.15.10",
"@types/webpack-env": "1.18.8",
"@types/yup": "0.29.14",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: Stale @types packages now shadowed by bundled typings

yup 1.x, uuid 14, shortid 2.2.17 and bcryptjs 3 all ship their own typings, yet @types/yup 0.29.14, @types/uuid 10, @types/shortid 2.2.0 and @types/bcryptjs 2.4.6 are retained (some merely bumped). TypeScript prefers the package's own types field, so these @types/* entries are dead weight that can drift out of sync with the runtime API; they are candidates for removal in a follow-up rather than a bump.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed — leaving the removals for a follow-up so this PR stays a pure version bump. Note @types/yup is still load-bearing for one import in the current source, so it cannot be dropped blindly.

Comment thread package.json
"codesandbox:start:api": "yarn tsnode:not-instrumented --files backend/app.ts",
"codesandbox:start": "NODE_ENV=development TSC_COMPILE_ON_ERROR=true concurrently \"vite\" yarn:start:api:codesandbox",
"postinstall": "husky install && patch-package"
"postinstall": "husky && patch-package"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: Leftover husky v4 config block after moving to husky 9

The postinstall script now uses husky (v9 style), but the top-level "husky": { "hooks": { "pre-push": "yarn types" } } block at package.json:199-203 is husky v4 configuration that has been ignored since v7, and .husky/ contains only a .gitignore — so no pre-push hook is actually installed. Worth either porting the hook to .husky/pre-push or deleting the dead config while touching husky.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Correct, the v4 husky block has been dead since v7. Leaving it (and the missing .husky/pre-push) alone here so this stays a dependency bump — the postinstall change was only to silence the husky install deprecation warning. Worth a follow-up to port pre-push: yarn types to .husky/pre-push.

…ort 0.7 and express 5

passport >=0.6 made req.logout async (session.save then session.regenerate), so destroying the session synchronously afterwards threw inside a processImmediate and killed the process. Express 5 exposes req.query as a lazy getter, so express-paginate's defaults and the express-validator query sanitizers were silently dropped; the query object is redefined as a writable property before those middlewares run.

Co-Authored-By: Phil Bedford <phil.bedford@cognition.ai>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

Open in Devin Review

Comment thread backend/app.ts
Comment on lines +69 to +79
// Express 5 exposes req.query as a lazy getter, so middleware that writes into it
// (express-paginate defaults, express-validator query sanitizers) has no effect.
app.use((req, _res, next) => {
Object.defineProperty(req, "query", {
value: req.query,
writable: true,
configurable: true,
enumerable: true,
});
next();
});

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Query snapshot middleware depends on Express 5's query getter staying configurable

The shim replaces the prototype-level lazy query getter with a per-request own data property. Two things worth confirming against the installed express 5.2.1: (1) the getter must not already be materialized as a non-configurable own property (otherwise Object.defineProperty throws for every request), and (2) Express 5 changed the default query parser from extended (qs) to simple (querystring.parse), which returns null-prototype objects and no longer supports nested/bracket params. The snapshot is taken with whatever parser is active, so any code that assumes qs semantics (e.g. isEmpty(req.query) / lodash helpers in backend/transaction-routes.ts:102-103 and backend/database.ts query filters) now operates on a plain, flat, null-prototype object. Worth a quick runtime smoke test of the paginated transaction endpoints.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Verified at runtime against express 5.2.1 on the running app.

Object.defineProperty does not throw: express 5 defines query as a configurable getter on the request prototype (defineGetter(req, 'query', ...) in lib/request.js), so replacing it with a per-request own data property is legal, and every request in the smoke tests below went through the shim without error.

On the query parser: app.get('query parser') reports simple under 5.2.1, but the app only ever reads flat scalar params (page, limit, q, dateRangeStart/dateRangeEnd, amountMin/amountMax), so the loss of bracket/nested support is not reachable. isEmpty/pick/omit from lodash all work on null-prototype objects.

Runtime smoke test of the paginated + filtered endpoints (logged in as a seeded user):

GET /transactions/public                                    -> {page:1, limit:10, totalPages:5, hasNextPages:true}   n=10
GET /transactions/public?page=2&limit=5                     -> {page:2, limit:5,  totalPages:8, hasNextPages:true}   n=5
GET /transactions/public?amountMin=1&amountMax=100000       -> {page:1, limit:10, totalPages:5}                      n=10
GET /transactions/public?amountMin=1&amountMax=100000&page=2&limit=3 -> {page:2, limit:3, totalPages:13}              n=3
GET /transactions/public?dateRangeStart=...&dateRangeEnd=... -> {page:1, limit:10, totalPages:5}                      n=10

Before the shim the parameterless request returned all 38 rows with totalPages: null, so the amount/date filter paths above are also confirming that the express-validator query sanitizers land.

Comment thread package.json
"@babel/preset-env": "^7.28.0",
"@cypress/code-coverage": "^3.14.5",
"@babel/preset-env": "^7.29.7",
"@cypress/code-coverage": "^4.0.3",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Verify @cypress/code-coverage v4 still ships the express middleware entrypoint

backend/app.ts:50 does require("@cypress/code-coverage/middleware/express")(app) when running instrumented (CYPRESS_COVERAGE=true). The plugin jumps a major version (3.x → 4.x); if that subpath entry was removed or moved behind package exports in v4, the coverage-enabled backend startup used by start:ci/tsnode would throw at require time. This path is guarded by global.__coverage__, so a normal yarn dev run would not surface it — the CI coverage run would.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Checked the installed 4.0.3: the subpath is still published, just relocated behind exports ("./middleware/express": "./dist/middleware/express.js"), so require("@cypress/code-coverage/middleware/express") resolves to dist/middleware/express.js and returns a function. The middleware/ source directory no longer exists in the package, which is exactly the case exports covers — a deep path into middleware/* would have broken, the mapped subpath does not.

node -e "console.log(typeof require('@cypress/code-coverage/middleware/express'))"  -> function

Also worth noting v4 raises its cypress peer to >=15.10.0; we are on 15.20.0.

Comment thread backend/auth.ts
Comment on lines +51 to 53
req.logout(() => {
req.session!.destroy(() => res.redirect("/"));
});

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: Logout now redirects once instead of racing two redirects

The old code called res.redirect("/") from the req.logout callback and again from the session.destroy callback, which produced an ERR_HTTP_HEADERS_SENT on whichever fired second, and also destroyed the session concurrently with logout. The new sequencing (logout → destroy → single redirect) removes both problems; note the previous destroy error argument is now discarded, so a session-store failure is silently ignored and still redirects — acceptable for the in-memory store used here.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Correct on both counts, and that double-redirect is what the fix removes. The crash I actually hit was upstream of it: passport 0.7's req.logout regenerates the session asynchronously, and the old synchronous req.session.destroy() ran first, so passport then dereferenced a destroyed session and took the backend process down with TypeError: Cannot read properties of undefined (reading 'regenerate').

The discarded destroy error is deliberate — the previous code ignored it too (it redirected regardless of err), and with the in-memory store there is no meaningful recovery beyond clearing the cookie, which already happened. Verified: POST /logout -> 302, and the API stays up afterwards (/checkAuth -> 401 rather than a dead socket).

@devin-ai-integration

Copy link
Copy Markdown
Author

Browser walkthrough @ 03ad9c2 — all previously-found regressions fixed and verified

Recorded browser verification against yarn dev (Vite :3000 + Express 5 API :3001, seeded user Heath93). Both backend fixes in this round are confirmed working, and there are no remaining blockers. This consolidates the full walkthrough from 6a4c6a6 plus this verification pass. Devin session: https://app.devin.ai/sessions/5146de38ebb147cfbda93ad807a68c4d

verification recording

Fix 1 — POST /logout no longer crashes the API (passport 0.7)

Clicking Logout returns to the sign-in page with no "Network Error" banner, and — the decisive check — logging back in immediately succeeds, which is impossible if the API process has died.

Logout → clean sign-in, no error banner Re-login succeeds → API survived
logout clean relogin
  • Backend process stayed up 19m38s across every logout, with zero Cannot read properties of undefined (reading 'regenerate') and zero [nodemon] app crashed — no restart masking a crash.
  • POST /logout → 302 (was 000/severed connection), /checkAuth → 401 afterwards, re-login → 200.
Fix 2 — Express 5 writable req.query: pagination + filters all work

GET /transactions/public with no params now returns 10 results and {page:1, limit:10, totalPages:5, hasNextPages:true} (was 38 results with page/limit absent and totalPages: null).

In the UI the feed loads exactly 10 rows and grows to ~28 on scroll with no blank rows, $NaN or $undefined. The express-validator query sanitizers that write into the same req.query also still work — no 422 on any filtered request:

Amount filter $250 – $1,000 applied Date filter Feb 2024
amount filter date filter
  • Amount: all 10 visible rows within range (min $283.19); smaller rows correctly dropped; Clear restores $0 - $1,000 and the excluded rows return.
  • Date: distinct non-empty set for Feb 2024, and correctly "No Transactions" for a 2026 range (seed data spans 2023-03 → 2024-03), so the filter discriminates rather than always returning empty.
  • API cross-check: ?amountMin=25000&amountMax=100000 returned the identical 10 amounts rendered in the UI; the Feb range returned 6 rows all with createdAt in 2024-02.
  • express 5.2.1 reports query parser: simple; the backend only reads flat scalar params (page, limit, q, dateRange*, amount*), so the loss of nested/bracket parsing is not reachable, and the query getter is configurable so the shim never throws.
Previously verified at 6a4c6a6 (unchanged by these backend-only commits)

full walkthrough

  • history 4.10.1 revert fixes the blank-feed regression: post-login redirect renders a populated feed; MINE/FRIENDS/EVERYONE update URL + aria-selected + rows
  • $25 payment end-to-end: yup 1 blocks an empty note (Please enter a note, Pay disabled), NumericFormat shows $25, submit → Transaction Submitted!, balance $1,509.53$1,484.53, appears first in the MINE feed
  • yarn lint, yarn types, yarn build, yarn test:unit:ci (44 passed) all green at 03ad9c2
  • yarn start:ci starts cleanly on http-proxy-middleware 3; with BACKEND_PORT=3001, /login, /checkAuth, /graphql return 200 application/json from the backend while / and /personal fall back to the SPA
  • @cypress/code-coverage 4 still exposes middleware/express (relocated behind package exports), so the instrumented backend path in backend/app.ts still requires cleanly
Pre-existing, unrelated to this PR: BACKEND_PORT is never defined

src/setupProxy.js targets http://localhost:${process.env.BACKEND_PORT}, but BACKEND_PORT is set nowhere — .env only has VITE_BACKEND_PORT=3001, and it is absent from .github/ workflows, so out of the box the target is http://localhost:undefined and proxied paths return 504. develop's setupProxy.js uses the same variable, so this predates the PR, but it means yarn start:ci cannot actually proxy in CI today. Worth a separate fix (e.g. fall back to VITE_BACKEND_PORT).

This PR does fix a latent bug in the same file: develop had "graphql" without a leading slash in the context array; HEAD uses /graphql.

Console: only pre-existing stop-color/stop-opacity warnings from SvgCypressLogo.tsx, xstate deprecation warnings, and Vite/React DevTools info. No 422s, no 5xx, no React 19 or axios errors.

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.

0 participants