Skip to content

fix(cli): read .worktreeinclude and .gitignore as UTF-8 in worktree setup - #68798

Closed
solyanviktor-star wants to merge 1 commit into
NousResearch:mainfrom
solyanviktor-star:fix/worktree-include-encoding
Closed

fix(cli): read .worktreeinclude and .gitignore as UTF-8 in worktree setup#68798
solyanviktor-star wants to merge 1 commit into
NousResearch:mainfrom
solyanviktor-star:fix/worktree-include-encoding

Conversation

@solyanviktor-star

Copy link
Copy Markdown
Contributor

Summary

cli._setup_worktree reads .worktreeinclude and .gitignore with the platform locale default encoding. Both are UTF-8 files (git treats path patterns as UTF-8; the same block already appends to .gitignore with encoding="utf-8"), so on Windows this breaks in three ways:

  1. All includes silently lost (worst case). On a cp932/GBK-locale machine, a UTF-8 include list with any non-ASCII byte raises UnicodeDecodeError on include_file.read_text(). The enclosing except Exception logs at DEBUG and swallows it — no entry is copied at all, so the worktree starts without .env/keys and the agent breaks with no visible error.
  2. Non-ASCII entries never copied. On cp1251 the same read produces mojibake paths; those entries fail src.is_file() and are silently skipped (reproduced live on a cp1251 machine).
  3. BOM breaks the first line on every platform. A Notepad-saved include list glues \ufeff to the first entry (never copied), and a BOM'd .gitignore defeats the .worktrees/ membership check, appending a duplicate entry on each worktree launch.

Fix

Read both files with encoding="utf-8-sig", errors="replace", matching the canonical .env readers in hermes_cli/config.py (utf-8-sig specifically because Notepad prepends a BOM) and the UTF-8 append this block already performs.

Same class as the merged/endorsed encoding fixes #60895 / #62123 / #62617, previously unfixed in the worktree path.

Testing

New TestWorktreeIncludeEncoding in tests/cli/test_worktree_security.py exercises the real cli._setup_worktree (same style as the existing security tests):

  • test_bom_in_worktreeinclude_does_not_hide_first_entry — fails without the fix on any platform
  • test_bom_in_gitignore_does_not_duplicate_worktrees_entry — fails without the fix on any platform
  • test_non_ascii_worktreeinclude_entry_copied — reproduces the Windows locale failure

All three fail without the fix on Windows (verified by stashing the diff); full tests/cli/test_worktree* suite otherwise unchanged (6 pre-existing failures in this Windows environment are present on clean main). ruff and scripts/check-windows-footguns.py clean.

Dedup: searched PRs/issues for worktreeinclude/gitignore encoding variants — existing .worktreeinclude PRs cover path traversal only; open Windows worktree PRs (#68252, #68391) touch quarantine/symlink semantics, not these reads.

🤖 Generated with Claude Code

…etup

_setup_worktree read both files with the locale default encoding. On a
cp1251/GBK Windows machine a UTF-8 include list either decodes to
mojibake paths (non-ASCII entries silently not copied) or raises
UnicodeDecodeError, which the enclosing handler logs at DEBUG and
swallows — no include is copied at all, so the worktree starts without
.env/keys and the agent breaks invisibly. A Notepad BOM likewise glues
to the first include entry on every platform, and to the first
.gitignore line, defeating the '.worktrees/' membership check and
appending a duplicate entry on each run.

Read both files with utf-8-sig + errors=replace, matching the canonical
.env readers in hermes_cli/config.py (utf-8-sig because Notepad adds a
BOM) and the UTF-8 append this same block already performs on
.gitignore.

Regression tests exercise the real cli._setup_worktree: the two BOM
tests fail without the fix on any platform, the non-ASCII include test
additionally reproduces the Windows locale failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 21, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #71078 — your commit(s) were cherry-picked onto current main with your authorship preserved in git log (rebase merge). This PR was part of the class-wide close-out of bare read_text/write_text calls: all 139 remaining sites now pass explicit encoding, and a new CI linter rule prevents regressions. Thanks for the contribution!

@teknium1 teknium1 closed this Jul 25, 2026
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 P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage 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