fix: simplify usb listing #1402
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughThe USB device parsing logic in the Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/src/graphql/resolvers/query/info.ts (1)
367-377: Drop the unnecessaryasyncwrapper aroundparseBasicDeviceto avoid extra micro-tasks.
parseBasicDeviceperforms only synchronous work, yet it’s declaredasync, forcingPromise.allto wrap each call needlessly.-const parseBasicDevice = async (device: PciDevice): Promise<PciDevice> => { +const parseBasicDevice = (device: PciDevice): PciDevice => {With this change the subsequent
Promise.all(devices.map(parseBasicDevice))can be replaced by a simpledevices.map(parseBasicDevice)(or kept—Promise.allwill happily consume plain values).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/graphql/resolvers/query/info.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
Comment on lines
+350
to
+365
| const parseUsbDevices = (stdout: string): PciDevice[] => | ||
| stdout | ||
| .split('\n') | ||
| .map((line) => { | ||
| const regex = new RegExp(/^.+: ID (?<id>\S+)(?<n>.*)$/); | ||
| const result = regex.exec(line); | ||
| if (!result?.groups) return null; | ||
|
|
||
| // Extract name from the line if available | ||
| const name = result.groups.n?.trim() || ''; | ||
| return { | ||
| ...result.groups, | ||
| name, | ||
| } as unknown as PciDevice; | ||
| }) | ||
| .filter((device): device is PciDevice => device !== null) ?? []; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up regex capture & output shape.
- The capture group is named
n, which then leaks as an unnecessary property after the spread. - Any extra whitespace between the product ID and the name isn’t consumed, so
namecan start with a leading space.
-const regex = new RegExp(/^.+: ID (?<id>\S+)(?<n>.*)$/);
+const regex = /^.+: ID (?<id>\S+)\s+(?<name>.*)$/;
...
-// Extract name from the line if available
-const name = result.groups.n?.trim() || '';
+const name = result.groups.name?.trim() || '';
...
-return { ...result.groups, name } as unknown as PciDevice;
+return { id: result.groups.id, name } as unknown as PciDevice;This removes the stray n field and produces a cleaner PciDevice payload.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const parseUsbDevices = (stdout: string): PciDevice[] => | |
| stdout | |
| .split('\n') | |
| .map((line) => { | |
| const regex = new RegExp(/^.+: ID (?<id>\S+)(?<n>.*)$/); | |
| const result = regex.exec(line); | |
| if (!result?.groups) return null; | |
| // Extract name from the line if available | |
| const name = result.groups.n?.trim() || ''; | |
| return { | |
| ...result.groups, | |
| name, | |
| } as unknown as PciDevice; | |
| }) | |
| .filter((device): device is PciDevice => device !== null) ?? []; | |
| const parseUsbDevices = (stdout: string): PciDevice[] => | |
| stdout | |
| .split('\n') | |
| .map((line) => { | |
| const regex = /^.+: ID (?<id>\S+)\s+(?<name>.*)$/; | |
| const result = regex.exec(line); | |
| if (!result?.groups) return null; | |
| const name = result.groups.name?.trim() || ''; | |
| return { id: result.groups.id, name } as unknown as PciDevice; | |
| }) | |
| .filter((device): device is PciDevice => device !== null) ?? []; |
🤖 Prompt for AI Agents
In api/src/graphql/resolvers/query/info.ts around lines 350 to 365, the regex
capture group named 'n' is leaking as an unnecessary property in the output
object and leading whitespace before the device name is not trimmed. To fix
this, rename or remove the 'n' capture group so it does not appear in the spread
object, and adjust the regex to consume any extra whitespace between the product
ID and the name. Then, trim the extracted name to ensure no leading spaces
remain, resulting in a cleaner PciDevice object without the stray 'n' field.
Comment on lines
+334
to
341
| // Use a simplified GUID generation instead of calling lsusb -v | ||
| const idParts = device.id.split(':'); | ||
| if (idParts.length === 2) { | ||
| const [vendorId, productId] = idParts; | ||
| modifiedDevice.guid = `${vendorId}-${productId}-basic`; | ||
| } else { | ||
| modifiedDevice.guid = `unknown-${Math.random().toString(36).substring(7)}`; | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid non-deterministic GUIDs – use a stable hash instead of Math.random.
Math.random() changes on every invocation, so any device that falls into the else branch will receive a different GUID each time the resolver runs, breaking caching/UI correlations and introducing collision risk.
-import { randomUUID } from 'crypto'; // at top of the file
+import { createHash } from 'node:crypto'; // at top of the file
...
- modifiedDevice.guid = `unknown-${Math.random().toString(36).substring(7)}`;
+ // Create a deterministic fallback GUID based on the raw lsusb line.
+ const hash = createHash('sha1').update(device.id ?? '').digest('hex').slice(0, 8);
+ modifiedDevice.guid = `unknown-${hash}`;A deterministic hash keeps the GUID stable across calls while still avoiding the expensive lsusb -v lookup.
🤖 Prompt for AI Agents
In api/src/graphql/resolvers/query/info.ts around lines 334 to 341, the current
code uses Math.random() to generate a GUID for devices without a standard id
format, which results in non-deterministic and unstable GUIDs. Replace
Math.random() with a deterministic hash function that takes a stable input (such
as the device id or other consistent device properties) to generate a stable
GUID. This ensures the GUID remains consistent across resolver calls, improving
caching and UI correlation without the overhead of lsusb -v.
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
zackspear
approved these changes
May 22, 2025
pujitm
pushed a commit
that referenced
this pull request
Jul 8, 2025
🤖 I have created a release *beep* *boop* --- ## [4.9.0](v4.8.0...v4.9.0) (2025-07-08) ### Features * add graphql resource for API plugins ([#1420](#1420)) ([642a220](642a220)) * add management page for API keys ([#1408](#1408)) ([0788756](0788756)) * add rclone ([#1362](#1362)) ([5517e75](5517e75)) * API key management ([#1407](#1407)) ([d37dc3b](d37dc3b)) * api plugin management via CLI ([#1416](#1416)) ([3dcbfbe](3dcbfbe)) * build out docker components ([#1427](#1427)) ([711cc9a](711cc9a)) * docker and info resolver issues ([#1423](#1423)) ([9901039](9901039)) * fix shading in UPC to be less severe ([#1438](#1438)) ([b7c2407](b7c2407)) * info resolver cleanup ([#1425](#1425)) ([1b279bb](1b279bb)) * initial codeql setup ([#1390](#1390)) ([2ade7eb](2ade7eb)) * initialize claude code in codebse ([#1418](#1418)) ([b6c4ee6](b6c4ee6)) * move api key fetching to use api key service ([#1439](#1439)) ([86bea56](86bea56)) * move to cron v4 ([#1428](#1428)) ([b8035c2](b8035c2)) * move to iframe for changelog ([#1388](#1388)) ([fcd6fbc](fcd6fbc)) * native slackware package ([#1381](#1381)) ([4f63b4c](4f63b4c)) * send active unraid theme to docs ([#1400](#1400)) ([f71943b](f71943b)) * slightly better watch mode ([#1398](#1398)) ([881f1e0](881f1e0)) * upgrade nuxt-custom-elements ([#1461](#1461)) ([345e83b](345e83b)) * use bigint instead of long ([#1403](#1403)) ([574d572](574d572)) ### Bug Fixes * activation indicator removed ([5edfd82](5edfd82)) * alignment of settings on ManagementAccess settings page ([#1421](#1421)) ([70c790f](70c790f)) * allow rclone to fail to initialize ([#1453](#1453)) ([7c6f02a](7c6f02a)) * always download 7.1 versioned files for patching ([edc0d15](edc0d15)) * api `pnpm type-check` ([#1442](#1442)) ([3122bdb](3122bdb)) * **api:** connect config `email` validation ([#1454](#1454)) ([b9a1b9b](b9a1b9b)) * backport unraid/webgui[#2269](https://github.com/unraid/api/issues/2269) rc.nginx update ([#1436](#1436)) ([a7ef06e](a7ef06e)) * bigint ([e54d27a](e54d27a)) * config migration from `myservers.cfg` ([#1440](#1440)) ([c4c9984](c4c9984)) * **connect:** fatal race-condition in websocket disposal ([#1462](#1462)) ([0ec0de9](0ec0de9)) * **connect:** mothership connection ([#1464](#1464)) ([7be8bc8](7be8bc8)) * console hidden ([9b85e00](9b85e00)) * debounce is too long ([#1426](#1426)) ([f12d231](f12d231)) * delete legacy connect keys and ensure description ([22fe91c](22fe91c)) * **deps:** pin dependencies ([#1465](#1465)) ([ba75a40](ba75a40)) * **deps:** pin dependencies ([#1470](#1470)) ([412b329](412b329)) * **deps:** storybook v9 ([#1476](#1476)) ([45bb49b](45bb49b)) * **deps:** update all non-major dependencies ([#1366](#1366)) ([291ee47](291ee47)) * **deps:** update all non-major dependencies ([#1379](#1379)) ([8f70326](8f70326)) * **deps:** update all non-major dependencies ([#1389](#1389)) ([cb43f95](cb43f95)) * **deps:** update all non-major dependencies ([#1399](#1399)) ([68df344](68df344)) * **deps:** update dependency @types/diff to v8 ([#1393](#1393)) ([00da27d](00da27d)) * **deps:** update dependency cache-manager to v7 ([#1413](#1413)) ([9492c2a](9492c2a)) * **deps:** update dependency commander to v14 ([#1394](#1394)) ([106ea09](106ea09)) * **deps:** update dependency diff to v8 ([#1386](#1386)) ([e580f64](e580f64)) * **deps:** update dependency dotenv to v17 ([#1474](#1474)) ([d613bfa](d613bfa)) * **deps:** update dependency lucide-vue-next to ^0.509.0 ([#1383](#1383)) ([469333a](469333a)) * **deps:** update dependency marked to v16 ([#1444](#1444)) ([453a5b2](453a5b2)) * **deps:** update dependency shadcn-vue to v2 ([#1302](#1302)) ([26ecf77](26ecf77)) * **deps:** update dependency vue-sonner to v2 ([#1401](#1401)) ([53ca414](53ca414)) * disable file changes on Unraid 7.2 ([#1382](#1382)) ([02de89d](02de89d)) * do not start API with doinst.sh ([7d88b33](7d88b33)) * do not uninstall fully on 7.2 ([#1484](#1484)) ([2263881](2263881)) * drop console with terser ([a87d455](a87d455)) * error logs from `cloud` query when connect is not installed ([#1450](#1450)) ([719f460](719f460)) * flash backup integration with Unraid Connect config ([#1448](#1448)) ([038c582](038c582)) * header padding regression ([#1477](#1477)) ([e791cc6](e791cc6)) * incorrect state merging in redux store ([#1437](#1437)) ([17b7428](17b7428)) * lanip copy button not present ([#1459](#1459)) ([a280786](a280786)) * move to bigint scalar ([b625227](b625227)) * node_modules dir removed on plugin update ([#1406](#1406)) ([7b005cb](7b005cb)) * omit Connect actions in UPC when plugin is not installed ([#1417](#1417)) ([8c8a527](8c8a527)) * parsing of `ssoEnabled` in state.php ([#1455](#1455)) ([f542c8e](f542c8e)) * pin ranges ([#1460](#1460)) ([f88400e](f88400e)) * pr plugin promotion workflow ([#1456](#1456)) ([13bd9bb](13bd9bb)) * proper fallback if missing paths config modules ([7067e9e](7067e9e)) * rc.unraid-api now cleans up older dependencies ([#1404](#1404)) ([83076bb](83076bb)) * remote access lifecycle during boot & shutdown ([#1422](#1422)) ([7bc583b](7bc583b)) * sign out correctly on error ([#1452](#1452)) ([d08fc94](d08fc94)) * simplify usb listing ([#1402](#1402)) ([5355115](5355115)) * theme issues when sent from graph ([#1424](#1424)) ([75ad838](75ad838)) * **ui:** notifications positioning regression ([#1445](#1445)) ([f73e5e0](f73e5e0)) * use some instead of every for connect detection ([9ce2fee](9ce2fee)) ### Reverts * revert package.json dependency updates from commit 711cc9a for api and packages/* ([94420e4](94420e4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit