fix: reject unavailable desktop profile and session targets
This commit is contained in:
@@ -18,6 +18,7 @@ def main():
|
||||
parser.add_argument('--repo', type=Path, required=True)
|
||||
parser.add_argument('--out', type=Path, required=True)
|
||||
parser.add_argument('--port', type=int, default=18020)
|
||||
parser.add_argument('--invalid-targets', action='store_true')
|
||||
parser.add_argument('--compress', action='store_true')
|
||||
parser.add_argument('--reset', action='store_true')
|
||||
parser.add_argument('--failure', action='store_true', help='inject one model-config preparation failure')
|
||||
@@ -94,7 +95,7 @@ def main():
|
||||
receipt.write(json.dumps(row) + '\n')
|
||||
print(str(row)[:250], flush=True)
|
||||
return row
|
||||
def rpc(method, params):
|
||||
def rpc(method, params, allow_error=False):
|
||||
nonlocal counter
|
||||
counter += 1
|
||||
ws.send(json.dumps({'jsonrpc': '2.0', 'id': counter, 'method': method, 'params': params}))
|
||||
@@ -102,6 +103,8 @@ def main():
|
||||
row = receive()
|
||||
if row.get('id') == counter:
|
||||
if 'error' in row:
|
||||
if allow_error:
|
||||
return row
|
||||
raise RuntimeError(row)
|
||||
return row['result']
|
||||
def turn(sid, text):
|
||||
@@ -137,7 +140,9 @@ def main():
|
||||
raise
|
||||
result['expected_failure'] = str(exc)
|
||||
result['launch_config_unchanged'] = (home / 'config.yaml').read_bytes() == launch_before
|
||||
result['worker_tools'] = yaml.safe_load((profile / 'config.yaml').read_text())['platform_toolsets']['cli']
|
||||
sys.path.insert(0, str(args.repo))
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
result['worker_tools'] = read_user_config_raw(profile / 'config.yaml')['platform_toolsets']['cli']
|
||||
else:
|
||||
(profile / 'SOUL.md').write_text('Capabilities changed for the worker.\n')
|
||||
if observe and args.reset:
|
||||
@@ -150,6 +155,16 @@ def main():
|
||||
with sqlite3.connect(p / 'state.db') as db:
|
||||
result['stores'][name] = db.execute('SELECT role, content, active FROM messages WHERE session_id=? ORDER BY id', (key,)).fetchall()
|
||||
result['wrong_profile_writes'] = any('PERSISTENCE_AFTER_REBUILD' in str(row) for row in result['stores']['launch'])
|
||||
if args.invalid_targets:
|
||||
result['target_controls'] = {str(p): rpc('session.list', {'profile': p}) for p in (None, 'default', 'worker')}
|
||||
rpc('session.close', {'session_id': sid})
|
||||
before = (home / 'config.yaml').read_bytes()
|
||||
result['stale_session'] = rpc('tools.configure', {'session_id': sid, 'action': 'enable', 'names': ['terminal']}, allow_error=True)
|
||||
profile.rename(home / 'removed-worker')
|
||||
result['invalid_profiles'] = {name: {method: rpc(method, dict(params, profile=name), allow_error=True) for method, params in [('session.list', {}), ('config.get', {'key': 'full'}), ('config.set', {'key': 'busy', 'value': 'steer'})]} for name in ('worker', 'unknown')}
|
||||
result['invalid_config_unchanged'] = before == (home / 'config.yaml').read_bytes()
|
||||
result['global_tools'] = rpc('tools.configure', {'action': 'enable', 'names': ['terminal']})
|
||||
result['global_config_changed'] = before != (home / 'config.yaml').read_bytes()
|
||||
if observe:
|
||||
rpc('session.close', {'session_id': sid})
|
||||
result['ownership_after_close'] = rpc('probe.ownership', {'session_id': sid})
|
||||
@@ -169,6 +184,11 @@ def main():
|
||||
print(json.dumps({k: result[k] for k in ('sha', 'home', 'wrong_profile_writes', 'duplicate_active_groups') if k in result}, indent=2))
|
||||
assert not result['wrong_profile_writes'], 'rebuild wrote the next turn to launch state.db'
|
||||
assert any('PERSISTENCE_AFTER_REBUILD' in str(row) for row in result['stores']['worker'])
|
||||
if args.invalid_targets:
|
||||
assert 'error' in result['stale_session'], 'stale session changed launch config'
|
||||
assert all('error' in reply for replies in result['invalid_profiles'].values() for reply in replies.values())
|
||||
assert result['invalid_config_unchanged']
|
||||
assert result['global_config_changed'] and not result['global_tools']['reset']
|
||||
if args.reset:
|
||||
assert result['launch_config_unchanged'], 'tools.configure changed launch config'
|
||||
assert 'web' in result['worker_tools'], 'tools.configure did not change worker config'
|
||||
|
||||
@@ -15451,6 +15451,7 @@ def test_session_branch_writes_to_parent_profile_db(monkeypatch, tmp_path):
|
||||
"""session.branch must copy history into the parent's profile state.db."""
|
||||
profile_home = tmp_path / "profiles" / "mlperf"
|
||||
profile_home.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
seen: dict = {"msgs": []}
|
||||
|
||||
class LaunchDB:
|
||||
@@ -15889,6 +15890,7 @@ def test_session_branch_installs_parent_profile_secret_scope(monkeypatch, tmp_pa
|
||||
|
||||
profile_home = tmp_path / "profiles" / "mlperf"
|
||||
profile_home.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
(profile_home / ".env").write_text(
|
||||
"PROXMOX_TOKEN=mlperf-secret\n", encoding="utf-8"
|
||||
)
|
||||
@@ -15981,6 +15983,7 @@ def test_session_branch_uses_persisted_display_history_after_compaction(monkeypa
|
||||
"""A live branch must copy the complete visible transcript, not the compacted model tail."""
|
||||
profile_home = tmp_path / "profiles" / "mlperf"
|
||||
profile_home.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
seen: dict = {"msgs": []}
|
||||
|
||||
display_history = [
|
||||
@@ -20663,7 +20666,7 @@ def test_prompt_submit_passes_persist_user_message_to_agent(monkeypatch):
|
||||
server._sessions.pop("sid", None)
|
||||
|
||||
|
||||
def test_prompt_submit_releases_old_history_before_heap_trim(monkeypatch):
|
||||
def test_prompt_submit_releases_old_history_before_heap_trim(monkeypatch, tmp_path):
|
||||
"""The trim boundary must not retain the just-pruned history snapshots."""
|
||||
observed = {}
|
||||
cleanup_order = []
|
||||
@@ -20702,7 +20705,10 @@ def test_prompt_submit_releases_old_history_before_heap_trim(monkeypatch):
|
||||
observed["run_kwargs"] = caller_locals.get("run_kwargs")
|
||||
|
||||
session = _session(agent=_Agent())
|
||||
session["profile_home"] = "/tmp/test-profile"
|
||||
profile_home = tmp_path / "profiles" / "worker"
|
||||
profile_home.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
session["profile_home"] = str(profile_home)
|
||||
session["history"] = [
|
||||
{"role": "tool", "tool_call_id": "old", "content": "x" * 20_000}
|
||||
]
|
||||
|
||||
@@ -32,6 +32,16 @@ def test_tools_configure_uses_live_session_profile(tmp_path, monkeypatch, explic
|
||||
assert "terminal" not in yaml.safe_load((profile / "config.yaml").read_text())["platform_toolsets"]["cli"]
|
||||
assert seen == [profile]
|
||||
assert get_hermes_home() == home
|
||||
worker_before = (profile / "config.yaml").read_bytes()
|
||||
monkeypatch.delitem(server._sessions, "profile-tools")
|
||||
response = server._methods["tools.configure"](2, params)
|
||||
assert response["error"]["code"] == 4001
|
||||
assert (home / "config.yaml").read_bytes() == launch_before
|
||||
assert (profile / "config.yaml").read_bytes() == worker_before
|
||||
response = server._methods["tools.configure"](3, {"action": "disable", "names": ["terminal"]})
|
||||
assert "error" not in response and not response["result"]["reset"]
|
||||
assert (home / "config.yaml").read_bytes() != launch_before
|
||||
assert (profile / "config.yaml").read_bytes() == worker_before
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", ["reset", "capabilities"])
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
"""An unavailable explicit target must never become the launch profile."""
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def test_explicit_profile_target_never_falls_back(tmp_path, monkeypatch):
|
||||
from tui_gateway import server
|
||||
from hermes_state import SessionDB
|
||||
|
||||
home = tmp_path / ".hermes"
|
||||
worker = home / "profiles" / "worker"
|
||||
worker.mkdir(parents=True)
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setattr(server, "_hermes_home", home)
|
||||
for path, marker in ((home, "launch"), (worker, "worker")):
|
||||
(path / "config.yaml").write_text(f"terminal:\n cwd: /{marker}\n")
|
||||
with SessionDB(db_path=path / "state.db") as db:
|
||||
db.create_session(marker, "tui")
|
||||
for name, marker in ((None, "launch"), ("default", "launch"), ("DEFAULT", "launch"), ("worker", "worker")):
|
||||
with server._profile_db({"profile": name}) as db:
|
||||
assert db.get_session(marker)
|
||||
response = server._methods["config.get"](1, {"profile": name, "key": "full"})
|
||||
assert response["result"]["config"]["terminal"]["cwd"] == f"/{marker}"
|
||||
before = (home / "config.yaml").read_bytes()
|
||||
worker.rename(worker.with_name("gone"))
|
||||
for name in ("worker", "unknown"):
|
||||
with pytest.raises(FileNotFoundError):
|
||||
with server._profile_db({"profile": name}):
|
||||
pytest.fail("unavailable profile reached a database")
|
||||
with pytest.raises(FileNotFoundError):
|
||||
server._methods["config.set"](2, {"profile": name, "key": "busy", "value": "steer"})
|
||||
assert (home / "config.yaml").read_bytes() == before
|
||||
# A real resolution I/O failure must propagate, too (no predicate patch).
|
||||
profiles = home / "profiles"
|
||||
profiles.rename(home / "saved-profiles")
|
||||
profiles.symlink_to("profiles")
|
||||
with pytest.raises((OSError, RuntimeError)):
|
||||
server._profile_home("worker")
|
||||
@@ -981,7 +981,11 @@ def _(rid, params: dict) -> dict:
|
||||
@_rpc("tools.configure", 5035)
|
||||
def _(rid, params: dict) -> dict:
|
||||
sid = params.get("session_id", "")
|
||||
session = _sessions.get(sid)
|
||||
session = None
|
||||
if sid:
|
||||
session, err = _sess_nowait(params, rid)
|
||||
if err:
|
||||
return err
|
||||
# The client sends session_id, not profile; the live session is authoritative.
|
||||
home = (session or {}).get("profile_home")
|
||||
scopes = _bind_build_profile_scopes(home) if home else None
|
||||
|
||||
@@ -460,13 +460,12 @@ def _profile_home(profile: str | None) -> Path | None:
|
||||
"""Resolve a named profile's home on THIS host, or None for the launch profile."""
|
||||
if not (name := (profile or "").strip()):
|
||||
return None
|
||||
try:
|
||||
from hermes_cli import profiles as profiles_mod
|
||||
home = Path(profiles_mod.get_profile_dir(name))
|
||||
except Exception:
|
||||
return None
|
||||
if home.resolve() == Path(_hermes_home).resolve() or not home.exists():
|
||||
return None # already the launch profile (no override needed), or no such profile
|
||||
from hermes_cli import profiles as profiles_mod
|
||||
home = Path(profiles_mod.get_profile_dir(name))
|
||||
if not home.is_dir():
|
||||
raise FileNotFoundError(f"Profile '{name}' does not exist.")
|
||||
if home.resolve() == Path(_hermes_home).resolve():
|
||||
return None # already the launch profile (no override needed)
|
||||
_served_profile_homes.add(home) # the change watcher must stat every served sibling store too
|
||||
return home
|
||||
|
||||
|
||||
@@ -17,6 +17,10 @@ Releasing the outgoing agent must not close the handle inherited by its replacem
|
||||
even when the client supplies only `session_id`. Rebuilds prepare model configuration
|
||||
before allocating a replacement, then install the agent and transfer ownership
|
||||
together; preparation failure leaves the existing agent responsible for teardown.
|
||||
Explicit profiles that cannot be resolved or whose directory has disappeared fail
|
||||
before accessing launch configuration or history. A stale `tools.configure`
|
||||
session ID likewise returns `session not found` without changing configuration;
|
||||
omitting the session ID still supports the global settings operation.
|
||||
|
||||
In-place compaction archives old rows with `active=0` and inserts the retained
|
||||
context as `active=1` rows. A protected message can therefore legitimately appear
|
||||
|
||||
Reference in New Issue
Block a user