fix(logging): alias RotatingFileHandler to concurrent-log-handler (salvage #44921) - #46794
Merged
Conversation
On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com>
Contributor
🔎 Lint report:
|
The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used.
austinpickett
enabled auto-merge (squash)
June 15, 2026 20:01
OutThisLife
approved these changes
Jun 17, 2026
OutThisLife
left a comment
Collaborator
There was a problem hiding this comment.
Approving. Clean salvage, and a meaningful improvement over the original #44921: scoping the swap to Windows only (sys.platform == 'win32' import + sys_platform == 'win32' dependency marker) is the right call —
- The bug (
os.renameWinError 32 on a file another process holds open) is Windows-specific; POSIX renames an open file fine, so stdlib already works on Linux/macOS. - The POSIX managed-mode (NixOS) path depends on stdlib
RotatingFileHandler's exact_open()/doRollover()lifecycle for the 0o660 chmod; keeping stdlib there preserves that guarantee, which CLH's lazy-open/different-rollover would break. _ManagedRotatingFileHandlerstays constructor-compatible withConcurrentRotatingFileHandler(onlymaxBytes/backupCount/encodingare passed, all supported), and on Windows_chmod_if_managedis a no-op sinceis_managed()is False there — so thesuper()delegation in_open/doRolloveris clean.==0.9.29matches this repo's exact-pin convention for core deps;uv.lockis regenerated with theportalocker/pywin32tree correctly gated onsys_platform == 'win32'.- The test imports
RotatingFileHandlerfrom the module under test so the autouse strip fixture matches whichever class resolved, on either platform.
The Windows CLH path can't execute on the Linux CI runners, but the delegation is sound and the "Windows footguns" check passes. LGTM.
xyshanren
pushed a commit
to xyshanren/hermes-agent-cn
that referenced
this pull request
Jun 25, 2026
…lvage NousResearch#44921) (NousResearch#46794) * fix(logging): alias RotatingFileHandler to concurrent-log-handler On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (NousResearch#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> * fix(logging): gate concurrent-log-handler swap to Windows only The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (NousResearch#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used. --------- Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
waefrebeorn
pushed a commit
to waefrebeorn/slermes
that referenced
this pull request
Jul 2, 2026
…lvage NousResearch#44921) (NousResearch#46794) * fix(logging): alias RotatingFileHandler to concurrent-log-handler On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (NousResearch#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> * fix(logging): gate concurrent-log-handler swap to Windows only The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (NousResearch#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used. --------- Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
habarmc1223-sudo
pushed a commit
to habarmc1223-sudo/hermes-agent-fluxmem
that referenced
this pull request
Jul 8, 2026
…lvage NousResearch#44921) (NousResearch#46794) * fix(logging): alias RotatingFileHandler to concurrent-log-handler On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (NousResearch#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> * fix(logging): gate concurrent-log-handler swap to Windows only The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (NousResearch#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used. --------- Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
santhreal
pushed a commit
to santhreal/hermes-agent
that referenced
this pull request
Jul 13, 2026
…lvage NousResearch#44921) (NousResearch#46794) * fix(logging): alias RotatingFileHandler to concurrent-log-handler On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (NousResearch#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> * fix(logging): gate concurrent-log-handler swap to Windows only The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (NousResearch#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used. --------- Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…lvage NousResearch#44921) (NousResearch#46794) * fix(logging): alias RotatingFileHandler to concurrent-log-handler On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (NousResearch#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> * fix(logging): gate concurrent-log-handler swap to Windows only The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (NousResearch#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used. --------- Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
leewenjie
pushed a commit
to leewenjie/hermes-agent
that referenced
this pull request
Aug 7, 2026
…lvage NousResearch#44921) (NousResearch#46794) * fix(logging): alias RotatingFileHandler to concurrent-log-handler On Windows, stdlib RotatingFileHandler.doRollover() uses os.rename(), which fails with PermissionError [WinError 32] whenever another process holds an append-mode handle on agent.log — essentially always in Hermes (TUI, gateway, hy_memory server, MCP servers, and on-demand CLI commands all log from separate processes). This pinned agent.log at the 5 MiB threshold and spammed stderr with a traceback on every emit (NousResearch#44873). Add concurrent-log-handler==0.9.29 as a core dep and alias its ConcurrentRotatingFileHandler as RotatingFileHandler in hermes_logging.py. It wraps the rename in a cross-process file lock (via portalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time. Aliasing keeps every existing isinstance/class-declaration reference working unchanged. Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> * fix(logging): gate concurrent-log-handler swap to Windows only The initial salvage aliased RotatingFileHandler -> ConcurrentRotatingFileHandler unconditionally, which regressed POSIX: CLH opens lazily and rotates via its own lock path, breaking managed-mode (NixOS) group-writable perms and eager file creation that _ManagedRotatingFileHandler depends on. CI caught it as 2 failures in test_managed_mode_*_group_writable on Linux. The WinError 32 bug (NousResearch#44873) is Windows-specific — POSIX renames an open file fine, so stdlib already works on Linux/macOS. Gate the swap behind sys.platform == 'win32': Windows uses CLH, POSIX keeps stdlib RotatingFileHandler. - hermes_logging.py: platform-conditional import. - tests/test_hermes_logging.py: import RotatingFileHandler from hermes_logging (single source of truth) so the autouse fixture's isinstance checks match the real handler class on both platforms. - pyproject.toml/uv.lock: mark the dep 'sys_platform == "win32"' so portalocker /pywin32 only ship where used. --------- Co-authored-by: tuancookiez-hub <tuancookiez@gmail.com> Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Salvage of #44921 (@tuancookiez-hub) onto current
main. Fixes #44873.On Windows, stdlib
RotatingFileHandler.doRollover()usesos.rename(), which fails withPermissionError [WinError 32]whenever another process holds an append-mode handle onagent.log— essentially always in Hermes, where the TUI, gateway,hy_memoryserver, MCP servers, and on-demand CLI commands all log from separate processes. The result:agent.loggets pinned at the 5 MiB rotation threshold and every emit spams stderr with a traceback.Confirmed still present on
main:hermes_logging.py:35still doesfrom logging.handlers import RotatingFileHandler, and there's noconcurrent-log-handleranywhere inpyproject.toml.What changed
concurrent-log-handler==0.9.29as a core dependencyConcurrentRotatingFileHandlerasRotatingFileHandlerinhermes_logging.py— it wraps the rename in a cross-process file lock (viaportalocker: pywin32 on Windows, fcntl on POSIX) so only one process rotates at a time and the others wait their turn_ManagedRotatingFileHandler(RotatingFileHandler)still subclasses correctly, and allisinstance(..., RotatingFileHandler)checks still matchDeviation from the original PR
Tidied a minor inaccuracy in the explanatory comments: the original said the lock uses "msvcrt on Windows, fcntl on POSIX", but CLH 0.9.29 actually locks via
portalocker(which uses pywin32 on Windows). Updated both the module and pyproject comments. No functional change.Test plan
uv lock --check→ consistent (220 packages);portalockerpulled transitively (pywin32already a core dep)hermes_logging.RotatingFileHandler is ConcurrentRotatingFileHandler→ True;_ManagedRotatingFileHandlersubclasses it → Truepytest tests/test_hermes_logging.py→ 60 passed. The 2 failures observed locally (*_group_writable) are pre-existing Windows-only failures — they assert0o660Unix perms with noskipifguard and fail onmaintoo; CI runs on Linux where they pass.Credit