Skip to content

fix: honor active repo and dashboard chat theme - #22650

Open
sgtworkman wants to merge 1 commit into
NousResearch:mainfrom
sgtworkman:fix/dashboard-chat-theme-update-check
Open

fix: honor active repo and dashboard chat theme#22650
sgtworkman wants to merge 1 commit into
NousResearch:mainfrom
sgtworkman:fix/dashboard-chat-theme-update-check

Conversation

@sgtworkman

Copy link
Copy Markdown
Contributor

Summary

  • Keeps the CLI update check scoped to the active Hermes checkout instead of a stale profile clone
  • Adds repo-aware update-check cache invalidation
  • Carries the dashboard chat theme plumbing through the React web build

Test Plan

  • scripts/run_tests.sh tests/hermes_cli/test_update_check.py tests/hermes_cli/test_web_server.py
  • cd web && npm run build

@sgtworkman

Copy link
Copy Markdown
Contributor Author

CI is awaiting maintainer approval for Actions on this fork PR (completed/action_required zero-duration runs). Local validation already passed:

  • scripts/run_tests.sh tests/hermes_cli/test_update_check.py tests/hermes_cli/test_web_server.py — 147 passed
  • cd web && npm run build — passed

@sgtworkman
sgtworkman force-pushed the fix/dashboard-chat-theme-update-check branch from e4b9856 to 8c8d5ea Compare May 9, 2026 16:26
@sgtworkman

Copy link
Copy Markdown
Contributor Author

Rebased onto latest origin/main after approval and force-pushed with lease. Current validation:\n\n- scripts/run_tests.sh tests/hermes_cli/test_update_check.py tests/hermes_cli/test_web_server.py — 147 passed\n- cd web && npm run build — passed\n\nBranch is clean and current with fork upstream; PR is still awaiting GitHub Actions/maintainer run approval (no checks reported yet).

@sgtworkman
sgtworkman force-pushed the fix/dashboard-chat-theme-update-check branch 2 times, most recently from a50b977 to 478d1cd Compare May 9, 2026 19:26
@sgtworkman

Copy link
Copy Markdown
Contributor Author

Hermes validation update

Final review/readiness pass completed after rebasing onto current main.

Status

  • Branch: fix/dashboard-chat-theme-update-check
  • Head: 478d1cdb295ab6381d7b6585a9664cd752d132c9
  • Base: main @ dae94fa6526dec0c7660276a4d875cebc6e344f6
  • Diff vs main: 0 behind / 1 ahead
  • Working tree: clean

Validation

  • scripts/run_tests.sh tests/hermes_cli/test_update_check.py tests/hermes_cli/test_web_server.py -q
    • 148 passed
  • cd web && npm run build
    • tsc -b OK
    • vite build OK

Review notes

  • Added a final regression fix during review: legacy repo-less .update_check cache entries are no longer trusted when no active local git checkout can be resolved.
  • Added coverage for that no-repo legacy-cache edge case.
  • Added-line security scan found no secrets, shell/eval/deserialization/SQL injection patterns, XSS sink additions, debug logs, TODO/FIXME markers, or conflict markers.

Ready for maintainer review/merge from my side.

@sgtworkman
sgtworkman force-pushed the fix/dashboard-chat-theme-update-check branch from 478d1cd to 59137b8 Compare May 10, 2026 02:15
@sgtworkman

Copy link
Copy Markdown
Contributor Author

Validation refresh

Rebased this PR onto latest origin/main and pushed the refreshed branch.

Head: 59137b838
Branch divergence after rebase: 0 behind / 1 ahead vs origin/main.

Local validation:

scripts/run_tests.sh tests/hermes_cli/test_update_check.py tests/hermes_cli/test_web_server.py -v

Result: 148 passed.

npm run build

Result: passed. Vite emitted only the existing large-chunk warning.

npx eslint src/pages/ChatPage.tsx src/themes/types.ts

Result: passed.

Notes:

  • Static added-line security scan: 0 findings.
  • Independent final diff review: passed, no blocking security or logic issues.
  • Full npm run lint still fails on existing dashboard-wide baseline issues outside this PR; targeted changed-file lint is clean except web/src/themes/context.tsx has the pre-existing react-refresh/only-export-components export warning/error already present on origin/main.
  • GitHub Actions for this fork PR may still require maintainer approval before remote CI runs.

@sgtworkman

Copy link
Copy Markdown
Contributor Author

Validation refresh

Rebased onto latest origin/main after upstream advanced and force-pushed with lease.

Head: 49d639e92
Branch divergence vs origin/main: 0 behind / 1 ahead.

Local validation:

  • scripts/run_tests.sh tests/hermes_cli/test_update_check.py tests/hermes_cli/test_web_server.py -q145 passed
  • cd web && npm run build — passed. Vite emitted only the existing large-chunk warning.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 10, 2026
@sgtworkman

Copy link
Copy Markdown
Contributor Author

Local integration validation

Validated the current open PR stack locally while GitHub Actions remain maintainer-gated for fork PRs.

Temporary stack from fresh origin/main (44cdf555a) merged these PR heads cleanly, with no conflicts:

Combined targeted validation:

scripts/run_tests.sh \
  tests/hermes_cli/test_tools_config.py \
  tests/hermes_cli/test_update_check.py \
  tests/hermes_cli/test_web_server.py \
  tests/cron/test_cron_no_agent.py \
  tests/cron/test_cron_script.py -q

Result: 280 passed in 3.04s.

Dashboard build also passed:

cd web && npm run build

Result: passed; only the existing Vite chunk-size warning was emitted.

The temp worktree was removed after validation. No production/deploy action was performed.

@feiskyer

Copy link
Copy Markdown

+1 on this — just ran into the same issue. The hardcoded #0d2626 in TERMINAL_THEME clashes badly with non-teal dashboard themes (e.g. mono uses #0e0e0e, leaving a visible color seam around the terminal). Independently arrived at the same fix (derive xterm colors from the active palette via useTheme()). The componentStyles.terminal override layer in this PR is a nice touch for per-theme fine-tuning.

@sgtworkman
sgtworkman force-pushed the fix/dashboard-chat-theme-update-check branch from 1cea65a to 3378434 Compare May 14, 2026 01:48
@sgtworkman
sgtworkman force-pushed the fix/dashboard-chat-theme-update-check branch from 3378434 to 32d30f5 Compare May 14, 2026 03:34
@alt-glitch alt-glitch added the comp/dashboard Web dashboard / control panel UI (dashboard/, landing) label Jun 27, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The update-cache half remains relevant, but the dashboard half has been superseded by later main changes.

Problems

  • Current main already derives and live-updates dashboard xterm colors from terminalBackground / terminalForeground in web/src/pages/ChatPage.tsx:276 and web/src/pages/ChatPage.tsx:1157; those fields are declared in web/src/themes/types.ts:180-185.
  • The proposed componentStyles.terminal path conflicts with the current schema direction: hermes_cli/web_server.py:15835-15838 does not admit terminal as a component-style bucket.
  • The cache issue is still present: hermes_cli/banner.py:330-344 validates only timestamp, revision, and version before returning a cached update count, while active-checkout resolution occurs later at hermes_cli/banner.py:351-360.

Suggested changes

  • Salvage the repo-identity cache key and its tests against the current ver-aware cache format; keep the current pip/Docker behavior intact.
  • Omit the superseded dashboard component-style changes.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants