mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-23 10:42:00 +00:00
fix(kanban): pin worker TERMINAL_CWD to the task workspace (#50348)
_default_spawn launched the worker subprocess with cwd=workspace and set HERMES_KANBAN_WORKSPACE, but never set TERMINAL_CWD — so the worker inherited the dispatching gateway's TERMINAL_CWD. That value takes precedence over the process cwd in two places: - tools/file_tools.py::_resolve_base_dir — a relative write_file path resolved against the gateway user's home instead of the workspace, so artifacts silently landed outside the workspace (#41312). - agent_init's context-file loader — AGENTS.md was discovered relative to the gateway's cwd, so under multi-profile dispatch a worker loaded whichever gateway won the claim race's AGENTS.md, not the task's (#34619). Both are the same root cause. Pinning TERMINAL_CWD to the workspace (where the task's work actually happens) fixes both. Guarded on an existing absolute dir because file_tools rejects relative/sentinel TERMINAL_CWD values — a non-dir workspace leaves the inherited value rather than writing a meaningless one. Closes #34619, closes #41312.
This commit is contained in:
parent
b6d1072408
commit
9630ec6c19
2 changed files with 115 additions and 0 deletions
|
|
@ -7298,6 +7298,20 @@ def _default_spawn(
|
|||
env["HERMES_TENANT"] = task.tenant
|
||||
env["HERMES_KANBAN_TASK"] = task.id
|
||||
env["HERMES_KANBAN_WORKSPACE"] = workspace
|
||||
# Pin TERMINAL_CWD to the task's workspace so the worker's file tools and
|
||||
# context-file loader anchor on the workspace, not whatever cwd the
|
||||
# dispatching gateway happened to export. The worker subprocess is already
|
||||
# launched with cwd=workspace, but TERMINAL_CWD takes precedence over the
|
||||
# process cwd in both file_tools._resolve_base_dir (#41312 — relative
|
||||
# write_file paths were landing in the gateway user's home) and
|
||||
# build_context_files_prompt (#34619 — workers loaded the dispatching
|
||||
# gateway's AGENTS.md instead of the task's). Setting it to the workspace
|
||||
# fixes both: the workspace is where the task's work actually happens.
|
||||
# Only pin a real, absolute directory — file_tools rejects relative /
|
||||
# sentinel TERMINAL_CWD values, so a non-dir workspace must NOT be set
|
||||
# here (leave the inherited value rather than write a meaningless one).
|
||||
if workspace and os.path.isabs(workspace) and os.path.isdir(workspace):
|
||||
env["TERMINAL_CWD"] = workspace
|
||||
if task.branch_name:
|
||||
env["HERMES_KANBAN_BRANCH"] = task.branch_name
|
||||
if task.current_run_id is not None:
|
||||
|
|
|
|||
101
tests/hermes_cli/test_kanban_worker_terminal_cwd.py
Normal file
101
tests/hermes_cli/test_kanban_worker_terminal_cwd.py
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
"""Tests: kanban worker spawn pins TERMINAL_CWD to the task workspace.
|
||||
|
||||
Regression coverage for #34619 and #41312 (same root cause): ``_default_spawn``
|
||||
launched the worker subprocess with ``cwd=workspace`` and set
|
||||
``HERMES_KANBAN_WORKSPACE``, but did NOT set ``TERMINAL_CWD``. Because
|
||||
``TERMINAL_CWD`` takes precedence over the process cwd in both
|
||||
``tools/file_tools.py::_resolve_base_dir`` (relative ``write_file`` paths) and
|
||||
``agent_init``'s context-file loader (``AGENTS.md`` discovery), workers inherited
|
||||
the dispatching gateway's cwd — relative writes landed in the gateway user's
|
||||
home (#41312) and the wrong profile's ``AGENTS.md`` was loaded (#34619).
|
||||
Pinning ``TERMINAL_CWD`` to the workspace fixes both.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
|
||||
|
||||
def _make_task(kb, *, assignee: str = "w"):
|
||||
return kb.Task(
|
||||
id="t_cwd",
|
||||
title="cwd pin",
|
||||
body=None,
|
||||
assignee=assignee,
|
||||
status="running",
|
||||
priority=0,
|
||||
created_by="test",
|
||||
created_at=1,
|
||||
started_at=None,
|
||||
completed_at=None,
|
||||
workspace_kind="dir",
|
||||
workspace_path=None,
|
||||
claim_lock="lock",
|
||||
claim_expires=None,
|
||||
tenant=None,
|
||||
current_run_id=1,
|
||||
)
|
||||
|
||||
|
||||
def _capture_spawn_env(kb, monkeypatch, workspace: str) -> dict:
|
||||
monkeypatch.setattr(kb, "_resolve_hermes_argv", lambda: ["hermes"])
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
class FakeProc:
|
||||
pid = 4242
|
||||
|
||||
def fake_popen(cmd, *args, **kwargs):
|
||||
captured["cmd"] = list(cmd)
|
||||
captured["env"] = dict(kwargs.get("env") or {})
|
||||
captured["cwd"] = kwargs.get("cwd")
|
||||
return FakeProc()
|
||||
|
||||
monkeypatch.setattr(subprocess, "Popen", fake_popen)
|
||||
kb._default_spawn(_make_task(kb), workspace)
|
||||
return captured
|
||||
|
||||
|
||||
def test_terminal_cwd_pinned_to_workspace(monkeypatch, tmp_path):
|
||||
"""A real, absolute workspace dir is pinned as TERMINAL_CWD."""
|
||||
root = tmp_path / ".hermes"
|
||||
(root / "profiles" / "w").mkdir(parents=True)
|
||||
(root / "profiles" / "w" / "config.yaml").write_text("toolsets:\n - kanban\n", encoding="utf-8")
|
||||
root.joinpath("config.yaml").write_text("toolsets:\n - kanban\n", encoding="utf-8")
|
||||
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||
|
||||
from hermes_cli import kanban_db as kb
|
||||
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
|
||||
captured = _capture_spawn_env(kb, monkeypatch, str(workspace))
|
||||
|
||||
assert captured["env"]["TERMINAL_CWD"] == str(workspace)
|
||||
# The subprocess cwd and TERMINAL_CWD must agree — both anchor the workspace.
|
||||
assert captured["cwd"] == str(workspace)
|
||||
assert captured["env"]["HERMES_KANBAN_WORKSPACE"] == str(workspace)
|
||||
|
||||
|
||||
def test_terminal_cwd_not_pinned_for_nonexistent_workspace(monkeypatch, tmp_path):
|
||||
"""A non-directory workspace must NOT clobber the inherited TERMINAL_CWD.
|
||||
|
||||
file_tools rejects relative / sentinel TERMINAL_CWD values, so writing a
|
||||
meaningless (nonexistent) path would be worse than leaving the inherited
|
||||
one. The guard requires an existing absolute dir.
|
||||
"""
|
||||
root = tmp_path / ".hermes"
|
||||
(root / "profiles" / "w").mkdir(parents=True)
|
||||
(root / "profiles" / "w" / "config.yaml").write_text("toolsets:\n - kanban\n", encoding="utf-8")
|
||||
root.joinpath("config.yaml").write_text("toolsets:\n - kanban\n", encoding="utf-8")
|
||||
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||
monkeypatch.setenv("TERMINAL_CWD", "/pre/existing/anchor")
|
||||
|
||||
from hermes_cli import kanban_db as kb
|
||||
|
||||
missing = tmp_path / "does-not-exist"
|
||||
|
||||
captured = _capture_spawn_env(kb, monkeypatch, str(missing))
|
||||
|
||||
# Inherited value is preserved (not overwritten with a bogus path).
|
||||
assert captured["env"]["TERMINAL_CWD"] == "/pre/existing/anchor"
|
||||
Loading…
Add table
Add a link
Reference in a new issue