Skip to content

fix(tirith): add Windows support for auto-install - #69646

Open
lo-phare wants to merge 1 commit into
NousResearch:mainfrom
lo-phare:fix/tirith-windows-support
Open

fix(tirith): add Windows support for auto-install#69646
lo-phare wants to merge 1 commit into
NousResearch:mainfrom
lo-phare:fix/tirith-windows-support

Conversation

@lo-phare

@lo-phare lo-phare commented Jul 22, 2026

Copy link
Copy Markdown

What

Adds Windows platform detection to tirith_security.py::_detect_target() so the Tirith binary auto-installs on Windows.

Why

Tirith v0.3.3 (June 19, 2026) ships a Windows binary (tirith-x86_64-pc-windows-msvc.zip). The Hermes wrapper still returns None for Windows, silently disabling the security scanner on all Windows installations.

This means Windows users have zero content-level injection protection — commands and file contents are not scanned for threats like homograph URLs, pipe-to-interpreter, or prompt injection patterns.

Why this PR exists (history of previous attempts)

This is the 5th attempt to fix #26044. Previous attempts failed to stick:

PR Author Date What happened
#23050 @fatinghenji May 10 Shows as MERGED, but the merge commit (ae4b09ce) contains unrelated code (plugin API auth tests). The tirith Windows fix is not in the current codebase. Likely reverted during a force-push or larger revert.
#26069 @spranab May 15 Closed as duplicate of #23050
#26132 @ruguoba May 15 Closed as duplicate of #23050
#26068 @LeonSGP43 May 15 Still open — includes test coverage

Meanwhile, #26618 and #26718 (both merged by @teknium1) silence the warning messages on Windows — suppressing the symptom without fixing the underlying installation.

On June 23, a comment on #26044 noted that Tirith v0.3.3 ships a Windows binary — no maintainer response.

Changes

  • _detect_target(): return "x86_64-pc-windows-msvc" when platform.system() == "Windows"
  • New _extract_tirith_binary_zip(): handles .zip archive format (Windows releases ship as .zip, not .tar.gz)
  • _install_tirith(): switches between .zip/.tar.gz and tirith.exe/tirith based on platform
  • _background_install() and ensure_installed(): use platform-aware binary name for PATH and hermes_bin checks

Related

- Add Windows (pc-windows-msvc) to _detect_target()
- Handle .zip archive format for Windows releases (vs .tar.gz on Linux/macOS)
- Use tirith.exe as binary name on Windows
- Update _background_install() and ensure_installed() to find tirith.exe
- Add _extract_tirith_binary_zip() for zip extraction

Tirith v0.3.3 ships a Windows binary. The wrapper still returns None
for Windows, silently disabling the security scanner on all Windows
installations.

Closes NousResearch#26044
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows needs-decision Awaiting maintainer decision before any implementation labels Jul 22, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for carrying the native Windows archive, .exe, and lookup changes forward. Current main still disables this path, so the underlying gap is real.

Problems

  • tools/tirith_security.py:260 only matches platform.system() == "Windows". The #26044 discussion identifies MSYS/MINGW/Cygwin as part of the reported case, so those shells still remain unsupported; the later raw platform.system() == "Windows" checks would also select .tar.gz rather than .zip.
  • The shared arch mapping would produce aarch64-pc-windows-msvc, but the verified Tirith v0.3.3 asset list has only tirith-x86_64-pc-windows-msvc.zip.
  • No tests or documentation are changed. Current tests/tools/test_tirith_security.py:248-256 asserts Windows/AMD64 is unsupported, while website/docs/user-guide/security.md:690 says Windows has no prebuilt binary.

Suggested changes

  • Normalize the Windows platform family and use that result consistently for target, archive, and executable selection; reject Windows ARM64 until an upstream artifact exists.
  • Add Windows/MSYS/ARM64 and real zip-install coverage, then update the security documentation.

Automated hermes-sweeper review.

Comment thread tools/tirith_security.py
plat = "apple-darwin"
elif system in {"Linux", "Android"}:
plat = "unknown-linux-gnu"
elif system == "Windows":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This exact-match branch misses the MSYS/MINGW/Cygwin platform names called out in #26044. Please normalize the whole Windows family and reuse that predicate for the later archive and executable-name decisions; otherwise those environments still take the unsupported path.

Comment thread tools/tirith_security.py
elif system in {"Linux", "Android"}:
plat = "unknown-linux-gnu"
elif system == "Windows":
plat = "pc-windows-msvc"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The existing architecture mapping will turn Windows ARM64 into aarch64-pc-windows-msvc, but the verified v0.3.3 release assets contain only tirith-x86_64-pc-windows-msvc.zip. Return None for Windows ARM64 until upstream ships that artifact.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 area/install-update Installer, updater, packaging, wheels, doctor labels Jul 30, 2026
@lo-phare
lo-phare force-pushed the fix/tirith-windows-support branch from 510aca7 to 4b057eb Compare August 1, 2026 22:16
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Six PRs address or reference the Tirith Windows auto-install failure. #8615, #26069, #26132, and #69646 add native-Windows target/archive/executable handling but miss the reported MSYS-family normalization; #26068 adds that normalization and regression coverage, while merged #23050 has no visible cached diff and serves only as historical context for the earlier base-Windows attempt.

Related pull requests

Duplicates

#8615, #26068, #26069, #26132, and #69646 substantially overlap on Windows target detection, ZIP handling, and tirith.exe; #26069 and #26132 are already closed in favor of #26068, while #23050 is the merged historical predecessor with no visible cached diff.

Suggested consolidation

Keep #26068 open with a salvage path: rebase it onto current main, retain the a91b1c8b3 regular-file protection, add equivalent safe ZIP validation plus real temp-HERMES_HOME extraction tests, and update the Windows documentation before maintainer reconsideration. Close #69646 as duplicate of #26068 after transferring any needed current-main adaptation; this departs from #69646's COMMENTED keep-open review because its visible diff still misses MSYS/MINGW/Cygwin, permits an unsupported Windows ARM64 target, and adds no tests or documentation, whereas the recorded best-fix #26068 covers those core issue requirements.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I26044(["issue #26044 (open)"])
    I26140(["issue #26140 (closed)"])
    subgraph Dup8615 ["PRs duplicating each other"]
        P8615["PR #8615 (closed)"]
        P23050["PR #23050 (merged)"]
        P26068["PR #26068 (open)"]
        P26069["PR #26069 (closed)"]
        P26132["PR #26132 (closed)"]
        P69646["PR #69646 (open)"]
    end
    P69646 -.->|partial| I26044
    P69646 -.->|partial| I26140
    class I26044 open
    class I26140 closed
    class P8615 closed
    class P23050 merged
    class P26068 open
    class P26069 closed
    class P26132 closed
    class P69646 open
    class P26068 best
    class P26068 best
    class P69646 target
    click I26044 "https://github.com/NousResearch/hermes-agent/issues/26044"
    click I26140 "https://github.com/NousResearch/hermes-agent/issues/26140"
    click P8615 "https://github.com/NousResearch/hermes-agent/pull/8615"
    click P23050 "https://github.com/NousResearch/hermes-agent/pull/23050"
    click P26068 "https://github.com/NousResearch/hermes-agent/pull/26068"
    click P26069 "https://github.com/NousResearch/hermes-agent/pull/26069"
    click P26132 "https://github.com/NousResearch/hermes-agent/pull/26132"
    click P69646 "https://github.com/NousResearch/hermes-agent/pull/69646"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 6 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 110 kB of PR diffs, 15 kB of issue/PR text, 7 kB of discussion (12 comments), 16 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/tools Tool registry, model_tools, toolsets needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage 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 sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tirith security scanner fails to install on Windows (MSYS/git-bash) - "unsupported_platform"

4 participants