Skip to content

Move glob import to top level in terminal_tool.py - #101

Closed
aydnOktay wants to merge 2 commits into
NousResearch:mainfrom
aydnOktay:refactor/move-glob-import-to-top-fixed
Closed

Move glob import to top level in terminal_tool.py#101
aydnOktay wants to merge 2 commits into
NousResearch:mainfrom
aydnOktay:refactor/move-glob-import-to-top-fixed

Conversation

@aydnOktay

Copy link
Copy Markdown
Contributor

This PR refactors the glob module import in terminal_tool.py by moving it from function scope to module level. The glob import was previously defined inline within three functions (_check_disk_usage_warning, get_active_environments_info, and cleanup_all_environments), which violates Python best practices. By moving the import to the top of the file alongside other standard library imports, we improve code organization, maintainability, and follow PEP 8 guidelines. This change has no functional impact and only improves code structure.

- Add return type hints to all helper functions
- Improve error handling in _check_disk_usage_warning with better logging
- Add exc_info=True to exception logging for better debugging
- Enhance docstrings with Args and Returns sections
- Improve error messages in cleanup functions
- Move glob import from function scope to module level
- Remove redundant inline imports for better code organization
- Follows Python best practices for import statements
@teknium1 teknium1 closed this Feb 27, 2026
Meraniya pushed a commit to Meraniya/hermes-agent that referenced this pull request Aug 6, 2026
…ousResearch#101)

Eight test files under tests/<subdir>/ had:

    sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))

which only ascends one level (to tests/), not to the project root. Since
tests/agent/, tests/hermes_cli/, and tests/cli/ are all real Python
packages (each has __init__.py), inserting tests/ at sys.path[0] causes
absolute imports like `from agent.model_metadata import ...` or `from
hermes_cli.auth import ...` to resolve to the test subpackage itself
instead of the real top-level package -- there's no such module in the
test subpackage, so it raises ModuleNotFoundError.

tests/agent/test_model_metadata_ssl.py and tests/hermes_cli/test_auth_ssl_macos.py
hit this every run (100% reproducible collection failure, not
environment-specific). The other six files (test_model_metadata_local_ctx.py,
test_voice_wrapper.py, test_tool_progress_scrollback.py, test_resume_display.py,
test_cli_user_message_preview.py, test_cli_init.py) have the identical bug but
it stayed dormant -- likely because the correctly-shadowed module was already
cached in sys.modules by the time these files were collected, depending on
collection order.

The line was always redundant: tests/conftest.py:32-34 already inserts the
real PROJECT_ROOT into sys.path unconditionally before any test file loads.
Deleted the line (and the now-unused os/sys imports where nothing else in
the file needed them) rather than fixing the ascent count, since conftest.py
already owns this responsibility.

Also installed psutil, ptyprocess, ruamel.yaml, croniter, and fire in this
environment -- all five are declared dependencies in pyproject.toml
(psutil/fire in the unconditional `dependencies` list, ruamel.yaml/croniter
also core, ptyprocess behind the `pty` extra) but were missing from this
session's Python environment, which was the root cause of ~65 of the 69
test failures seen in a fresh full-suite run on this branch. That's a
container-provisioning gap, not a code issue -- nothing to fix in the repo
for those. Full suite now passes 25370/25371; the one remaining failure
(test_auth_nous_provider.py::test_no_ca_bundle_returns_true) fails only
because this sandbox's outbound-proxy sets REQUESTS_CA_BUNDLE system-wide,
which _resolve_verify() correctly honors per its documented CA-bundle
priority order -- not reproducible in a normal CI/dev environment.


Claude-Session: https://claude.ai/code/session_01PTSqxb1h9MRkvGvVJqSuQh

Co-authored-by: Claude <noreply@anthropic.com>
nicezic pushed a commit to nicezic/hermes-agent that referenced this pull request Aug 26, 2026
…on-events

[CN-fork] P-047 CLI 委派可视化:delegation.cli.* 事件与后台输出流
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants