charles/claude-hooks

Fork 0

feat(storage): disk-usage card on dashboard #15

Merged

claude-desktop merged 1 commit from dev/8 into main

2026-04-17 12:26:48 +00:00

claude-desktop commented

2026-04-17 10:11:52 +00:00

Collaborator

Adds a GET /storage endpoint and a Storage card to the dashboard.

Closes #8

Changes

src/main.ts: treeSize() recursive walk, getStorageStats() with 60 s cache, GET /storage route
src/dashboard.html: Storage card (counts + human-formatted sizes, refreshes every 60 s)
src/main.test.ts: unit tests for treeSize and GET /storage

Adds a `GET /storage` endpoint and a Storage card to the dashboard. Closes #8 ## Changes - `src/main.ts`: `treeSize()` recursive walk, `getStorageStats()` with 60 s cache, `GET /storage` route - `src/dashboard.html`: Storage card (counts + human-formatted sizes, refreshes every 60 s) - `src/main.test.ts`: unit tests for `treeSize` and `GET /storage`

dev was assigned by claude-desktop

2026-04-17 10:11:52 +00:00

claude-desktop requested review from reviewer

2026-04-17 10:12:09 +00:00

claude-desktop commented

2026-04-17 10:15:10 +00:00

Author

Collaborator

src/main.ts — treeSize uses stat instead of lstat (line ~53)

stat(child) follows symlinks. If a project repo or worktree contains a git-tracked symlink that points to a directory, treeSize will recurse into it and count that directory’s contents as part of the tree. This inflates sizes, may traverse out-of-tree paths, and in a pathological cycle would loop forever.

Git worktrees do check out symlinks faithfully from the object store, so this is a realistic code path (e.g. a monorepo that symlinks packages/shared → ../shared).

Fix — replace stat with lstat in treeSize:

import { mkdir, readFile, readdir, lstat, symlink, writeFile } from "node:fs/promises";

// inside treeSize:
info = await lstat(child);
// lstat returns info about the symlink itself, not the target,
// so isDirectory() is false for symlinks → they are not traversed.

With lstat, a symlink entry has info.isSymbolicLink() === true and info.isDirectory() === false, so it falls into the else branch and contributes info.size (the link itself, typically 0 bytes on most filesystems for the stat size of a short path). That’s the correct du-like behaviour.

**`src/main.ts` — `treeSize` uses `stat` instead of `lstat` (line ~53)** `stat(child)` follows symlinks. If a project repo or worktree contains a git-tracked symlink that points to a directory, `treeSize` will recurse into it and count that directory’s contents as part of the tree. This inflates sizes, may traverse out-of-tree paths, and in a pathological cycle would loop forever. Git worktrees do check out symlinks faithfully from the object store, so this is a realistic code path (e.g. a monorepo that symlinks `packages/shared` → `../shared`). Fix — replace `stat` with `lstat` in `treeSize`: ```ts import { mkdir, readFile, readdir, lstat, symlink, writeFile } from "node:fs/promises"; // inside treeSize: info = await lstat(child); // lstat returns info about the symlink itself, not the target, // so isDirectory() is false for symlinks → they are not traversed. ``` With `lstat`, a symlink entry has `info.isSymbolicLink() === true` and `info.isDirectory() === false`, so it falls into the `else` branch and contributes `info.size` (the link itself, typically 0 bytes on most filesystems for the stat size of a short path). That’s the correct `du`-like behaviour.

claude-desktop commented

2026-04-17 10:15:20 +00:00

Author

Collaborator

src/dashboard.html — loadStorage() catch block leaves card in "loading…" state forever (line ~416)

The catch block is empty:

} catch {
  // silently ignore fetch errors
}

If the very first call to /storage fails (service restart, network blip), the card’s inner HTML is never replaced from the initial loading… placeholder — so the user sees no data and no indication of a problem.

Fix — show a brief error state on failure:

} catch {
  const row = document.getElementById("storageRow");
  if (row && row.innerHTML.includes("loading")) {
    row.innerHTML = '<span style="color:var(--text2);font-size:12px">unavailable</span>';
  }
}

(Subsequent failures after a successful render can stay silent, since the last-good data is still displayed.)

**`src/dashboard.html` — `loadStorage()` catch block leaves card in "loading…" state forever (line ~416)** The `catch` block is empty: ```js } catch { // silently ignore fetch errors } ``` If the very first call to `/storage` fails (service restart, network blip), the card’s inner HTML is never replaced from the initial `loading…` placeholder — so the user sees no data and no indication of a problem. Fix — show a brief error state on failure: ```js } catch { const row = document.getElementById("storageRow"); if (row && row.innerHTML.includes("loading")) { row.innerHTML = '<span style="color:var(--text2);font-size:12px">unavailable</span>'; } } ``` (Subsequent failures after a successful render can stay silent, since the last-good data is still displayed.)

claude-desktop commented

2026-04-17 10:15:39 +00:00

Author

Collaborator

Review: Request Changes

Acceptance criteria

All acceptance criteria from #8 are met:

GET /storage returns the correct JSON shape (cache_clones, worktrees, sessions with count/bytes)
Recursive directory walk (treeSize)
60 s cache (STORAGE_CACHE_TTL_MS)
Dashboard Storage card with human-formatted sizes
60 s refresh (setInterval(loadStorage, 60_000))
Visual style uses CSS custom properties matching existing cards
Unit tests for treeSize (known-content tempdir) and GET /storage shape

Issues

Must fix (correctness bug)

stat vs lstat in treeSize — see comment #4733. Using stat follows symlinks into directories, inflating sizes and potentially escaping the tree. Drop-in fix: change stat → lstat in the import and the call site.

Should fix (UX)

Silent loadStorage error — see comment #4734. First-call failure leaves the card in a permanent “loading…” state with no indication of a problem.

Everything else looks good

countDepth2 for worktrees is correct: worktrees/<agent>/<owner>__<name>__<branch> — depth-2 children are individual worktrees, verified against workdir.ts.
Cache invalidation logic is correct (TTL check before computing).
fmtBytes KB/MB/GB thresholds are correct (powers of 1024).
Tests are well-structured; the tempdir approach for treeSize is solid.

## Review: Request Changes ### Acceptance criteria All acceptance criteria from #8 are met: - [x] `GET /storage` returns the correct JSON shape (`cache_clones`, `worktrees`, `sessions` with `count`/`bytes`) - [x] Recursive directory walk (`treeSize`) - [x] 60 s cache (`STORAGE_CACHE_TTL_MS`) - [x] Dashboard Storage card with human-formatted sizes - [x] 60 s refresh (`setInterval(loadStorage, 60_000)`) - [x] Visual style uses CSS custom properties matching existing cards - [x] Unit tests for `treeSize` (known-content tempdir) and `GET /storage` shape ### Issues **Must fix (correctness bug)** - **`stat` vs `lstat` in `treeSize`** — see comment #4733. Using `stat` follows symlinks into directories, inflating sizes and potentially escaping the tree. Drop-in fix: change `stat` → `lstat` in the import and the call site. **Should fix (UX)** - **Silent `loadStorage` error** — see comment #4734. First-call failure leaves the card in a permanent “loading…” state with no indication of a problem. ### Everything else looks good - `countDepth2` for worktrees is correct: `worktrees/<agent>/<owner>__<name>__<branch>` — depth-2 children are individual worktrees, verified against `workdir.ts`. - Cache invalidation logic is correct (TTL check before computing). - `fmtBytes` KB/MB/GB thresholds are correct (powers of 1024). - Tests are well-structured; the tempdir approach for `treeSize` is solid.

charles force-pushed dev/8 from dbe4a2adcd

qa / qa (pull_request) Successful in 44s

Details

to 625239ed27

qa / qa (pull_request) Successful in 44s

Details

2026-04-17 12:21:08 +00:00

Compare

claude-desktop merged commit dd5156b6fb into main

2026-04-17 12:26:48 +00:00

claude-desktop deleted branch dev/8

2026-04-17 12:26:48 +00:00