fix: handle stricter Express request param types and update dependencies #524
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces a new request parameter utility function to normalize Express Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @WikiRik, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the project's stability and maintainability by updating various dependencies and improving type safety. The dependency updates led to stricter TypeScript checks, particularly for Express request parameters, which are now correctly typed to include array possibilities. To address this, a new utility function was introduced and integrated into several route handlers, ensuring robust and type-safe handling of request parameters. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a type-safety issue that arose from dependency updates, specifically the stricter typing of Express request parameters. The introduction of the getStringParam helper function is a clean and reusable solution to handle the string | string[] type. The dependency updates themselves are well-documented and categorized. My main feedback is a suggestion to make the new getStringParam function more robust by handling the edge case of an empty array parameter to prevent potential runtime errors.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
The test was failing because commit c2430e5 changed the authentication error detection to be more specific (JSON format only) to avoid false positives with garbled winpty encoding on Windows. Updated the test to send the correct JSON format pattern.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/ui/package.json`:
- Around line 69-73: The xterm addon versions in package.json are incompatible
with `@xterm/xterm` 5.5.0; update the dependency versions for the addons to match
the 5.5.0 release: change "@xterm/addon-fit" from 0.11.0 to 0.10.0,
"@xterm/addon-search" from 0.16.0 to 0.15.0, "@xterm/addon-web-links" from
0.12.0 to 0.11.0, and "@xterm/addon-webgl" from 0.19.0 to 0.18.0 (then run your
package manager install to lock the correct versions).
🧹 Nitpick comments (5)
apps/server/src/routes/terminal/routes/session-resize.ts (1)
5-13: Use@automaker/import for request utils.*
The new relative import ofgetStringParamconflicts with the guideline to import from shared@automaker/*packages. Please re-export it from an@automaker/*package (or existing shared utils) and import from there. As per coding guidelines, ...apps/server/src/routes/terminal/routes/session-delete.ts (1)
5-13: Use@automaker/import for request utils.*
The new relative import ofgetStringParamconflicts with the guideline to import from shared@automaker/*packages. Please re-export it from an@automaker/*package (or existing shared utils) and import from there. As per coding guidelines, ...apps/server/src/routes/sessions/routes/delete.ts (1)
5-14: Use@automaker/import for request utils.*
The new relative import ofgetStringParamconflicts with the guideline to import from shared@automaker/*packages. Please re-export it from an@automaker/*package (or existing shared utils) and import from there. As per coding guidelines, ...apps/server/src/routes/sessions/routes/update.ts (1)
5-14: Use@automaker/import for request utils.*
The new relative import ofgetStringParamconflicts with the guideline to import from shared@automaker/*packages. Please re-export it from an@automaker/*package (or existing shared utils) and import from there. As per coding guidelines, ...apps/server/src/routes/sessions/routes/archive.ts (1)
5-14: Use@automaker/import for request utils.*
The new relative import ofgetStringParamconflicts with the guideline to import from shared@automaker/*packages. Please re-export it from an@automaker/*package (or existing shared utils) and import from there. As per coding guidelines, ...
| "@xterm/addon-fit": "0.11.0", | ||
| "@xterm/addon-search": "0.16.0", | ||
| "@xterm/addon-web-links": "0.12.0", | ||
| "@xterm/addon-webgl": "0.19.0", | ||
| "@xterm/xterm": "5.5.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check peer dependency requirements for specific xterm addon versions
echo "Checking npm peer dependencies for xterm addons..."
for pkg in "@xterm/addon-fit@0.11.0" "@xterm/addon-search@0.16.0" "@xterm/addon-web-links@0.12.0" "@xterm/addon-webgl@0.19.0"; do
echo "=== $pkg ==="
npm view "$pkg" peerDependencies --json 2>/dev/null || echo "No peer deps found"
echo ""
doneRepository: AutoMaker-Org/automaker
Length of output: 260
🏁 Script executed:
#!/bin/bash
# Get full package metadata from npm registry
echo "Checking full package metadata for xterm addons..."
npm view "@xterm/addon-fit@0.11.0" --json 2>/dev/null | jq -r '.peerDependencies, .engines, .dependencies' || echo "Failed to get fit metadata"
echo ""
echo "Checking `@xterm/xterm`@5.5.0 metadata..."
npm view "@xterm/xterm@5.5.0" --json 2>/dev/null | jq -r '.version, .dependencies' || echo "Failed to get xterm metadata"Repository: AutoMaker-Org/automaker
Length of output: 184
🏁 Script executed:
#!/bin/bash
# Get complete package metadata
echo "=== Full metadata for `@xterm/addon-fit`@0.11.0 ==="
npm view "@xterm/addon-fit@0.11.0" --json 2>/dev/null | head -50
echo ""
echo "=== Checking npm registry URL for addon compatibility info ==="
npm view "@xterm/addon-fit" versions --json 2>/dev/null | tail -20Repository: AutoMaker-Org/automaker
Length of output: 1620
🏁 Script executed:
#!/bin/bash
# Get specific fields from npm registry that contain dependency info
echo "=== Checking dependencies for addon versions ==="
npm view "@xterm/addon-fit@0.11.0" --json 2>/dev/null | jq '.peerDependencies // "no peer deps" | .dependencies // "no deps"'
echo ""
echo "=== Getting full object keys to understand structure ==="
npm view "@xterm/addon-fit@0.11.0" --json 2>/dev/null | jq 'keys' | head -30Repository: AutoMaker-Org/automaker
Length of output: 683
🏁 Script executed:
#!/bin/bash
# Search for xterm addons in the codebase to find any package-lock or version info
echo "=== Checking for xterm addon info in node_modules or lock files ==="
cd apps/ui
if [ -f "package-lock.json" ]; then
echo "Found package-lock.json, checking xterm addon entries..."
jq '.dependencies[] | select(.name | startswith("@xterm/addon")) | {name, version, requires}' package-lock.json 2>/dev/null | head -100
elif [ -f "yarn.lock" ]; then
echo "Checking yarn.lock for xterm addons..."
grep -A 5 "@xterm/addon" yarn.lock | head -50
else
echo "No lock file found"
fiRepository: AutoMaker-Org/automaker
Length of output: 153
🏁 Script executed:
#!/bin/bash
# Check the actual package.json content for context
cd apps/ui
echo "=== Full xterm dependency section from package.json ==="
cat package.json | jq '.dependencies | to_entries[] | select(.key | contains("xterm"))' 2>/dev/nullRepository: AutoMaker-Org/automaker
Length of output: 393
🌐 Web query:
xterm addon compatibility with @xterm/xterm 5.5.0 addon-fit addon-search addon-web-links addon-webgl
💡 Result:
For @xterm/xterm 5.5.0 the compatible addon versions are:
@xterm/addon-fit: 0.10.0. [1]@xterm/addon-search: 0.15.0. [1]@xterm/addon-web-links: 0.11.0. [1]@xterm/addon-webgl: 0.18.0. [1]
(See xterm.js 5.5.0 release notes / compatible addon table.) [1][2]
Sources:
[1] xtermjs/xterm.js releases (5.5.0) — GitHub.
[2] xterm.js 5.5.0 release summary (NewReleases).
Downgrade xterm addons to versions compatible with @xterm/xterm 5.5.0.
The current addon versions are incompatible with the installed xterm version. According to the official xterm.js 5.5.0 release notes, the following versions must be used:
@xterm/addon-fit: change from 0.11.0 to 0.10.0@xterm/addon-search: change from 0.16.0 to 0.15.0@xterm/addon-web-links: change from 0.12.0 to 0.11.0@xterm/addon-webgl: change from 0.19.0 to 0.18.0
Using newer addon versions with xterm 5.5.0 will cause runtime errors in the terminal component.
🤖 Prompt for AI Agents
In `@apps/ui/package.json` around lines 69 - 73, The xterm addon versions in
package.json are incompatible with `@xterm/xterm` 5.5.0; update the dependency
versions for the addons to match the 5.5.0 release: change "@xterm/addon-fit"
from 0.11.0 to 0.10.0, "@xterm/addon-search" from 0.16.0 to 0.15.0,
"@xterm/addon-web-links" from 0.12.0 to 0.11.0, and "@xterm/addon-webgl" from
0.19.0 to 0.18.0 (then run your package manager install to lock the correct
versions).
There was a problem hiding this comment.
The peer dependency version for these packages was removed between the 5.5.0 release and the current version. As the tests are passing, it might not be a breaking change internally. Did you check what the changes between those versions are and what the impact might be?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Summary
This PR updates low-risk patch and medium priority dependencies while maintaining pinned versions. The dependency updates triggered stricter TypeScript checking that required fixing how we handle Express request parameters.
Type Safety Fix
The dependency updates caused
@types/express-serve-static-coreto be updated from 5.1.0 → 5.1.1, which now correctly typesreq.paramsvalues asstring | string[]instead of juststring. This is more accurate since Express does support array values in route parameters. See DefinitelyTyped/DefinitelyTyped#74161 for more information about this changeWhy This Change Was Needed
When we updated the dependencies, TypeScript started failing with errors like:
This happened because the Express type definitions became more accurate -
req.paramscan technically be arrays in certain cases. While Express routing typically guarantees single string values for matched route parameters, the type system needed to be satisfied.Solution
Created a helper function
getStringParam()inapps/server/src/lib/request-utils/index.ts:This helper:
stringandstring[]typesstringtypeFiles Updated
Updated 6 route handlers to use the helper:
apps/server/src/routes/sessions/routes/archive.tsapps/server/src/routes/sessions/routes/delete.tsapps/server/src/routes/sessions/routes/unarchive.tsapps/server/src/routes/sessions/routes/update.tsapps/server/src/routes/terminal/routes/session-delete.tsapps/server/src/routes/terminal/routes/session-resize.tsDependency Updates
Low-Risk Patches
Medium Priority Updates
Testing
All tests pass successfully:
Notes
This PR was generated through user prompts with OpenCode and Claude Sonnet 4.5. It's a rebased version of #523
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.