Comfy-Org/desktop

Fork 0

mirror of https://github.com/Comfy-Org/desktop.git synced 2026-11-04 23:47:43 +00:00

[GH-ISSUE #1403] Disk space validation can falsely fail when fsSize mount paths don’t match install path #399

New issue

Closed

opened 2026-10-31 22:12:30 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 22:12:30 +00:00

Owner

Originally created by @benceruleanlu on GitHub (Nov 19, 2025).
Original GitHub issue: https://github.com/Comfy-Org/desktop/issues/1403

Summary

VALIDATE_INSTALL_PATH relies on systeminformation.fsSize() + mount comparison to pick a disk and compute free space. On some platforms/layouts (especially macOS APFS and paths under symlinks or non-trivial mountpoints), this match can fail, leaving freeSpace at -1 and causing valid install locations to be rejected.

Current behavior

In src/handlers/pathHandlers.ts:

normalizeMountPoint() normalizes disk.mount and normalizePathForComparison() normalizes the user-chosen path.
The code picks a disk via:

const disks = await si.fsSize();
if (disks.length) {
  const disk = disks.find((disk) => {
    const normalizedMount = normalizeMountPoint(disk.mount);
    return normalizedMount && normalizedPath && isPathInside(normalizedPath, normalizedMount);
  });
  if (disk) result.freeSpace = disk.available;
} else {
  result.freeSpace = result.requiredSpace; // fallback
}

If no disk matches, result.freeSpace stays at -1 (the initial value), and later the validation logic does:

const hasBlockingIssues =
  result.cannotWrite ||
  result.parentMissing ||
  (!bypassSpaceCheck && result.freeSpace < requiredSpace) ||
  Boolean(result.error) ||
  result.isNonDefaultDrive ||
  result.isOneDrive ||
  result.isInsideAppInstallDir ||
  result.isInsideUpdaterCache;

So freeSpace === -1 ⇒ < requiredSpace and the path is treated as invalid, even if there is plenty of disk space.

Why this is fragile

The matching assumes that disk.mount is a simple ancestor of the install path in filesystem terms. That’s not always true:

macOS APFS layouts have system/data volumes and mount paths like /System/Volumes/Data while user paths are under /Users/....
Paths may go through symlinks where fsSize().mount does not appear as a simple prefix.

In those cases the code doesn’t find any disk, freeSpace stays -1, and users see a spurious “insufficient free space” error.

Expected behavior

If we cannot confidently resolve free space for a path, we should:
- Either fall back to a reasonable default disk (e.g. the root / / system drive) when fsSize() returns data but the heuristic fails, or
- Treat freeSpace === -1 as “unknown” and not a hard error (or at least only show a warning rather than blocking installation).

Possible fixes

A few options (not mutually exclusive):

Fallback to a default disk when no mount matches
- After disks.find(...), if no disk is found, pick a fallback (e.g. the disk whose mount === '/' on POSIX, or the disk whose mount matches the system drive on Windows) and use its available value.
Change the semantics of freeSpace === -1
- In VALIDATE_INSTALL_PATH, treat freeSpace === -1 as “unknown” instead of “0”, so it doesn’t trigger the freeSpace < requiredSpace branch.
- The frontend (InstallLocationPicker.vue) could show a softer warning like “Could not determine free space” instead of a blocking error.
Improve mount/path matching
- Use fs.stat/statfs-style information or realpath resolution to map the chosen directory to a device, then compare against fsSize() results instead of relying entirely on path prefixes.

Impact

Users on certain macOS or custom filesystem setups can be locked out of installation on perfectly valid paths because disk space can’t be resolved.
From the UI perspective, they see an “insufficient space” error that is misleading.

Suggested work

Add tests in tests/unit/handlers/pathHandler.test.ts that simulate:
- fsSize() returning a mount that doesn’t look like a prefix of the install path.
- freeSpace remaining -1 and verifying we don’t block installation solely because space is unknown.
Implement one of the fallback strategies above and align the frontend messaging accordingly.

┆Issue is synchronized with this Notion page by Unito

Originally created by @benceruleanlu on GitHub (Nov 19, 2025). Original GitHub issue: https://github.com/Comfy-Org/desktop/issues/1403 ### Summary `VALIDATE_INSTALL_PATH` relies on `systeminformation.fsSize()` + mount comparison to pick a disk and compute free space. On some platforms/layouts (especially macOS APFS and paths under symlinks or non-trivial mountpoints), this match can fail, leaving `freeSpace` at `-1` and causing valid install locations to be rejected. ### Current behavior In `src/handlers/pathHandlers.ts`: - `normalizeMountPoint()` normalizes `disk.mount` and `normalizePathForComparison()` normalizes the user-chosen path. - The code picks a disk via: ```ts const disks = await si.fsSize(); if (disks.length) { const disk = disks.find((disk) => { const normalizedMount = normalizeMountPoint(disk.mount); return normalizedMount && normalizedPath && isPathInside(normalizedPath, normalizedMount); }); if (disk) result.freeSpace = disk.available; } else { result.freeSpace = result.requiredSpace; // fallback } ``` - If no disk matches, `result.freeSpace` stays at `-1` (the initial value), and later the validation logic does: ```ts const hasBlockingIssues = result.cannotWrite || result.parentMissing || (!bypassSpaceCheck && result.freeSpace < requiredSpace) || Boolean(result.error) || result.isNonDefaultDrive || result.isOneDrive || result.isInsideAppInstallDir || result.isInsideUpdaterCache; ``` - So `freeSpace === -1` ⇒ `< requiredSpace` and the path is treated as invalid, even if there is plenty of disk space. ### Why this is fragile The matching assumes that `disk.mount` is a simple ancestor of the install path in filesystem terms. That’s not always true: - macOS APFS layouts have system/data volumes and mount paths like `/System/Volumes/Data` while user paths are under `/Users/...`. - Paths may go through symlinks where `fsSize().mount` does not appear as a simple prefix. In those cases the code doesn’t find any disk, `freeSpace` stays `-1`, and users see a spurious “insufficient free space” error. ### Expected behavior - If we cannot confidently resolve free space for a path, we should: - Either fall back to a reasonable default disk (e.g. the root `/` / system drive) when `fsSize()` returns data but the heuristic fails, or - Treat `freeSpace === -1` as “unknown” and **not** a hard error (or at least only show a warning rather than blocking installation). ### Possible fixes A few options (not mutually exclusive): 1. **Fallback to a default disk when no mount matches** - After `disks.find(...)`, if no disk is found, pick a fallback (e.g. the disk whose `mount === '/'` on POSIX, or the disk whose mount matches the system drive on Windows) and use its `available` value. 2. **Change the semantics of `freeSpace === -1`** - In `VALIDATE_INSTALL_PATH`, treat `freeSpace === -1` as “unknown” instead of “0”, so it doesn’t trigger the `freeSpace < requiredSpace` branch. - The frontend (`InstallLocationPicker.vue`) could show a softer warning like “Could not determine free space” instead of a blocking error. 3. **Improve mount/path matching** - Use `fs.stat`/`statfs`-style information or realpath resolution to map the chosen directory to a device, then compare against `fsSize()` results instead of relying entirely on path prefixes. ### Impact - Users on certain macOS or custom filesystem setups can be locked out of installation on perfectly valid paths because disk space can’t be resolved. - From the UI perspective, they see an “insufficient space” error that is misleading. ### Suggested work - Add tests in `tests/unit/handlers/pathHandler.test.ts` that simulate: - `fsSize()` returning a mount that doesn’t look like a prefix of the install path. - `freeSpace` remaining `-1` and verifying we don’t block installation solely because space is unknown. - Implement one of the fallback strategies above and align the frontend messaging accordingly. ┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-1403-Disk-space-validation-can-falsely-fail-when-fsSize-mount-paths-don-t-match-install-pat-2b06d73d36508172aedaede70d48eb4f) by [Unito](https://www.unito.io)

bsakaguchi

2026-10-31 22:12:30 +00:00

closed this issue
added the
bug

area: desktop-app

platform: cross-platform

component: installer
labels

bsakaguchi commented

2026-10-31 22:12:31 +00:00

Author

Owner

@dosubot[bot] commented on GitHub (Feb 18, 2026):

Hi, @benceruleanlu. I'm Dosu, and I'm helping the desktop team manage their backlog and am marking this issue as stale.

Issue Summary:

You reported that disk space validation fails on macOS with APFS due to mount path mismatches.
The problem is caused by the assumption that mount points are simple ancestors of the install path.
This leads to incorrect free space reporting and rejected install locations.
You proposed falling back to a default disk when free space cannot be reliably determined.
No further updates or comments have been made on this issue.

Next Steps:

Please let me know if this issue is still relevant with the latest version of the desktop app by commenting here.
If I don’t hear back within 7 days, I will automatically close this issue.

Thank you for your understanding and contribution!

@dosubot[bot] commented on GitHub (Feb 18, 2026): Hi, @benceruleanlu. I'm [Dosu](https://dosu.dev), and I'm helping the desktop team manage their backlog and am marking this issue as stale. **Issue Summary:** - You reported that disk space validation fails on macOS with APFS due to mount path mismatches. - The problem is caused by the assumption that mount points are simple ancestors of the install path. - This leads to incorrect free space reporting and rejected install locations. - You proposed falling back to a default disk when free space cannot be reliably determined. - No further updates or comments have been made on this issue. **Next Steps:** - Please let me know if this issue is still relevant with the latest version of the desktop app by commenting here. - If I don’t hear back within 7 days, I will automatically close this issue. Thank you for your understanding and contribution!

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/desktop#399

No description provided.

Rows
Columns