Fix ctl Python wrapper ownership
This commit is contained in:
1 parent
9268f411d8
commit
d5dda03ec2
3 files changed
+20
-6
No files matched your search
@@ -4,6 +4,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- `ctl.sh status` and `ctl.sh stop` now recognize daemons launched through a custom `HERMES_WEBUI_PYTHON` wrapper, keeping the Python 3.13 process-lifecycle regression tests stable without weakening stale-PID protection.
|
||||
- **PR #2136** by @LumenYoung — Stale stream writebacks no longer poison the active session transcript. `cancel_stream()` intentionally clears `active_stream_id` early so the UI can accept a follow-up turn while an old worker is unwinding — but the old worker could still return later from `run_conversation()` and persist its stale result over the newer transcript, causing visible transcript / turn journal / `state.db` to disagree (especially around cancel+retry on compressed continuations). Adds a single-line ownership check `_stream_writeback_is_current(session, stream_id)` (token equality against `session.active_stream_id`) and short-circuits both finalize paths: the success path in `_run_agent_streaming` and the cancel-handler path in `cancel_stream()`. When the stream no longer owns the writeback, both paths log `Skipping stale stream/cancel writeback` and return cleanly without persisting. 89-line regression suite in `tests/test_stale_stream_writeback.py`; companion updates to `tests/test_issue1361_cancel_data_loss.py` and `tests/test_sprint42.py` for the new return-without-persist behavior.
|
||||
|
||||
### Added
|
||||
|
||||
@@ -129,11 +129,12 @@ _build_bootstrap_args() {
|
||||
}
|
||||
|
||||
_write_state() {
|
||||
local pid="$1" host="$2" port="$3"
|
||||
local pid="$1" host="$2" port="$3" python_exe="${4:-}"
|
||||
local state_dir="${HERMES_WEBUI_STATE_DIR:-${DEFAULT_STATE_DIR}}"
|
||||
{
|
||||
printf 'PID=%q\n' "${pid}"
|
||||
printf 'REPO_ROOT=%q\n' "${REPO_ROOT}"
|
||||
printf 'PYTHON_EXE=%q\n' "${python_exe}"
|
||||
printf 'HOST=%q\n' "${host}"
|
||||
printf 'PORT=%q\n' "${port}"
|
||||
printf 'LOG_FILE=%q\n' "${LOG_FILE}"
|
||||
@@ -168,14 +169,15 @@ _proc_args() {
|
||||
}
|
||||
|
||||
_is_owned_webui_pid() {
|
||||
local pid="$1" args state_repo=""
|
||||
local pid="$1" args state_repo="" state_python=""
|
||||
[[ -f "${STATE_FILE}" ]] || return 1
|
||||
_load_state_if_present
|
||||
state_repo="${REPO_ROOT:-}"
|
||||
state_python="${PYTHON_EXE:-}"
|
||||
[[ "${state_repo}" == "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ]] || return 1
|
||||
args="$(_proc_args "${pid}")"
|
||||
[[ -n "${args}" ]] || return 1
|
||||
[[ "${args}" == *"${state_repo}/bootstrap.py"* || "${args}" == *"${state_repo}/server.py"* || "${args}" == *"${state_repo}/start.sh"* ]]
|
||||
[[ "${args}" == *"${state_repo}/bootstrap.py"* || "${args}" == *"${state_repo}/server.py"* || "${args}" == *"${state_repo}/start.sh"* || ( -n "${state_python}" && "${args}" == *"${state_python}"* ) ]]
|
||||
}
|
||||
|
||||
_current_pid() {
|
||||
@@ -222,7 +224,7 @@ start_cmd() {
|
||||
pid=$!
|
||||
|
||||
printf '%s\n' "${pid}" > "${PID_FILE}"
|
||||
_write_state "${pid}" "${CTL_HOST}" "${CTL_PORT}"
|
||||
_write_state "${pid}" "${CTL_HOST}" "${CTL_PORT}" "${python_exe}"
|
||||
sleep 0.15
|
||||
if ! _is_alive "${pid}"; then
|
||||
echo "[ctl] Hermes WebUI failed to stay running. Log: ${LOG_FILE}" >&2
|
||||
|
||||
@@ -75,6 +75,17 @@ def wait_for_pid_file(pid_file: Path, timeout: float = 3.0) -> int:
|
||||
raise AssertionError(f"PID file was not written: {pid_file}")
|
||||
|
||||
|
||||
def wait_for_file_text(path: Path, timeout: float = 3.0) -> str:
|
||||
deadline = time.time() + timeout
|
||||
while time.time() < deadline:
|
||||
if path.exists():
|
||||
text = path.read_text(encoding="utf-8")
|
||||
if text:
|
||||
return text
|
||||
time.sleep(0.05)
|
||||
raise AssertionError(f"File was not written: {path}")
|
||||
|
||||
|
||||
def assert_process_exits(pid: int, timeout: float = 3.0) -> None:
|
||||
deadline = time.time() + timeout
|
||||
while time.time() < deadline:
|
||||
@@ -110,7 +121,7 @@ def test_start_writes_pid_under_hermes_home_runs_foreground_no_browser_and_logs(
|
||||
try:
|
||||
assert pid > 1
|
||||
assert log_file.exists()
|
||||
fake_output = fake_log.read_text(encoding="utf-8")
|
||||
fake_output = wait_for_file_text(fake_log)
|
||||
assert "bootstrap.py --no-browser --foreground" in fake_output
|
||||
assert "host=0.0.0.0 port=18991" in fake_output
|
||||
assert str(hermes_home / "webui") in fake_output
|
||||
@@ -154,7 +165,7 @@ def test_start_loads_dotenv_but_inline_overrides_win(tmp_path):
|
||||
assert result.returncode == 0, result.stderr + result.stdout
|
||||
pid = wait_for_pid_file(tmp_path / ".hermes" / "webui.pid")
|
||||
try:
|
||||
fake_output = fake_log.read_text(encoding="utf-8")
|
||||
fake_output = wait_for_file_text(fake_log)
|
||||
assert "fake-python args:" in fake_output
|
||||
assert "host=0.0.0.0 port=18888" in fake_output
|
||||
finally:
|
||||
|
||||
Reference in new issue
Block a user