chore(deps): combine open Dependabot updates - #678
Conversation
Combines the following Dependabot updates into a single PR: - Bump mcp from 1.27.2 to 1.28.1 (#663) - Bump actions/setup-python from 6.3.0 to 7.0.0 (#665) - Bump actions/checkout from 7.0.0 to 7.0.1 (#666) - Bump astral-sh/setup-uv from 8.3.2 to 9.0.0 (#667) - Bump openai from 2.45.0 to 2.48.0 (#668) - Bump fastapi from 0.139.0 to 0.140.0 (#669) - Bump sqlglot from 30.12.0 to 30.13.0 (#670) - Bump snowflake-connector-python from 4.6.0 to 4.7.1 (#671) - Bump fastmcp from 3.2.4 to 3.4.4 (#672) - Bump brace-expansion from 5.0.7 to 5.0.9 (latest available) in /app (#674) - Bump rojopolis/spellcheck-github-actions from 0.63.0 to 0.63.1 (#675) - Bump docker/login-action from 4.4.0 to 4.6.0 (#676) - Bump the npm-minor-patch group across 1 directory with 29 updates in /app (#677) Verified locally: uv run pytest (210 passed), uv run pylint (10.00/10), npm run lint and npm run build (in app/), all against a running FalkorDB container. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Completed Working on "Code Review"✅ Review publishing completed successfully. Posted comments from all chunks and submitted final review: COMMENT with 2 total comments across 2 files. ✅ Workflow completed successfully. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe pull request updates pinned GitHub Actions revisions, refreshes frontend and server dependencies, migrates router imports to ChangesDependency refresh
Workflow action refresh
End-to-end routing coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Dependency ReviewThe following issues were found:
|
There was a problem hiding this comment.
Summary of posted review findings:
- Counts by importance: 0 BLOCKER, 0 CRITICAL, 2 MAJOR, 0 MINOR, 0 SUGGESTION, 0 PRAISE.
- Affected files: 2 (
.github/workflows/publish-docker.yml,pyproject.toml).
Key themes:
- Dependency/version governance and reproducibility risk (unbounded runtime dependency range).
- CI/security auditability clarity for pinned GitHub Actions (comment/version mismatch).
Actionable next steps:
- Constrain
fastmcpto a bounded, validated 3.x range and regenerate the lockfile. - Update the Docker login action inline comment to match the pinned release version for consistent maintenance and security review context.
There was a problem hiding this comment.
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 @.github/workflows/playwright.yml:
- Around line 40-42: Configure setup-uv caching steps with prune-cache: true to
preserve cache pruning. Apply this to .github/workflows/playwright.yml lines
40-42, .github/workflows/pylint.yml lines 21-22, and both setup-uv steps in
.github/workflows/tests.yml lines 41-42 and 125-126.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d715458-a58d-40d3-a2cd-e849af01aaa8
⛔ Files ignored due to path filters (2)
app/package-lock.jsonis excluded by!**/package-lock.jsonuv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/dependency-review.yml.github/workflows/playwright.yml.github/workflows/publish-docker.yml.github/workflows/publish-pypi.yml.github/workflows/pylint.yml.github/workflows/spellcheck.yml.github/workflows/tests.ymlapp/package.jsonpyproject.toml
There was a problem hiding this comment.
Pull request overview
This PR consolidates multiple Dependabot updates across the Python backend (uv-managed), the React/TypeScript frontend (npm-managed), and GitHub Actions workflows to keep dependencies and CI tooling current.
Changes:
- Bumps Python dependencies and refreshes
uv.lock(notably FastAPI, OpenAI SDK, MCP, SQLGlot, Snowflake connector, FastMCP). - Updates frontend dependency versions in
app/package.json. - Updates pinned SHAs for several GitHub Actions used in CI workflows.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Updates resolved Python dependency versions and metadata for the uv lockfile. |
| pyproject.toml | Expands version bounds for fastapi and snowflake-connector-python in the server extra. |
| app/package.json | Bumps frontend direct dependencies (Radix UI, React, React Router, Vite plugin, PostCSS, etc.). |
| .github/workflows/tests.yml | Updates pinned SHAs for checkout/setup-python/setup-uv used by test jobs. |
| .github/workflows/spellcheck.yml | Updates pinned SHAs for checkout and the spellcheck action. |
| .github/workflows/pylint.yml | Updates pinned SHAs for checkout/setup-python/setup-uv used by pylint job. |
| .github/workflows/publish-pypi.yml | Updates pinned SHAs for checkout/setup-python/setup-uv in PyPI publish workflow. |
| .github/workflows/publish-docker.yml | Updates pinned SHAs for checkout and docker/login-action. |
| .github/workflows/playwright.yml | Updates pinned SHAs for checkout/setup-python/setup-uv used by Playwright workflow. |
| .github/workflows/dependency-review.yml | Updates pinned SHA for checkout in dependency review workflow. |
The root package.json depends on queryweaver-app via 'file:app', so its
lockfile embeds a resolved snapshot of app/'s dependency tree. It was
left stale after the app/ dependency bumps, causing 'npm ci' to fail in
CI ('lock file does not satisfy...'). Regenerated via 'npm install'.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (4)
.github/workflows/tests.yml:129
astral-sh/setup-uvwas bumped to v9, which changed the defaultprune-cachetofalse(breaking change). Consider settingprune-cacheexplicitly here so cache size/cost behavior is intentional.
- name: Install uv
uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
with:
version: "latest"
.github/workflows/pylint.yml:25
- After upgrading to
astral-sh/setup-uvv9, the defaultprune-cacheis nowfalse. Set it explicitly to keep CI cache usage predictable (and optionally preserve the pre-v9 behavior).
- name: Install uv
uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
with:
version: "latest"
.github/workflows/playwright.yml:45
- With
astral-sh/setup-uvv9,prune-cachedefaults tofalse(breaking change). If you want to avoid cache growth surprises, setprune-cacheexplicitly in this step.
- name: Install uv
uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
with:
version: "0.7.12"
.github/workflows/tests.yml:45
astral-sh/setup-uvwas bumped to v9, which changed the defaultprune-cachetofalse(breaking change). If you want to preserve the previous behavior and avoid unbounded cache growth/cost surprises, setprune-cacheexplicitly in the workflow.
This issue also appears on line 125 of the same file.
- name: Install uv
uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
with:
version: "0.7.12"
…c8h2) react-router-dom's latest release (7.18.2) never received a fix for GHSA-qwww-vcr4-c8h2 (CSRF bypass in unstable RSC mode) — the fix only ships in react-router v8.3.0+, after react-router-dom was folded back into the react-router package. QueryWeaver already meets v8's runtime requirements (React 19.2+, Node 22+) and only uses plain SPA routing (BrowserRouter/Routes/Route/Link/ useNavigate/useLocation) with no loaders, actions, middleware, or meta() usage, so none of v8's actual breaking-change surface applies. Changes: - app/package.json, app/package-lock.json: drop react-router-dom, add react-router@^8.3.0 - app/src/App.tsx, Sidebar.tsx, Settings.tsx, NotFound.tsx: update imports from "react-router-dom" to "react-router" - package-lock.json (root): resync with app/'s file: dependency - e2e: add settings-navigation and unknown-route (404) coverage to sidebar.spec.ts/sidebar POM to exercise the migrated router APIs, plus a small getPageContent() helper on BasePage Verified locally: npm run lint, npm run build (app/), and `npx playwright test --list` all pass; no react-router-dom references remain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (2)
e2e/tests/sidebar.spec.ts:70
- The test name says it verifies that browser back navigation returns home, but the test never performs a back-navigation action or asserts the home route. This makes the test description misleading and harder to understand when it fails.
// Routing tests: verify react-router (v8) navigation still works after the
// react-router-dom -> react-router migration.
test('settings button navigates to /settings and back navigates home', async () => {
e2e/tests/sidebar.spec.ts:69
- PR description says no
react-router-domreferences remain in the codebase, but this comment still contains the stringreact-router-dom. If you want that statement to be literally true (and keep future greps/CI checks clean), reword the comment to avoid mentioningreact-router-domby name.
// Routing tests: verify react-router (v8) navigation still works after the
// react-router-dom -> react-router migration.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/package.json (1)
55-60: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSet Node 22.22.0 before using React Router v8.
react-router@8.3.0requires Node>=22.22.0, but the rootpackage.jsonhas noengines.nodeand CI only usesNODE_VERSION: 'lts/*'. Make every install/runtime path Node 22.22.0+ or use an olderreact-routerversion.🤖 Prompt for 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. In `@app/package.json` around lines 55 - 60, Ensure the project’s declared and CI/runtime Node version is at least 22.22.0 before retaining react-router v8.3.0: add or update the root package engine and all installation/runtime configuration, including the CI NODE_VERSION setting, to enforce Node 22.22.0+. Alternatively, downgrade the react-router dependency to a version compatible with the existing Node support.
🧹 Nitpick comments (2)
e2e/tests/sidebar.spec.ts (2)
70-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the test title with its assertions.
The title says that the test verifies navigation back to home. The test does not navigate back or assert
/. Rename the test to describe settings navigation and refresh persistence, or add the missing back-navigation assertion.Minimal rename
- test('settings button navigates to /settings and back navigates home', async () => { + test('settings button navigates to /settings and persists after refresh', async () => {🤖 Prompt for 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. In `@e2e/tests/sidebar.spec.ts` at line 70, Rename the test titled “settings button navigates to /settings and back navigates home” to accurately describe only the settings navigation and refresh persistence it currently asserts, without adding back-navigation behavior.
78-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify the rendered Settings page after refresh.
The assertion checks only the browser URL. A refresh can leave
/settingsin the address bar while rendering an error response. Assert a stable Settings-specific element or text afterrefreshPage().🤖 Prompt for 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. In `@e2e/tests/sidebar.spec.ts` around lines 78 - 80, Update the test around sidebar.refreshPage() to assert a stable Settings-specific element or text in the rendered page after refreshing, while retaining the existing URL assertion. Use an existing Settings page locator or content assertion rather than relying only on sidebar.getCurrentURL().
🤖 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.
Outside diff comments:
In `@app/package.json`:
- Around line 55-60: Ensure the project’s declared and CI/runtime Node version
is at least 22.22.0 before retaining react-router v8.3.0: add or update the root
package engine and all installation/runtime configuration, including the CI
NODE_VERSION setting, to enforce Node 22.22.0+. Alternatively, downgrade the
react-router dependency to a version compatible with the existing Node support.
---
Nitpick comments:
In `@e2e/tests/sidebar.spec.ts`:
- Line 70: Rename the test titled “settings button navigates to /settings and
back navigates home” to accurately describe only the settings navigation and
refresh persistence it currently asserts, without adding back-navigation
behavior.
- Around line 78-80: Update the test around sidebar.refreshPage() to assert a
stable Settings-specific element or text in the rendered page after refreshing,
while retaining the existing URL assertion. Use an existing Settings page
locator or content assertion rather than relying only on
sidebar.getCurrentURL().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e9c69b9d-2c4d-41dc-b918-37a8be937e60
⛔ Files ignored due to path filters (2)
app/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
app/package.jsonapp/src/App.tsxapp/src/components/layout/Sidebar.tsxapp/src/pages/NotFound.tsxapp/src/pages/Settings.tsxe2e/infra/ui/basePage.tse2e/logic/pom/sidebar.tse2e/tests/sidebar.spec.ts
- publish-docker.yml: fix stale docker/login-action version comment (was still "# v4" after the bump to v4.6.0) - pyproject.toml/uv.lock: bound fastmcp to >=3.4.4,<4.0.0 (was unbounded >=3.2.4), matching the bounded-range convention used by the other server-extra deps in this file - playwright.yml, pylint.yml, tests.yml (x2): explicitly set prune-cache: true on setup-uv steps — astral-sh/setup-uv v9.0.0 (bumped in this PR) silently flipped the prune-cache default from true to false, which would otherwise change cache-retention behavior as a side effect of the version bump Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed in 4f5ea3a:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
e2e/tests/sidebar.spec.ts:80
- The test name says it verifies “back navigates home”, but the test never navigates back/home; it only refreshes and re-asserts the /settings URL. This makes the test intent misleading and leaves the settings-toggle behavior (settings -> home) untested.
test('settings button navigates to /settings and back navigates home', async () => {
Combines the following Dependabot updates into one PR:
Python (uv):
mcpfrom 1.27.2 to 1.28.1 (chore(deps): bump mcp from 1.27.2 to 1.28.1 in the uv group across 1 directory #663)openaifrom 2.45.0 to 2.48.0 (chore(deps): bump openai from 2.45.0 to 2.48.0 #668)fastapifrom 0.139.0 to 0.140.0 (chore(deps): bump fastapi from 0.139.0 to 0.140.0 #669)sqlglotfrom 30.12.0 to 30.13.0 (chore(deps): bump sqlglot from 30.12.0 to 30.13.0 #670)snowflake-connector-pythonfrom 4.6.0 to 4.7.1 (chore(deps): bump snowflake-connector-python from 4.6.0 to 4.7.1 #671)fastmcpfrom 3.2.4 to 3.4.4 (chore(deps): bump fastmcp from 3.2.4 to 3.4.4 #672)npm (app/):
brace-expansionfrom 5.0.7 to 5.0.9 (latest available; transitive) (chore(deps): bump brace-expansion from 5.0.7 to 5.0.8 in /app in the npm_and_yarn group across 1 directory #674)GitHub Actions:
actions/setup-pythonfrom 6.3.0 to 7.0.0 (chore(deps): bump actions/setup-python from 6.3.0 to 7.0.0 #665)actions/checkoutfrom 7.0.0 to 7.0.1 (chore(deps): bump actions/checkout from 7.0.0 to 7.0.1 #666)astral-sh/setup-uvfrom 8.3.2 to 9.0.0 (chore(deps): bump astral-sh/setup-uv from 8.3.2 to 9.0.0 #667)rojopolis/spellcheck-github-actionsfrom 0.63.0 to 0.63.1 (chore(deps): bump rojopolis/spellcheck-github-actions from 0.63.0 to 0.63.1 #675)docker/login-actionfrom 4.4.0 to 4.6.0 (chore(deps): bump docker/login-action from 4.4.0 to 4.6.0 #676)Verified locally:
uv run pytest(210 passed, 1 skipped),uv run pylint(10.00/10),npm run lintandnpm run build(inapp/), all against a running FalkorDB Docker container.Closes #663, #665, #666, #667, #668, #669, #670, #671, #672, #674, #675, #676, #677.
Security fix (beyond the Dependabot bumps above):
react-router-dom(^7.18.2) toreact-router(^8.3.0) to pick up the fix forGHSA-qwww-vcr4-c8h2
(CSRF bypass in unstable RSC mode), which was flagged by the
dependency-reviewCI check.react-router-dom's final release (7.18.2) never received a backport of this fix — it's onlypatched in
react-router8.3.0+, afterreact-router-domwas folded back into the mainreact-routerpackage. There's an open community request for a 7.x backport(remix-run/react-router#15348) but no
maintainer commitment or ETA, so upgrading to v8 was the only way to close this out.
SPA routing (
BrowserRouter/Routes/Route/Link/useNavigate/useLocation) with noloaders, actions, middleware, or
meta()usage, so none of v8's breaking-change surface(always-on middleware, removed
AppLoadContext,meta({data})removal, ESM-only) applies here.App.tsx,Sidebar.tsx,Settings.tsx,NotFound.tsxfromreact-router-domtoreact-router.e2e/tests/sidebar.spec.ts) for/settingsnavigation and unknown-route(404) rendering to exercise the migrated router APIs.
npm run lint,npm run build(inapp/), andnpx playwright test --listall pass;no
react-router-domreferences remain in the codebase.Summary by CodeRabbit