Skip to content

fix: simplify usb listing #1402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elibosley merged 1 commit into from
May 22, 2025
Merged

fix: simplify usb listing #1402

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 36 additions & 66 deletions api/src/graphql/resolvers/query/info.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,86 +325,56 @@ export const generateDevices = async (): Promise<Devices> => {
};
};

const parseDeviceLine = (line: Readonly<string>): { value: string; string: string } => {
const emptyLine = { value: '', string: '' };

// If the line is blank return nothing
if (!line) {
return emptyLine;
}

// Parse the line
const [, _] = line.split(/[ \t]{2,}/).filter(Boolean);

const match = _.match(/^(\S+)\s(.*)/)?.slice(1);

// If there's no match return nothing
if (!match) {
return emptyLine;
}

return {
value: match[0],
string: match[1],
};
};

// Add extra fields to device
const parseDevice = (device: Readonly<PciDevice>) => {
// Simplified basic device parsing without verbose details
const parseBasicDevice = async (device: PciDevice): Promise<PciDevice> => {
const modifiedDevice: PciDevice = {
...device,
};
const info = execaCommandSync(`lsusb -d ${device.id} -v`).stdout.split('\n');
const deviceName = device.name.trim();
const iSerial = parseDeviceLine(info.filter((line) => line.includes('iSerial'))[0]);
const iProduct = parseDeviceLine(info.filter((line) => line.includes('iProduct'))[0]);
const iManufacturer = parseDeviceLine(
info.filter((line) => line.includes('iManufacturer'))[0]
);
const idProduct = parseDeviceLine(info.filter((line) => line.includes('idProduct'))[0]);
const idVendor = parseDeviceLine(info.filter((line) => line.includes('idVendor'))[0]);
const serial = `${iSerial.string.slice(8).slice(0, 4)}-${iSerial.string
.slice(8)
.slice(4)}`;
const guid = `${idVendor.value.slice(2)}-${idProduct.value.slice(2)}-${serial}`;

modifiedDevice.serial = iSerial.string;
modifiedDevice.product = iProduct.string;
modifiedDevice.manufacturer = iManufacturer.string;
modifiedDevice.guid = guid;

// Set name if missing
if (deviceName === '') {
modifiedDevice.name = `${iProduct.string} ${iManufacturer.string}`.trim();
}

// Name still blank? Replace using fallback default
if (deviceName === '') {
modifiedDevice.name = '[unnamed device]';
// 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)}`;
}
Comment on lines +334 to 341
Copy link
Contributor

@coderabbitai coderabbitai bot May 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.


// Ensure name is trimmed
modifiedDevice.name = device.name.trim();
// Use the name from basic lsusb output
const deviceName = device.name?.trim() || '';
modifiedDevice.name = deviceName || '[unnamed device]';

return modifiedDevice;
};

const parseUsbDevices = (stdout: string) =>
stdout.split('\n').map((line) => {
const regex = new RegExp(/^.+: ID (?<id>\S+)(?<name>.*)$/);
const result = regex.exec(line);
return result?.groups as unknown as PciDevice;
}) ?? [];
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) ?? [];
Comment on lines +350 to +365
Copy link
Contributor

@coderabbitai coderabbitai bot May 22, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Clean up regex capture & output shape.

  1. The capture group is named n, which then leaks as an unnecessary property after the spread.
  2. Any extra whitespace between the product ID and the name isn’t consumed, so name can 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.


// Get all usb devices
// Get all usb devices with basic listing only
const usbDevices = await execa('lsusb')
.then(async ({ stdout }) =>
parseUsbDevices(stdout)
.map(parseDevice)
.then(async ({ stdout }) => {
const devices = parseUsbDevices(stdout);
// Process devices in parallel
const processedDevices = await Promise.all(devices.map(parseBasicDevice));
return processedDevices
.filter(filterBootDrive)
.filter(filterUsbHubs)
.map(sanitizeVendorName)
)
.map(sanitizeVendorName);
})
.catch(() => []);

return usbDevices;
Expand Down
Loading