Skip to content

fix(kanban): catch RuntimeError from expanduser() in workspace root resolver - #44005

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/kanban-expanduser-runtimeerror
Open

fix(kanban): catch RuntimeError from expanduser() in workspace root resolver#44005
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/kanban-expanduser-runtimeerror

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a RuntimeError crash in _is_managed_scratch_path() when the HERMES_KANBAN_WORKSPACES_ROOT environment variable contains a ~user path where user is not a real account (e.g. ~nonexistent_user/workspace).

Path.expanduser() raises RuntimeError("Could not determine home directory.") for the ~user form with a non-existent user — this is distinct from $HOME-based ~ expansion and is not caught by except OSError. The workspace-root resolver now catches RuntimeError alongside OSError so invalid overrides are silently skipped instead of crashing the kanban subsystem.

This is the same bug pattern as #43963 (_add_path_candidate in agent/subdirectory_hints.py), applied to a sibling call site that was not covered by PR #43970.

Related Issue

Same pattern as #43963

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/kanban_db.py: Broadened except OSError to except (OSError, RuntimeError) on the expanduser().resolve() call inside _is_managed_scratch_path()'s HERMES_KANBAN_WORKSPACES_ROOT resolver.
  • tests/hermes_cli/test_kanban_db.py: Added test_is_managed_scratch_path_tilde_user_env_does_not_crash — sets the env var to ~nonexistent_user/workspace and asserts the function returns False instead of raising.

How to Test

  1. Set HERMES_KANBAN_WORKSPACES_ROOT=~nonexistent_user/workspace in your environment
  2. Run any kanban operation that calls _is_managed_scratch_path() — it should not crash
  3. Run the specific test: pytest tests/hermes_cli/test_kanban_db.py::test_is_managed_scratch_path_tilde_user_env_does_not_crash -v

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

…esolver

Path.expanduser() raises RuntimeError (not OSError) when the path contains
a ~user token for a non-existent user account (e.g. "~nonexistent_user/x").
The HERMES_KANBAN_WORKSPACES_ROOT env var resolver only caught OSError,
allowing the RuntimeError to propagate and crash the kanban subsystem.

Same bug pattern as NousResearch#43963 (_add_path_candidate in subdirectory_hints.py).

Fix: broaden except to (OSError, RuntimeError) so invalid workspace-root
overrides are silently skipped instead of crashing.

Fixes NousResearch#43963 (same pattern, different call site)

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Looks Good

  • hermes_cli/kanban_db.py:10 — Clean fix: RuntimeError from Path.expanduser() for non-existent ~user tokens is now caught alongside OSError, preventing crashes when HERMES_KANBAN_WORKSPACES_ROOT points to a non-existent user's home.
  • tests/hermes_cli/test_kanban_db.py:22-31 — Dedicated test case covering the exact scenario. Clear docstring explaining the RuntimeError behavior. Assertion is minimal and correct.

Minor Suggestion

  • The test's monkeypatch.setenv approach is correct, but note that since _is_managed_scratch_path calls kb._is_managed_scratch_path(task_dir) with /tmp, it only validates that the path doesn't crash — not that non-existent user paths in the scratch root are actually skipped. This is fine for the scope of this fix; the assertion correctly reflects the intended behavior.

Reviewed by Hermes Agent

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have labels Jun 11, 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 tracing the Path.expanduser() failure in the cleanup containment guard. The current-main premise is valid at hermes_cli/kanban_db.py:4405.

Problems

  • The same HERMES_KANBAN_WORKSPACES_ROOT value is expanded without a RuntimeError guard in workspaces_root() at hermes_cli/kanban_db.py:554. Standard scratch resolution calls it at hermes_cli/kanban_db.py:5768, so an invalid ~user override can still abort ordinary workspace creation.
  • The proposed test covers only _is_managed_scratch_path; it does not cover that remaining normal workspace-resolution path.

Suggested changes

  • Apply the same invalid-override fallback in workspaces_root() and add a regression test through workspaces_root() or resolve_workspace().

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
@@ -3779,7 +3779,7 @@ def _is_managed_scratch_path(p: Path) -> bool:
if override:

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 protects the cleanup containment resolver, but workspaces_root() still expands the same HERMES_KANBAN_WORKSPACES_ROOT value without handling RuntimeError (current main: hermes_cli/kanban_db.py:554). Normal scratch resolution calls it, so please cover that sibling path and add a regression test through workspace resolution.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants