Skip to content

fix(dashboard): reduce idle disk writes - #60885

Closed
embwl0x wants to merge 10 commits into
NousResearch:mainfrom
embwl0x:agent/reduce-idle-dashboard-writes
Closed

fix(dashboard): reduce idle disk writes#60885
embwl0x wants to merge 10 commits into
NousResearch:mainfrom
embwl0x:agent/reduce-idle-dashboard-writes

Conversation

@embwl0x

@embwl0x embwl0x commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #58733.

Summary

  • keep /api/status session counting read-only so health/status polling does not open state.db as a writer or checkpoint the WAL
  • make read-only SessionDB.close() skip TRUNCATE WAL checkpointing to preserve the read-only contract
  • keep dashboard MCP discovery lazy until a real chat/socket path needs tools, while preserving MCP startup before agent construction
  • preserve the newer helper-based status/session counting path from current main

Verification

  • uv run python -m py_compile hermes_state.py hermes_cli/web_server.py hermes_cli/main.py tests/test_hermes_state.py tests/hermes_cli/test_web_server.py tests/hermes_cli/test_dashboard_unified_launch.py tests/test_tui_gateway_ws.py
  • focused read-only/status regression selection — 5 passed
  • uv run --extra dev pytest tests/hermes_cli/test_dashboard_unified_launch.py tests/test_tui_gateway_ws.py tests/hermes_cli/test_mcp_startup.py -q — 18 passed
  • uv run --extra dev pytest tests/hermes_cli/test_web_server.py -k '(get_status or get_sessions or DesktopCronTicker) and not memory_status' -q — 16 passed, 1 warning
  • uv run --extra dev pytest tests/test_hermes_state.py::TestApplyWalProbe tests/test_hermes_state_wal_fallback.py::TestSessionDbUsesWalFallback -q — 12 passed
  • git diff --check origin/main..HEAD
  • gitleaks git --log-opts='origin/main..HEAD' --redact . — no leaks found

@alt-glitch alt-glitch added type/perf Performance improvement or optimization comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jul 8, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Comment (token read-only)

PR 60885 reduces idle disk writes in the dashboard. Performance fix for disk I/O. Well-scoped (6 files, 75 additions, 36 deletions). No security issues or debug artifacts detected.

LGTM - awaiting maintainer approval.

@embwl0x
embwl0x force-pushed the agent/reduce-idle-dashboard-writes branch 2 times, most recently from 538ff1c to ba09c23 Compare July 10, 2026 09:33
@embwl0x
embwl0x marked this pull request as ready for review July 10, 2026 09:33
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused idle-I/O fix. The current-main path is verified: /api/status constructs SessionDB(read_only=True) at hermes_cli/web_server.py:1199, but SessionDB.close() still unconditionally runs PRAGMA wal_checkpoint(TRUNCATE) at hermes_state.py:1252. The proposed guard directly restores the documented read-only behavior.

The MCP deferral is also coherent: tui_gateway/ws.py:303-317 already starts the shared background discovery on the actual dashboard/desktop WebSocket path before agent construction, so removing the dashboard-startup call does not remove MCP availability for chat users.

The PR's focused regression test covers the new close contract, and the change adds no configuration surface or model-tool footprint.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 10, 2026
@embwl0x

embwl0x commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

The current required-check failure is an upstream baseline regression, not a dashboard-diff failure: the API-server enrollment hardening requires a 16+ character key, while an older gateway config test still uses the weak fixture secret-key. Current main fails the same test. Maintainer PR #70273 contains the focused, production-code-neutral correction; all eight Python slices and the aggregate required-check gate pass there. This dashboard PR remains scope-clean and can be refreshed after #70273 merges.

…aseline-fix

# Conflicts:
#	tests/test_hermes_state.py
@embwl0x

embwl0x commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current origin/main at 760112adb in merge commit 1201b0960.

Conflict resolution preserved both sides of tests/test_hermes_state.py: this PR's read-only close regression and main's newer display-metadata read-path coverage. Local verification:

  • tests/hermes_cli/test_dashboard_unified_launch.py: 9 passed
  • tests/test_hermes_state.py: 461 passed
  • focused status/read-only regressions: 2 passed
  • tests/hermes_cli/test_web_server.py: 515 passed, 1 sandbox-only failure in profile deletion; the same test fails identically on untouched origin/main because macOS psutil.process_iter() is denied by the unattended sandbox
  • publish gate: passed; gitleaks found no leaks

Overlap note: #67903 also guards read-only SessionDB.close() and expands genuine read-only opens across dashboard session endpoints. This earlier PR remains materially distinct because it also defers dashboard MCP discovery until the actual chat/WebSocket path; the sweeper review confirmed that path preserves MCP availability. GitHub now reports this head MERGEABLE; replacement CI is running.

@embwl0x

embwl0x commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the current-main conflict in 173798131.

The only content conflict was SessionDB.close(). The resolution preserves both upstream's token-writer drain/atexit cleanup and this PR's invariant that read-only connections never request a TRUNCATE WAL checkpoint. The dashboard MCP-startup deferral remains unchanged.

Verification on current origin/main 2e9559adf:

  • dashboard/state/web-server matrix: 1,025 passed; one profile-deletion test fails because the unattended macOS sandbox denies psutil.process_iter() via sysctl()
  • that same test fails identically on an untouched worktree at exact upstream base ad6df5eb9, confirming it is unrelated to the PR
  • WAL checkpoint + async token-accounting suites: 29 passed
  • contribution publish gate passed: current main contained, public commit identity, clean PR diff, metadata scan, and gitleaks

GitHub now reports the refreshed head mergeable; replacement CI is running.

@embwl0x

embwl0x commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the current-main conflict in 234673026.

The only content conflict was at the end of tests/test_hermes_state.py. The resolution preserves both this PR's read-only-close checkpoint regression and current main's gateway-routing primary-key healing regressions. The dashboard MCP-startup deferral remains unchanged.

Verification on current origin/main 015718066:

  • affected-file matrix: 1,016 passed
  • one profile-deletion test failed because the unattended macOS sandbox denies psutil.process_iter() via sysctl()
  • that same test fails identically on an untouched worktree at exact upstream base 015718066, confirming it is unrelated to this PR
  • existing-PR publish gate passed with the prior sweeper review as the concrete materially-distinct-scope follow-up; git diff --check, public identity, metadata scan, and gitleaks all passed
  • replacement CI: all eight Python slices, Desktop Playwright, lints, e2e, security checks, and the aggregate All required checks pass job succeeded

GitHub reports the refreshed head MERGEABLE. Only the two optional Docker builds remain in progress.

# Conflicts:
#	tests/hermes_cli/test_dashboard_unified_launch.py
#	tests/test_hermes_state.py
@embwl0x

embwl0x commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the current-main conflict in 8212653aa.

The production changes merged cleanly. The two test conflicts came from current main's suite pruning; the resolution keeps only this PR's behavior regressions (lazy dashboard MCP startup and read-only close without a WAL checkpoint) and does not resurrect the pruned legacy cases.

Verification on current origin/main 9accf79d8:

  • tests/test_hermes_state.py: 138 passed
  • tests/hermes_cli/test_web_server.py: 104 passed
  • dashboard/WebSocket MCP matrix: 504 passed; one order-dependent test_write_json_serializes_concurrent_writes failure
  • the exact serialization test passes alone, and the full-file failure reproduced three times in an untouched worktree at exact upstream base 9accf79d8, confirming it is an unrelated current-main order-pollution baseline
  • Ruff, current-main diff checks, existing-PR publish gate, metadata scan, and gitleaks passed

GitHub reports the refreshed head MERGEABLE; replacement CI is running.

@embwl0x

embwl0x commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Replacement CI for 8212653aa completed successfully: all eight Python slices, Desktop E2E, both Docker architectures, lint, security, and the aggregate required-check gate passed. GitHub reports the PR mergeable.

@embwl0x

embwl0x commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of the earlier #60602. Current main now independently includes the read-only SessionDB.close checkpoint guard and regression that were this PR’s distinct state-storage portion. After composing current main, the remaining diff is only the eager dashboard MCP-discovery removal plus the lazy WebSocket-path regression—the same invariant and implementation already carried by #60602. Preserving the earlier contributor’s work rather than maintaining an exact duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Dashboard process causes continuous disk writes on NAS-backed ZFS storage, even when idle and unvisited

4 participants