Skip to content

fix(cli): load Hermes dotenv in mini_swe and sample_and_compress - #8415

Open
Tianworld wants to merge 1 commit into
NousResearch:mainfrom
Tianworld:fix/dotenv-encoding-fallback-entrypoints
Open

fix(cli): load Hermes dotenv in mini_swe and sample_and_compress#8415
Tianworld wants to merge 1 commit into
NousResearch:mainfrom
Tianworld:fix/dotenv-encoding-fallback-entrypoints

Conversation

@Tianworld

@Tianworld Tianworld commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

What

Selective salvage of remaining bare load_dotenv() entrypoints on current main:

  • mini_swe_runner.py — use load_hermes_dotenv(project_env=...) and add missing from pathlib import Path
  • scripts/sample_and_compress.py — same shared loader (UTF-8 then latin-1)
  • tests/test_mini_swe_runner_dotenv.py — regression: latin-1 ~/.hermes/.env loads successfully when importing mini_swe_runner

Obsolete/redundant hunks already fixed on main were dropped (trajectory_compressor, discord-voice-doctor, dump, hermes_base_env, load_dotenv_path).

Why

CONTRIBUTING documents latin-1 fallback for .env on Windows; these two entrypoints still used plain load_dotenv(), which could raise or mis-read non-UTF8 files.

How to test

pytest tests/test_mini_swe_runner_dotenv.py -v

Platforms

Logic-only; behavior aligned with existing load_hermes_dotenv / gateway/cron patterns.

Made with Cursor

@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 consolidating the remaining dotenv entrypoints. The underlying issue remains on current main in mini_swe_runner.py:36-40 and scripts/sample_and_compress.py:25-26, both of which still call bare load_dotenv().

Problems

  • The added Path(__file__) expression in mini_swe_runner.py has no corresponding from pathlib import Path import in PR commit 2a9c13e; importing the runner would raise NameError.
  • This branch needs selective salvage: GitHub reports it conflicting, environments/hermes_base_env.py is absent from current main, and current main already uses load_hermes_dotenv() in trajectory_compressor.py:51-56 and scripts/discord-voice-doctor.py:177-184.

Suggested changes

  • Retain the two still-needed entrypoint conversions, add the missing Path import, and drop the obsolete/redundant hunks.
  • Cover a latin-1 .env through an actual affected entrypoint path.

Automated hermes-sweeper review.

Comment thread mini_swe_runner.py
@@ -37,10 +37,12 @@
from typing import List, Dict, Any, Optional, Literal

import fire
from dotenv import load_dotenv

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 added Path(...) expression needs from pathlib import Path. The module's imports do not currently define Path, so importing mini_swe_runner.py would fail with NameError before dotenv loading runs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in efd676f: added from pathlib import Path before the Path(__file__) usage.

@teknium1 teknium1 added 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:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 12, 2026
Replace bare load_dotenv() with load_hermes_dotenv so UTF-8 and latin-1
.env files work on Windows. Add missing Path import for mini_swe_runner.
Drop obsolete hunks already fixed on main. Add import-path regression test.
@Tianworld
Tianworld force-pushed the fix/dotenv-encoding-fallback-entrypoints branch from 2a9c13e to efd676f Compare July 19, 2026 11:50
@Tianworld

Copy link
Copy Markdown
Contributor Author

Addressed review feedback:

  1. Selective salvage on current main — only mini_swe_runner.py and scripts/sample_and_compress.py (still used bare load_dotenv()). Dropped obsolete/redundant hunks (trajectory_compressor, discord-voice-doctor, dump, hermes_base_env, load_dotenv_path).
  2. Added from pathlib import Path in mini_swe_runner.py so Path(__file__) does not raise NameError.
  3. Regression test tests/test_mini_swe_runner_dotenv.py — latin-1 ~/.hermes/.env is loaded successfully when importing the mini_swe_runner entrypoint.

@Tianworld Tianworld changed the title fix(cli): unify .env loading with UTF-8 and latin-1 fallback fix(cli): load Hermes dotenv in mini_swe and sample_and_compress Jul 26, 2026
@Tianworld

Copy link
Copy Markdown
Contributor Author

Ready for re-review.

Addressed the sweeper feedback on current main:

  1. Selective salvage — only mini_swe_runner.py and scripts/sample_and_compress.py (still bare load_dotenv()); dropped obsolete/redundant hunks.
  2. Added from pathlib import Path so Path(__file__) does not raise NameError.
  3. Regression test tests/test_mini_swe_runner_dotenv.py covers latin-1 Hermes .env via the mini_swe import path.

CI green (including Python tests/lints); no conflicts. PR title/description updated to match efd676f.

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

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants