feat(web): show Hub SQLite storage usage in Settings - #1225
Conversation
There was a problem hiding this comment.
Findings
- [Major] Storage route leaks hub-wide SQLite state across namespaces — HAPI documents namespaces as multi-user isolation, and the existing session/machine APIs scope reads through
c.get('namespace'). This new authenticated route does not check the namespace and returns the global SQLite file path plus aggregate DB/WAL/SHM sizes, so any namespace can learn host path details and cross-namespace storage/activity signals. Evidence:hub/src/web/routes/storage.ts:18,hub/src/web/routes/storage.ts:27.
Suggested fix:export type SqliteStorageUsageResponse = { databaseBytes: number walBytes: number shmBytes: number totalBytes: number } app.get('/storage/sqlite', async (c) => { if (c.get('namespace') !== 'default') { return c.json({ error: 'Storage usage is only available to the hub owner' }, 403) } const response: SqliteStorageUsageResponse = { databaseBytes, walBytes, shmBytes, totalBytes: databaseBytes + walBytes + shmBytes, } return c.json(response) })
- [Minor] Storage path row can overflow on narrow screens —
max-w-80 truncateis applied to an inline<code>, so the max width/truncation will not reliably constrain the absolute path. BecauseSettingsRowrenders trailing content asshrink-0, a long DB path can force horizontal overflow. Evidence:web/src/routes/settings/storage.tsx:35.
Suggested fix:<SettingsRow label={t('settings.storage.path')} trailing={ <code className="block max-w-[min(20rem,55vw)] truncate text-xs text-[var(--app-hint)]" title={query.data.path} > {query.data.path} </code> } />
Questions
- None.
Summary
- Review mode: initial
- Full diff reviewed. Residual risk: the new web settings page has no browser/screenshot coverage, and the hub test covers only the happy path.
Testing
- Not run (automation review); PR body reports
bun typecheck,bun run test, andgit diff --check.
HAPI Bot
| fileSize(`${dbPath}-shm`), | ||
| ]) | ||
| const response: SqliteStorageUsageResponse = { | ||
| path: dbPath, |
There was a problem hiding this comment.
[MAJOR] HAPI documents namespaces as multi-user isolation, and the existing session/machine APIs scope reads through c.get('namespace'). This route is behind auth, but it never checks the namespace and returns the global SQLite file path plus aggregate DB/WAL/SHM sizes, so any namespace can learn host path details and cross-namespace storage/activity signals.
Suggested fix:
export type SqliteStorageUsageResponse = {
databaseBytes: number
walBytes: number
shmBytes: number
totalBytes: number
}
app.get('/storage/sqlite', async (c) => {
if (c.get('namespace') !== 'default') {
return c.json({ error: 'Storage usage is only available to the hub owner' }, 403)
}
const response: SqliteStorageUsageResponse = {
databaseBytes,
walBytes,
shmBytes,
totalBytes: databaseBytes + walBytes + shmBytes,
}
return c.json(response)
})| <SettingsRow label={t('settings.storage.database')} trailing={<span className="text-[var(--app-hint)]">{formatFileSize(query.data.databaseBytes)}</span>} /> | ||
| <SettingsRow label={t('settings.storage.wal')} trailing={<span className="text-[var(--app-hint)]">{formatFileSize(query.data.walBytes)}</span>} /> | ||
| <SettingsRow label={t('settings.storage.shm')} trailing={<span className="text-[var(--app-hint)]">{formatFileSize(query.data.shmBytes)}</span>} /> | ||
| <SettingsRow label={t('settings.storage.path')} trailing={<code className="max-w-80 truncate text-xs text-[var(--app-hint)]">{query.data.path}</code>} /> |
There was a problem hiding this comment.
[MINOR] max-w-80 truncate is applied to an inline <code>, so the max width/truncation will not reliably constrain the absolute path. Since SettingsRow wraps trailing content in a shrink-0 container, a long DB path can force horizontal overflow on small screens.
Suggested fix:
<SettingsRow
label={t('settings.storage.path')}
trailing={
<code
className="block max-w-[min(20rem,55vw)] truncate text-xs text-[var(--app-hint)]"
title={query.data.path}
>
{query.data.path}
</code>
}
/>There was a problem hiding this comment.
Findings
- No issues found in the latest diff.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Full latest diff reviewed, including the follow-up changes since the prior HAPI Bot review. The earlier namespace and path-overflow findings are addressed at current head.
- Residual risk: the new web settings page has no browser/screenshot coverage; backend coverage exercises the storage happy path and non-default namespace rejection.
Testing
- Not run (automation review); PR body reports
bun typecheck,bun run test, andgit diff --check.
HAPI Bot
b0473bb to
e7b7a83
Compare
There was a problem hiding this comment.
Findings
- [Minor] Hide owner-only Storage from tenant namespaces —
web/src/routes/settings/categories.ts:7adds the Storage category for every authenticated web user, but the new endpoint rejects any namespace other thandefaultathub/src/web/routes/storage.ts:19, so tenant users get a Settings entry that can only render a 403 error page.
Suggested fix:// After threading the authenticated namespace into app/settings context: const visibleCategories = settingsCategories.filter( (category) => category.id !== 'storage' || namespace === 'default' )
Questions
- None.
Summary
Review mode: follow-up after new commits
Full latest diff reviewed, including follow-up context from the prior HAPI Bot pass. One minor regression found: the new owner-only storage page is advertised to non-owner namespaces.
Testing
- Not run (automation review; did not execute PR code). PR body reports
bun typecheck,bun run test, andgit diff --check.
HAPI Bot
| { id: 'chat', path: '/settings/chat', titleKey: 'settings.chat.title' }, | ||
| { id: 'voice', path: '/settings/voice', titleKey: 'settings.voice.title' }, | ||
| { id: 'machines', path: '/settings/machines', titleKey: 'settings.machines.title' }, | ||
| { id: 'storage', path: '/settings/storage', titleKey: 'settings.storage.title' }, |
There was a problem hiding this comment.
[MINOR] This exposes the Storage settings entry to every namespace, but the new hub route returns 403 whenever namespace !== "default" (hub/src/web/routes/storage.ts:19). A tenant user can therefore navigate to /settings/storage, but the page can only show an owner-only error. Please gate the category/page from the authenticated namespace instead of advertising a guaranteed failure.
Suggested fix:
// After threading the authenticated namespace into app/settings context:
const visibleCategories = settingsCategories.filter(
(category) => category.id !== 'storage' || namespace === 'default'
)There was a problem hiding this comment.
Findings
- No issues found.
Questions
- None.
Summary
Review mode: follow-up after new commits
Full latest diff reviewed, including the prior HAPI Bot review and compare diff. No issues found in added/modified lines. Residual risk: I did not manually exercise the settings UI in a browser during this automation review.
Testing
- Not run (automation review; did not execute PR code). PR body reports
bun typecheck,bun run test, andgit diff --check.
HAPI Bot
Summary
Fixes #1224
Root cause / impact
The Hub already knows the effective SQLite path, but exposed no disk-usage information to authenticated Web clients. Operators therefore had to log into the Hub host and inspect the database and sidecars manually. This keeps the feature Hub-only: runners do not own HAPI's SQLite database.
Validation
bun typecheckbun run test(CLI 1,527; Hub 644; Web 1,560; Shared 142)git diff --check.github/prompts/codex-pr-review.mdOne unrelated CLI test timed out once during the first full rerun; its targeted rerun and the subsequent full suite both passed.
AI disclosure
Implemented with AI assistance. The agent inspected the existing Settings routing, Hub configuration, authenticated route registration, API client, and tests; then ran the repository's complete pre-push gates.