Skip to content

fix: enable dashboard PTY chat on native Windows - #40022

Closed
Nea74 wants to merge 1 commit into
NousResearch:mainfrom
Nea74:fix/windows-dashboard-conpty
Closed

fix: enable dashboard PTY chat on native Windows#40022
Nea74 wants to merge 1 commit into
NousResearch:mainfrom
Nea74:fix/windows-dashboard-conpty

Conversation

@Nea74

@Nea74 Nea74 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Use the existing pywinpty/ConPTY path on win32 so the dashboard chat tab can spawn and manage terminal sessions without depending on POSIX PTY modules.

Also update the PTY availability test import so it accepts either the Windows or POSIX bridge, depending on platform.

What does this PR do?

Previously, hermes_cli.web_server only attempted to import the POSIX PTY bridge, which depends on POSIX-only modules. On native Windows this caused the dashboard /api/pty path to be unavailable even though Hermes already has pywinpty support.

This PR:

adds a Windows PTY bridge implementation for the dashboard chat path
selects that bridge on win32
preserves the existing POSIX behavior on non-Windows platforms
updates the PTY availability test import so it works on either platform

Related Issue

Related issue: N/A

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

added hermes_cli/win_pty_bridge.py as a pywinpty/ConPTY-backed counterpart to PtyBridge
updated hermes_cli/web_server.py to:
use WinPtyBridge on Windows
keep using the existing POSIX PTY bridge on non-Windows platforms
preserve the existing stub/fallback behavior when the platform-specific import is genuinely unavailable
updated tests/hermes_cli/test_web_server.py so PtyUnavailableError is imported from the active bridge implementation depending on platform

How to Test

On native Windows:

Start Hermes dashboard.
Open the dashboard Chat tab.
Confirm that a terminal-backed chat session can start instead of failing due to PTY unavailability.
Verification used for this PR:

manual smoke test through WinPtyBridge.spawn(['cmd.exe', '/c', 'echo hello-from-conpty-main'])
verified that hermes_cli.web_server resolves PtyBridge to hermes_cli.win_pty_bridge.WinPtyBridge on Windows
ran targeted test:
uv run --with pytest --with pytest-xdist --with pyyaml python -m pytest tests/hermes_cli/test_web_server.py -k 'unavailable_platform_closes_with_message' -o addopts='' -q

Observed result:
1 skipped, 234 deselected

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

grafik

Use the existing pywinpty/ConPTY path on win32 so the dashboard chat tab can spawn and manage terminal sessions without depending on POSIX PTY modules.

Also update the PTY availability test import so it accepts either the Windows or POSIX bridge, depending on platform.

(cherry picked from commit 3eccd018c1678fb9cd71a2c6809f9d9fae95ce89)
@Nea74
Nea74 force-pushed the fix/windows-dashboard-conpty branch from 3eccd01 to c5032b9 Compare June 5, 2026 18:32
@daimon-nous daimon-nous Bot added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 5, 2026
@Nea74

Nea74 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I checked the updated PR branch after rebasing the fix onto current main.
The PR now points to commit c5032b9, but the Checks tab currently shows:
“Workflow runs completed with no jobs”
So this looks like a workflow/policy issue on the PR side rather than a code-level failure in the patch itself. If needed, I’m happy to re-run / adjust anything once the workflows are allowed to execute.

@teknium1

teknium1 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Hi @Nea74 — your design converged on the same shape as #39913 (separate win_pty_bridge.py, same platform-branched import in web_server.py), so I salvaged that one in #42251 to avoid rewriting the same bridge twice. You're credited as co-author on the test commit and added to scripts/release.py AUTHOR_MAP under your andreas@schwarz-ketsch.de email so the release-note generator picks you up. Thanks for the converging implementation — confirms the design wasn't a one-off opinion.

Closing this once #42251 merges.

@alt-glitch alt-glitch added type/feature New feature or request comp/dashboard Web dashboard / control panel UI (dashboard/, landing) platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows and removed type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the converging Windows implementation. This has already landed through the maintained salvage PR, with your contribution credited.

  • Automated hermes-sweeper review verified current main selects WinPtyBridge on Windows in hermes_cli/web_server.py:14179-14193.
  • The pywinpty/ConPTY bridge is present in hermes_cli/win_pty_bridge.py:76-179.
  • Windows-specific import-resolution and bridge behavior coverage is present in tests/hermes_cli/test_web_server_pty_import.py:38-49 and tests/hermes_cli/test_win_pty_bridge.py:83-315.
  • The matching implementation merged as abcf996b1f749a647a1b213653a80d1eee58f6d1 (#42251), whose commit metadata credits Nea74 as co-author.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 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 platform/windows Native Windows-specific behavior or breakage sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants