fix(tui): session-bound config.set persists into the session profile, not the launch profile
Desktop app-global remote mode and the Ink TUI send `config.set` with `session_id` and no `profile` param. `@_profile_scoped` only bound `params["profile"]`, so a write from a focused worker session round-tripped `_load_cfg_raw()`/`_save_cfg()` against the LAUNCH profile's config.yaml while the worker profile kept its old value (and reverted on restart). The decorator now falls back to the named session's `profile_home` when `profile` is absent, binding the full runtime scope (home + secrets + terminal) the same way an explicit profile does. `config.set cwd` from a profile-bound session no longer publishes `TERMINAL_CWD` into the shared process env (that variable belongs to the launch profile). Closes #85669. Supersedes #85676 (its diff predates the methods_* split). Co-authored-by: worlldz <cryptoworlldz@gmail.com>
This commit is contained in:
64
tests/tui_gateway/test_config_set_session_profile_scope.py
Normal file
64
tests/tui_gateway/test_config_set_session_profile_scope.py
Normal file
@@ -0,0 +1,64 @@
|
||||
"""``config.set`` bound by ``session_id`` alone persists into the session's profile (issue #85669).
|
||||
|
||||
Desktop app-global remote mode and the Ink TUI send ``config.set`` with ``session_id`` and no
|
||||
``profile`` param. The session record carries ``profile_home``; a write that ignores it lands in
|
||||
the launch profile's config.yaml while the focused profile silently keeps its old value.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
import tui_gateway.server as server
|
||||
|
||||
|
||||
def _write_cfg(home: Path, busy: str, approvals: str) -> None:
|
||||
home.mkdir(parents=True, exist_ok=True)
|
||||
(home / "config.yaml").write_text(
|
||||
yaml.safe_dump({"display": {"busy_input_mode": busy}, "approvals": {"mode": approvals}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _read(home: Path) -> dict:
|
||||
return yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8")) or {}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def homes(tmp_path, monkeypatch):
|
||||
launch, worker = tmp_path / "launch", tmp_path / "profiles" / "worker"
|
||||
_write_cfg(launch, "queue", "manual")
|
||||
_write_cfg(worker, "queue", "manual")
|
||||
monkeypatch.setenv("HERMES_HOME", str(launch))
|
||||
monkeypatch.setattr(server, "_hermes_home", launch)
|
||||
monkeypatch.setattr(server, "_cfg_cache", None)
|
||||
monkeypatch.setattr(server, "_cfg_sig", None)
|
||||
monkeypatch.setattr(server, "_cfg_path", None)
|
||||
monkeypatch.setattr(server, "_emit", lambda *a, **k: None)
|
||||
return launch, worker
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("key", "value", "section", "field"),
|
||||
[("busy", "steer", "display", "busy_input_mode"), ("approval_mode", "off", "approvals", "mode")],
|
||||
)
|
||||
def test_session_bound_config_set_writes_the_session_profile(homes, key, value, section, field):
|
||||
launch, worker = homes
|
||||
session = {"agent": None, "profile_home": str(worker), "session_key": "worker-session"}
|
||||
with patch.dict(server._sessions, {"s-worker": session}, clear=False):
|
||||
resp = server._methods["config.set"]("rid", {"session_id": "s-worker", "key": key, "value": value})
|
||||
assert resp["result"]["value"] == value
|
||||
assert _read(worker)[section][field] == value
|
||||
assert _read(launch)[section][field] != value
|
||||
|
||||
|
||||
def test_unbound_config_set_still_writes_the_launch_profile(homes):
|
||||
launch, worker = homes
|
||||
resp = server._methods["config.set"]("rid", {"key": "busy", "value": "steer"})
|
||||
assert resp["result"]["value"] == "steer"
|
||||
assert _read(launch)["display"]["busy_input_mode"] == "steer"
|
||||
assert _read(worker)["display"]["busy_input_mode"] == "queue"
|
||||
@@ -414,7 +414,10 @@ def _set_cwd(rid, params, key, value, session):
|
||||
if not os.path.isdir(cwd):
|
||||
return _err(rid, 4002, f"working directory does not exist: {raw}")
|
||||
_write_config_key("terminal.cwd", cwd)
|
||||
os.environ["TERMINAL_CWD"] = cwd
|
||||
# ``TERMINAL_CWD`` is the LAUNCH process's; a profile-bound session persists its cwd through its
|
||||
# own config.yaml (written above under the session scope) and must not leak it into other profiles.
|
||||
if not (isinstance(session, dict) and session.get("profile_home")):
|
||||
os.environ["TERMINAL_CWD"] = cwd
|
||||
return _kv(rid, "terminal.cwd", cwd, cwd=cwd, branch=git_probe.branch(cwd))
|
||||
|
||||
|
||||
|
||||
@@ -553,10 +553,21 @@ def _profile_scoped(handler):
|
||||
systemd / ``op run`` injection); once multiplexing is active it binds its own scope from the env
|
||||
frozen at activation (``_session_profile_runtime_scope``), never ambient state a secondary context
|
||||
might have poisoned (#107422).
|
||||
|
||||
No ``profile`` param but a ``session_id`` naming a live session: that session's ``profile_home``
|
||||
is the scope. The TUI and the Desktop's ambient dispatcher send session-bound RPCs with the
|
||||
session id alone, so a ``config.set`` from a focused worker session otherwise persisted into the
|
||||
LAUNCH profile's config.yaml while the worker's stayed unchanged (#85669).
|
||||
"""
|
||||
def wrapper(rid, params):
|
||||
home = _profile_home(params.get("profile") if isinstance(params, dict) else None)
|
||||
with _session_profile_runtime_scope({"profile_home": str(home) if home else None}):
|
||||
p = params if isinstance(params, dict) else {}
|
||||
if str(p.get("profile") or "").strip():
|
||||
home = _profile_home(p.get("profile"))
|
||||
profile_home = str(home) if home else None
|
||||
else:
|
||||
session = _sessions.get(str(p.get("session_id") or ""))
|
||||
profile_home = session.get("profile_home") if isinstance(session, dict) else None
|
||||
with _session_profile_runtime_scope({"profile_home": profile_home or None}):
|
||||
return handler(rid, params)
|
||||
return wrapper
|
||||
|
||||
|
||||
Reference in New Issue
Block a user