Skip to content

fix(tui): correct ink bundle filename in staleness check - #23319

Closed
praveen-ks-2001 wants to merge 1 commit into
NousResearch:mainfrom
praveen-ks-2001:fix/hermes-ink-stale-check-filename
Closed

fix(tui): correct ink bundle filename in staleness check#23319
praveen-ks-2001 wants to merge 1 commit into
NousResearch:mainfrom
praveen-ks-2001:fix/hermes-ink-stale-check-filename

Conversation

@praveen-ks-2001

Copy link
Copy Markdown

What does this PR do?

_hermes_ink_bundle_stale() in hermes_cli/main.py checks for packages/hermes-ink/dist/ink-bundle.js to decide whether the @hermes/ink bundle needs rebuilding. Since v0.13.0, the build output is dist/entry-exports.js instead. The check now always returns True, which makes _tui_build_needed() always return True, which makes _make_tui_argv() run a full npm run build synchronously every time hermes --tui or the dashboard's embedded chat (hermes dashboard --tui -> /api/pty) starts.

For the standalone CLI the impact is mild, extra npm run build runs, completes quickly because nothing actually changed. For the dashboard's embedded Chat tab the impact is severe: the rebuild runs synchronously inside the asyncio event loop in the /api/pty WebSocket handler for 30–60+ seconds. The dashboard becomes unresponsive, the browser typically times out the WebSocket, and ChatPage.tsx falls through to its [session ended] fallback.

Why this slipped: between v0.12.0 and v0.13.0, ui-tui/packages/hermes-ink/package.json changed its build script from --outfile=dist/ink-bundle.js to --outdir=dist, and index.js was updated to export * from './dist/entry-exports.js'. The staleness check in hermes_cli/main.py was missed.

Fixing the path string is the minimal change; matches what the build now produces and what the wrapper imports.

Related Issue

No existing issue filed, discovered during a downstream version bump (chat works on v2026.4.30, breaks on v2026.5.7 with no other changes).

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/main.py:974 -> update _hermes_ink_bundle_stale() to look for dist/entry-exports.js instead of dist/ink-bundle.js

  • tests/hermes_cli/test_tui_npm_install.py:29 -> update _touch_ink_bundle() test helper to use the same filename so existing tests stay aligned with the actual build output

How to Test

cd ui-tui
npm install
npm run build
ls packages/hermes-ink/dist/    # entry-exports.js exists; ink-bundle.js does NOT
python -c "from hermes_cli.main import _hermes_ink_bundle_stale; from pathlib import Path; print(_hermes_ink_bundle_stale(Path('.')))"
  1. Without this PR: prints True (staleness check misfires).
  2. With this PR: prints False (correct).
  3. End-to-end: run hermes dashboard --tui, open /chat. Without the PR: 30–60s stall before the TUI appears, or [session ended] if the WS times out. With the PR: TUI loads instantly.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(tui): correct ink bundle filename in staleness check)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (2 one-line changes across 2 files)
  • I've run pytest tests/ -q and all tests pass
  • Existing tests in tests/hermes_cli/test_tui_npm_install.py continue to pass; the helper rename keeps them aligned with the new filename. Did not add a new dedicated regression test for the renamed-output case — happy to add one if reviewers prefer.
  • I've tested on my platform: Linux>

Documentation & Housekeeping

  • N/A — no docstring or doc changes required (path string only)
  • N/A — no cli-config.yaml.example keys touched
  • N/A — no architecture or workflow changes
  • Cross-platform impact considered: this is a pathlib.Path filename change with no platform-specific semantics; behaves identically on Linux/macOS/Windows
  • N/A — no tool schemas/descriptions touched

Screenshots / Logs

Reproduction (browser console + dashboard log) on v2026.5.7 before this PR:

  • Browser: WebSocket closed; [session ended] rendered by ChatPage.tsx xterm.js fallback.
  • Server: _make_tui_argv() enters subprocess.run([npm, "run", "build"], ...); the dashboard event loop blocks until npm completes (~30–60s), at which point the WS is already closed by the browser.

After this PR: /chat opens the TUI directly with no rebuild step — same fast-path behavior as v0.12.0 / v2026.4.30.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #20951 — same fix (ink-bundle.js → entry-exports.js in _hermes_ink_bundle_stale()). Multiple duplicates exist: #21630, #22039, #20686.

@teknium1

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: this PR's fix is now obsolete on current main; the stale filename check it patched has been removed/refactored away.

Evidence:

Thanks for the focused bug report and minimal fix — it matched a real issue in the old launcher, but current main has moved past that implementation.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants