From f6b133b5981593572ee25d3d9c58885a018e4714 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 03:02:48 +0000 Subject: [PATCH 1/7] fix(*): refresh an acp agent's model menu from a dispatch reply The model menu on an acp agent's card is read from the session/new handshake and cached in the capability snapshot, which never expires by age; only a Test, a Connect or the boot backfill re-measured it. Every dispatch already re-learned the mode menu from the session reply it holds, but not the model menu beside it in the same reply, so a card could keep offering model ids the agent had long stopped serving. The relearn path (now relearn_session_menus) reads both menus off that reply with the probe's own parsers, grouped choices included, and opens no session and starts no process to do it. Each menu is judged on its own, and a reply that carries no model entry, an empty one, or one the adapter flagged as unread keeps the recorded menu rather than clearing it. A menu recorded as unmeasured becomes measured once a reply carries one. The models.availableModels key and the model_source rule are unchanged. The write now goes through SnapshotStore.revise, which stands down when the stored row carries a newer measurement stamp than the record the backend was built from. A Test or boot verify that lands while a dispatch is in flight on the previous backend is no longer overwritten by that backend's older record; this protects the mode relearn too. --- raven/acp_client/acp_agent.py | 21 +-- raven/acp_client/capabilities.py | 84 +++++++--- raven/agent/subagent/probe.py | 6 +- tests/test_rpc_subagents.py | 59 +++++++ tests/test_subagent_acp.py | 259 +++++++++++++++++++++++++++++-- tests/test_subagent_probe.py | 2 +- 6 files changed, 383 insertions(+), 48 deletions(-) diff --git a/raven/acp_client/acp_agent.py b/raven/acp_client/acp_agent.py index faf8a5772..b0f132ab0 100644 --- a/raven/acp_client/acp_agent.py +++ b/raven/acp_client/acp_agent.py @@ -42,7 +42,7 @@ from raven.acp_client.capabilities import ( CapabilitySnapshot, read_session_current_model, - relearn_session_modes, + relearn_session_menus, steer_offered, ) from raven.acp_client.elicitor import Elicitor @@ -1257,17 +1257,18 @@ def _note_session_model(self, session_id: str | None, result: Any) -> None: if current: self._model_baseline[session_id] = current - def _relearn_modes(self, result: Any) -> None: - """Take the modes a session response advertises over the cached ones. + def _relearn_menus(self, result: Any) -> None: + """Take the modes and models a session response advertises over the cached ones. - Best-effort at every step. The menu is a description the NEXT dispatch - reads, never something this turn depends on, so neither the write nor - the table rebuild may cost the turn that happened to carry the evidence. + Best-effort at every step. The menus are what the NEXT dispatch and the + row's picker read, never something this turn depends on, so neither the + write nor the table rebuild may cost the turn that happened to carry the + evidence. """ try: - updated = relearn_session_modes(self._snapshot, result) + updated = relearn_session_menus(self._snapshot, result) except Exception as exc: # noqa: BLE001 - a cache refresh must not sink a turn - logger.warning("acp agent {!r}: refreshing modes from the session failed: {}", self.name, exc) + logger.warning("acp agent {!r}: refreshing menus from the session failed: {}", self.name, exc) return if updated is None: return @@ -2198,7 +2199,7 @@ async def request_or_busy(method: str, params: dict[str, Any]) -> Any: raise loaded = None if loaded is not None: - self._relearn_modes(loaded) + self._relearn_menus(loaded) self._note_session_model(known, loaded) await self._set_mode(client, known, mode, budget=budget) await self._set_model(client, known, session_model, budget=budget) @@ -2223,7 +2224,7 @@ async def request_or_busy(method: str, params: dict[str, Any]) -> Any: raise result = await request_or_busy("session/new", {"cwd": cwd, "mcpServers": mcp_servers}) - self._relearn_modes(result) + self._relearn_menus(result) session_id = (result or {}).get("sessionId") if isinstance(result, dict) else None if not isinstance(session_id, str) or not session_id: raise AcpEmptyTurnError(f"acp agent {self.name!r}: session/new returned no sessionId") diff --git a/raven/acp_client/capabilities.py b/raven/acp_client/capabilities.py index 77d65b233..98e9abf50 100644 --- a/raven/acp_client/capabilities.py +++ b/raven/acp_client/capabilities.py @@ -396,6 +396,33 @@ def merge(current: str | None) -> tuple[str, None]: except OSError as exc: # a snapshot is a cache, never a dependency logger.warning("acp capability snapshot write failed (not remembered): {}", exc) + def revise(self, snapshot: CapabilitySnapshot) -> bool: + """Store ``snapshot`` unless a newer measurement has replaced the row it was read from. + + For a write that re-measured part of a record and carries the rest of it + unchanged: the stored row must still hold the measurement stamp the + caller started from. A Test or the boot backfill that recorded in the + meantime is newer evidence about everything, and writing the older + record back over it would undo that verdict and its fingerprint. The + same "newest measurement wins" rule the boot backfill applies to its own + late verdicts. ``False`` when the write stood down. + """ + + def merge(current: str | None) -> tuple[str | None, bool]: + rows = self._parse(current) + stored = next((r for r in rows if r.get("agent") == snapshot.agent), None) + if stored is not None and int(stored.get("measuredAtMs") or 0) != snapshot.measured_at_ms: + return None, False + rows = [r for r in rows if r.get("agent") != snapshot.agent] + rows.append(snapshot.to_row()) + return self._dump(rows), True + + try: + return atomic_update(self._path, merge) + except OSError as exc: # a snapshot is a cache, never a dependency + logger.warning("acp capability snapshot write failed (not remembered): {}", exc) + return False + def load(self, configs: Sequence[Any], *, allow_stale: bool = False) -> dict[str, CapabilitySnapshot]: """Snapshots for these configs, keyed by agent name. @@ -670,43 +697,62 @@ def _read_session_modes(result: Any) -> tuple[AcpMode, ...]: ) -def relearn_session_modes( +def relearn_session_menus( snapshot: CapabilitySnapshot | None, result: Any, *, store: SnapshotStore | None = None, ) -> CapabilitySnapshot | None: - """Re-record an agent's modes from a live session response, if they moved. + """Re-record an agent's modes and model menu from a live session response, if they moved. - The menu is measured once at probe time and cached, but the agent - re-advertises it on every route into a session (``_with_modes`` in the + Both menus are measured once at probe time and cached, but the agent + re-advertises them on every route into a session (``_with_modes`` in the fork's ACP methods), so the response a dispatch already holds is newer evidence than the snapshot. Without this the cache only ever moves when someone presses Test: ``snapshot_fingerprint`` covers how the agent is - LAUNCHED, and an agent that reworded its own modes launches identically, so - nothing marks the stored text stale and the dispatching model goes on - reading a menu the agent no longer serves. + LAUNCHED, and an agent that reworded its own modes or renamed its models + launches identically, so nothing marks the stored menus stale -- the + dispatching model goes on reading modes the agent no longer serves, and the + row's picker goes on offering model ids it no longer takes. - The stored fingerprint is kept rather than recomputed. Only the modes were - re-measured here, and claiming a fresh handshake would clear a staleness - flag the rest of the snapshot has not earned. + Read with the probe's own parsers, from the response this route was sending + for anyway: nothing here opens a session or starts a process. + + The stored fingerprint and measurement stamp are kept rather than renewed. + Only the menus were re-measured here, and claiming a fresh handshake would + clear a staleness flag the rest of the snapshot has not earned. The write + stands down when the stored row carries a newer stamp than ``snapshot`` + (``SnapshotStore.revise``), since this record would undo that measurement. ``None`` when there was nothing to learn, which includes a response carrying - no modes at all. That is read as "this route did not report them", never as - "the agent dropped them": erasing the menu takes the ``mode`` argument out - of the spawn schema entirely, and a stale description is the smaller harm. + neither menu. Each menu is judged on its own, and an absent or empty one is + read as "this route did not report it", never as "the agent dropped it": + erasing the modes takes the ``mode`` argument out of the spawn schema, and + erasing the models takes the picker off the row, both worse than a stale + entry. A model menu the agent flagged as unread is not taken either, for + the reason ``_read_session_menu_unread`` gives. """ if snapshot is None: return None + changes: dict[str, Any] = {} modes = _read_session_modes(result) - if not modes or modes == snapshot.available_modes: + if modes and modes != snapshot.available_modes: + changes["available_modes"] = modes + choices = () if _read_session_menu_unread(result) else _read_session_model_choices(result) + if choices and (choices != snapshot.model_choices or not snapshot.model_menu_measured): + changes["model_choices"] = choices + changes["model_menu_measured"] = True + if not changes: + return None + updated = replace(snapshot, **changes) + if not (store or SnapshotStore()).revise(updated): + logger.info("acp agent {!r}: a newer measurement is stored; session menus not re-recorded", snapshot.agent) return None - updated = replace(snapshot, available_modes=modes) - (store or SnapshotStore()).record(updated) logger.info( - "acp agent {!r}: re-recorded {} mode(s) from a session response", + "acp agent {!r}: re-recorded {} mode(s) and {} model(s) from a session response", snapshot.agent, - len(modes), + len(updated.available_modes), + len(updated.model_choices), ) return updated @@ -1079,7 +1125,7 @@ def done( "looks_like_auth", "npx_fetch_failure", "npx_fetch_lead", - "relearn_session_modes", + "relearn_session_menus", "snapshot_fingerprint", "steer_offered", "verify_agent", diff --git a/raven/agent/subagent/probe.py b/raven/agent/subagent/probe.py index f031bcccd..8b45e8f69 100644 --- a/raven/agent/subagent/probe.py +++ b/raven/agent/subagent/probe.py @@ -1349,7 +1349,7 @@ def _measured_at(snapshot: Any) -> int | None: """When the record's verdict was measured; ``None`` for no record. Every measurement stamps its own time (``_test_record`` carries it over), and - a modes relearn keeps the stamp it found, so a changed stamp means a newer + a menu relearn keeps the stamp it found, so a changed stamp means a newer verdict and nothing else does. """ return None if snapshot is None else getattr(snapshot, "measured_at_ms", 0) @@ -1545,9 +1545,9 @@ async def _verify_missing_snapshots(manager: Any, rows: list[Any], *, configured # Read again with nothing awaited between here and the write: the # handshake held the row for up to the ready timeout. A measurement # recorded meanwhile -- a manual Test, a connect -- is newer evidence - # than this one and stands. A dispatch relearning the agent's modes + # than this one and stands. A dispatch relearning the agent's menus # measures nothing and keeps the stamp, so it does not stop this - # write, and a failure below is kept over the re-read record, modes + # write, and a failure below is kept over the re-read record, menus # included. current = acp_snapshot_for(cfg) if _measured_at(current) != _measured_at(snapshot): diff --git a/tests/test_rpc_subagents.py b/tests/test_rpc_subagents.py index aa0223727..882157715 100644 --- a/tests/test_rpc_subagents.py +++ b/tests/test_rpc_subagents.py @@ -3198,6 +3198,65 @@ async def test_list_marks_a_config_row_whose_handshake_named_raven_as_ravens_own assert rows["Other-Quiet"]["own"] is False and rows["Other-Quiet"]["model_source"] == "agent" +async def test_list_shows_the_model_menu_a_dispatch_relearned( + config_path: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The row's picker follows the agent without anyone pressing Test. + + The listing reads the stored snapshot, and the snapshot never expires by + age, so before a dispatch could refresh it the card went on offering the + ids the agent held at its last Test. One dispatch now carries the agent's + current menu to the card, under the same ``agent`` rule as before. + """ + from raven.acp_client.capabilities import AcpModelChoice, CapabilitySnapshot, SnapshotStore, snapshot_fingerprint + from raven.acp_client.pool import close_pool + from raven.agent.subagent.backends import build_third_party_backend + from raven.config.schema import SubagentsConfig + + stub = Path(__file__).with_name("acp_stub_server.py") + raw = json.loads(config_path.read_text()) + raw["subagents"]["agents"].append( + { + "name": "Stubby", + "kind": "acp", + "command": f"{sys.executable} {stub}", + "env": {"ACP_STUB_MODE": "ok"}, + "description": "d", + "enabled": True, + } + ) + config_path.write_text(json.dumps(raw), encoding="utf-8") + store_path = tmp_path / "caps.json" + monkeypatch.setattr("raven.acp_client.capabilities.default_snapshot_path", lambda: store_path) + cfg = next(c for c in SubagentsConfig(agents=raw["subagents"]["agents"]).agents if c.name == "Stubby") + SnapshotStore(path=store_path).record( + CapabilitySnapshot( + agent="Stubby", + fingerprint=snapshot_fingerprint(cfg), + status="ready", + detail="", + measured_at_ms=1, + agent_name="stub-agent", + model_choices=(AcpModelChoice(value="stub:retired", name="retired", group="Stub"),), + ) + ) + before = {r["name"]: r for r in (await subagents_list({"probe": False}))["rows"]}["Stubby"] + assert before["model_choices"] == [{"value": "stub:retired", "name": "retired", "group": "Stub"}] + + try: + reply = await build_third_party_backend(cfg).run("ping", task_id="t1", workspace=tmp_path, executor=None) + finally: + await close_pool() + assert reply == "pong" + + after = {r["name"]: r for r in (await subagents_list({"probe": False}))["rows"]}["Stubby"] + assert after["model_choices"] == [ + {"value": "stub:model-a", "name": "model-a", "group": "Stub"}, + {"value": "stub:model-b", "name": "model-b", "group": "Stub"}, + ] + assert after["model_source"] == before["model_source"] == "agent" + + async def test_test_can_target_a_discovered_product( config_path: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_subagent_acp.py b/tests/test_subagent_acp.py index 4e240a14d..bfab803b1 100644 --- a/tests/test_subagent_acp.py +++ b/tests/test_subagent_acp.py @@ -33,7 +33,7 @@ AcpModelChoice, CapabilitySnapshot, SnapshotStore, - relearn_session_modes, + relearn_session_menus, snapshot_fingerprint, verify_agent, ) @@ -908,7 +908,7 @@ async def test_a_row_recorded_before_the_menu_reads_attention_until_it_is_measur assert (await probe_one(cfg, source="config")).status == "ready" -# ---- relearning the mode menu from a live session -------------------------- +# ---- relearning the session menus from a live session ---------------------- def _with_modes(cfg: Any, *modes: AcpMode) -> CapabilitySnapshot: @@ -950,7 +950,7 @@ def test_a_session_response_rewrites_a_mode_description_the_probe_missed(tmp_pat stored = _with_modes(cfg, AcpMode(id="fast", name="Fast", description="old wording")) store.record(stored) - updated = relearn_session_modes( + updated = relearn_session_menus( stored, _session_result({"id": "fast", "name": "Fast", "description": "new"}), store=store ) @@ -973,7 +973,7 @@ def test_relearning_keeps_the_fingerprint_it_was_measured_under(tmp_path: Path) available_modes=(AcpMode(id="fast", description="old"),), ) - updated = relearn_session_modes(stored, _session_result({"id": "fast", "description": "new"}), store=store) + updated = relearn_session_menus(stored, _session_result({"id": "fast", "description": "new"}), store=store) assert updated is not None and updated.fingerprint == "measured-under-this" assert store.load([cfg]) == {} @@ -989,8 +989,8 @@ def test_a_response_without_modes_leaves_the_menu_alone(tmp_path: Path) -> None: stored = _with_modes(cfg, AcpMode(id="fast", description="kept")) store.record(stored) - assert relearn_session_modes(stored, {"sessionId": "s1"}, store=store) is None - assert relearn_session_modes(stored, _session_result(), store=store) is None + assert relearn_session_menus(stored, {"sessionId": "s1"}, store=store) is None + assert relearn_session_menus(stored, _session_result(), store=store) is None assert [m.description for m in store.load([cfg])["a"].available_modes] == ["kept"] @@ -1002,7 +1002,7 @@ def test_an_unchanged_menu_is_not_rewritten(tmp_path: Path) -> None: stored = _with_modes(cfg, AcpMode(id="fast", name="Fast", description="same")) assert ( - relearn_session_modes( + relearn_session_menus( stored, _session_result({"id": "fast", "name": "Fast", "description": "same"}), store=store ) is None @@ -1014,10 +1014,188 @@ def test_nothing_is_invented_for_an_agent_never_measured(tmp_path: Path) -> None """A snapshot carries a fingerprint and a status this response cannot supply, so there is nothing to update -- the probe is what creates one.""" store = SnapshotStore(path=tmp_path / "caps.json") - assert relearn_session_modes(None, _session_result({"id": "fast"}), store=store) is None + assert relearn_session_menus(None, _session_result({"id": "fast"}), store=store) is None assert not (tmp_path / "caps.json").exists() +def _menu_reply( + *, models: list[dict[str, Any]] | None = None, modes: list[dict[str, str]] | None = None, unread: bool = False +) -> dict[str, Any]: + """A session response carrying whichever menus are given, in the wire shapes.""" + from raven.acp_client.protocol import MENU_UNREAD_META + + reply: dict[str, Any] = {"sessionId": "s1"} + if models is not None: + reply["configOptions"] = [ + {"id": "mode", "category": "mode", "type": "select", "options": [{"value": "not-a-model"}]}, + {"id": "model", "category": "model", "type": "select", "currentValue": "x", "options": models}, + ] + if modes is not None: + reply["modes"] = {"currentModeId": "fast", "availableModes": modes} + if unread: + reply["_meta"] = {"raven": {MENU_UNREAD_META: True}} + return reply + + +_GROUPED_MODELS: list[dict[str, Any]] = [ + {"group": "vendor-a", "name": "Vendor A", "options": [{"value": "a/new-1", "name": "new-1"}]}, + {"group": "vendor-b", "name": "Vendor B", "options": [{"value": "b/new-2", "name": "new-2"}, {"name": "no-value"}]}, +] +_GROUPED_CHOICES = ( + AcpModelChoice(value="a/new-1", name="new-1", group="Vendor A"), + AcpModelChoice(value="b/new-2", name="new-2", group="Vendor B"), +) + + +def test_a_session_response_rewrites_a_model_menu_the_probe_captured_long_ago(tmp_path: Path) -> None: + """The model menu has the gap the mode menu had. + + Nothing expires a snapshot by age, and an agent that renamed its models + launches identically, so the row's picker went on offering the ids it held + at the last Test. The dispatch already holds the agent's current list; it + is read with the probe's own parser, groups included, and kept under the + measurement stamp and fingerprint the rest of the record was taken with. + """ + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + stored = _with_model_choices(cfg, AcpModelChoice(value="old/retired", name="retired", group="Old")) + store.record(stored) + + updated = relearn_session_menus(stored, _menu_reply(models=_GROUPED_MODELS), store=store) + + assert updated is not None and updated.model_choices == _GROUPED_CHOICES + assert (updated.fingerprint, updated.measured_at_ms) == (stored.fingerprint, stored.measured_at_ms) + assert store.load([cfg])["a"].model_choices == _GROUPED_CHOICES + + +def test_a_response_without_a_model_menu_keeps_the_one_recorded(tmp_path: Path) -> None: + """Silence is "this route did not report it", never "the agent has none". + + Four silences, each one a shape a live route returns: no ``configOptions`` + at all, options with no ``model`` entry, a ``model`` entry whose list is + empty (a resumed session's answer), and raven's own adapter saying its + catalog read failed. Erasing the menu on any of them takes the picker off + the row until someone presses Test. + """ + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + kept = (AcpModelChoice(value="v/kept", name="kept", group="V"),) + stored = _with_model_choices(cfg, *kept) + store.record(stored) + + no_model_entry = {"sessionId": "s1", "configOptions": [{"id": "mode", "category": "mode", "options": []}]} + for reply in ( + {"sessionId": "s1"}, + no_model_entry, + _menu_reply(models=[]), + _menu_reply(models=[{"name": "no-value-anywhere"}]), + _menu_reply(models=[], unread=True), + ): + assert relearn_session_menus(stored, reply, store=store) is None, reply + assert store.load([cfg])["a"].model_choices == kept + + +def test_a_menu_the_catalog_failed_to_read_is_not_taken(tmp_path: Path) -> None: + """Raven's adapter flags a reply whose catalog read failed. Whatever it + carries beside that flag is not a measurement of the agent's models.""" + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + kept = (AcpModelChoice(value="v/kept"),) + stored = _with_model_choices(cfg, *kept) + store.record(stored) + + assert relearn_session_menus(stored, _menu_reply(models=_GROUPED_MODELS, unread=True), store=store) is None + assert store.load([cfg])["a"].model_choices == kept + + +def test_the_model_and_mode_menus_are_relearned_independently(tmp_path: Path) -> None: + """One reply may carry either menu, both, or neither, and each is judged on + its own: a reply naming models only must not cost the modes, and one naming + modes only must not cost the models.""" + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + stored = replace( + _with_modes(cfg, AcpMode(id="fast", description="old")), + model_choices=(AcpModelChoice(value="old/model"),), + ) + store.record(stored) + + models_only = relearn_session_menus(stored, _menu_reply(models=_GROUPED_MODELS), store=store) + assert models_only is not None + assert models_only.model_choices == _GROUPED_CHOICES + assert models_only.available_modes == stored.available_modes + + modes_only = relearn_session_menus(models_only, _session_result({"id": "deep", "description": "new"}), store=store) + assert modes_only is not None + assert [m.id for m in modes_only.available_modes] == ["deep"] + assert modes_only.model_choices == _GROUPED_CHOICES + + flat = [{"value": "flat/one", "name": "one"}] + both = relearn_session_menus( + modes_only, _menu_reply(models=flat, modes=[{"id": "fast", "description": "back"}]), store=store + ) + assert both is not None + assert both.model_choices == (AcpModelChoice(value="flat/one", name="one", group=""),) + assert [m.id for m in both.available_modes] == ["fast"] + + reread = store.load([cfg])["a"] + assert (reread.model_choices, reread.available_modes) == (both.model_choices, both.available_modes) + + +def test_an_unchanged_model_menu_is_not_rewritten(tmp_path: Path) -> None: + """Every route into a session carries the menu; a write per dispatch is the + cost the mode relearn already refuses.""" + cfg = stub_config("a") + stored = _with_model_choices(cfg, *_GROUPED_CHOICES) + + assert ( + relearn_session_menus(stored, _menu_reply(models=_GROUPED_MODELS), store=SnapshotStore(tmp_path / "c")) is None + ) + assert not (tmp_path / "c").exists() + + +def test_a_menu_never_measured_becomes_measured_once_a_reply_carries_it(tmp_path: Path) -> None: + """A row recorded before the menu existed reads back with an empty one. The + same menu read off a reply is a measurement, and the row must say so, or the + boot backfill goes on re-measuring a menu that is already known.""" + cfg = stub_config("a") + path = tmp_path / "caps.json" + stored = replace(_with_model_choices(cfg), model_menu_measured=False) + store = SnapshotStore(path=path) + store.record(stored) + assert store.has_model_menu("a") is False + + updated = relearn_session_menus(stored, _menu_reply(models=_GROUPED_MODELS), store=store) + + assert updated is not None and updated.model_menu_measured is True + assert store.has_model_menu("a") is True + assert store.load([cfg])["a"].model_choices == _GROUPED_CHOICES + + +def test_a_relearn_from_an_older_record_does_not_overwrite_a_newer_measurement(tmp_path: Path) -> None: + """A Test, or the boot backfill, may record while a dispatch is in flight on + a backend built from the record before it. That backend's relearn holds the + older record whole, and writing it back would undo the newer verdict, + fingerprint included -- so it stands down when the stamp it was built from + is no longer the one stored. The newer measurement brought its own menus.""" + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + older = _with_model_choices(cfg, AcpModelChoice(value="old/retired")) + store.record(older) + newer = replace( + older, + status="attention", + detail="measured after the dispatch began", + measured_at_ms=older.measured_at_ms + 1, + model_choices=(AcpModelChoice(value="test/fresh"),), + ) + store.record(newer) + + assert relearn_session_menus(older, _menu_reply(models=_GROUPED_MODELS), store=store) is None + assert relearn_session_menus(older, _session_result({"id": "deep"}), store=store) is None + assert store.load([cfg])["a"] == newer + + def test_has_model_menu_is_false_for_a_row_recorded_before_the_key_existed(tmp_path: Path) -> None: """A fabricated older-format row: everything ``record`` would have written, minus the ``modelChoices`` key that did not exist yet when it was recorded. @@ -1084,9 +1262,9 @@ def test_the_backend_rebuilds_the_agent_table_when_the_menu_moved(tmp_path: Path rebuilds: list[int] = [] backend.bind_caps_listener(lambda: rebuilds.append(1)) - with patch("raven.acp_client.acp_agent.relearn_session_modes") as fake: + with patch("raven.acp_client.acp_agent.relearn_session_menus") as fake: fake.return_value = _with_modes(cfg, AcpMode(id="fast", description="new")) - backend._relearn_modes(_session_result({"id": "fast", "description": "new"})) + backend._relearn_menus(_session_result({"id": "fast", "description": "new"})) assert rebuilds == [1] assert [m.description for m in backend._snapshot.available_modes] == ["new"] @@ -1100,13 +1278,13 @@ def test_a_failed_rebuild_does_not_reach_the_turn() -> None: backend = AcpAgentBackend(name="a", command="true", snapshot=stored) backend.bind_caps_listener(lambda: (_ for _ in ()).throw(RuntimeError("boom"))) - with patch("raven.acp_client.acp_agent.relearn_session_modes") as fake: + with patch("raven.acp_client.acp_agent.relearn_session_menus") as fake: fake.side_effect = RuntimeError("store is unwritable") - backend._relearn_modes(_session_result({"id": "fast", "description": "new"})) + backend._relearn_menus(_session_result({"id": "fast", "description": "new"})) - with patch("raven.acp_client.acp_agent.relearn_session_modes") as fake: + with patch("raven.acp_client.acp_agent.relearn_session_menus") as fake: fake.return_value = _with_modes(cfg, AcpMode(id="fast", description="new")) - backend._relearn_modes(_session_result({"id": "fast", "description": "new"})) + backend._relearn_menus(_session_result({"id": "fast", "description": "new"})) assert [m.description for m in backend._snapshot.available_modes] == ["new"] @@ -1161,7 +1339,7 @@ def test_the_relearned_menu_reaches_the_roster_the_clamp_and_picker_read( mgr = SubagentManager(provider=_StubProvider(), workspace=tmp_path, max_concurrent=1, agents=[cfg]) assert "what it used to say" in [m.description for m in mgr.agent_modes("a")] - updated = relearn_session_modes( + updated = relearn_session_menus( stored, _session_result({"id": "fast", "name": "Fast", "description": "what it says now"}), store=SnapshotStore(path=path), @@ -1936,6 +2114,57 @@ async def test_a_dispatch_relearns_the_menu_the_agent_now_serves( ] +async def test_a_dispatch_relearns_the_model_menu_from_its_own_session_and_nothing_else( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The model menu rides the same reply, and costs nothing to read off it. + + Pinned both ways: the stored menu moves to what the agent serves now, and + the dispatch sends the one ``session/new`` it was always going to send -- + no verify, no probe, no second launch. A refresh that bought its evidence + with a handshake would spend a process start on every dispatch. + """ + from raven.acp_client.client import AcpClient + + path = tmp_path / "caps.json" + monkeypatch.setattr("raven.acp_client.capabilities.default_snapshot_path", lambda: path) + cfg = stub_config("a") + SnapshotStore(path=path).record( + _with_model_choices(cfg, AcpModelChoice(value="stub:retired", name="retired", group="Stub")) + ) + + async def no_handshake(*_a: Any, **_kw: Any) -> Any: + raise AssertionError("a dispatch must not measure the agent to refresh its menu") + + monkeypatch.setattr("raven.acp_client.capabilities.verify_agent", no_handshake) + monkeypatch.setattr("raven.agent.subagent.probe.record_capabilities", no_handshake) + monkeypatch.setattr("raven.agent.subagent.probe.ping_agent", no_handshake) + sent: list[str] = [] + launches: list[str] = [] + real_request, real_launch = AcpClient.request, AcpClient.launch.__func__ + + async def counting_request(self: Any, method: str, *args: Any, **kwargs: Any) -> Any: + sent.append(method) + return await real_request(self, method, *args, **kwargs) + + async def counting_launch(cls: Any, **kwargs: Any) -> Any: + launches.append(kwargs.get("name", "")) + return await real_launch(cls, **kwargs) + + monkeypatch.setattr(AcpClient, "request", counting_request) + monkeypatch.setattr(AcpClient, "launch", classmethod(counting_launch)) + + backend = build_third_party_backend(cfg) + assert await backend.run("ping", task_id="t1", workspace=tmp_path, executor=None) == "pong" + + assert SnapshotStore(path=path).load([cfg])["a"].model_choices == ( + AcpModelChoice(value="stub:model-a", name="model-a", group="Stub"), + AcpModelChoice(value="stub:model-b", name="model-b", group="Stub"), + ) + assert launches == ["a"], "one process, the dispatch's own" + assert sent.count("initialize") == 1 and sent.count("session/new") == 1, sent + + async def test_dispatch_records_the_runs_own_transcript(tmp_path: Path) -> None: """The acp lane can see every step, so the record keeps them. diff --git a/tests/test_subagent_probe.py b/tests/test_subagent_probe.py index b4b9d457c..fdfb7242c 100644 --- a/tests/test_subagent_probe.py +++ b/tests/test_subagent_probe.py @@ -2060,7 +2060,7 @@ async def fake_verify(cfg, **kwargs): manager = _FakeManager(rows, with_refresh=True) boot = asyncio.ensure_future(probe_mod._verify_missing_snapshots(manager, rows)) await asyncio.wait_for(boot_parked.wait(), 2) - relearned = caps.relearn_session_modes( + relearned = caps.relearn_session_menus( store.load([cfg])[cfg.name], {"sessionId": "s1", "modes": {"availableModes": [{"id": "fast", "name": "Fast"}], "currentModeId": "fast"}}, store=store, From 5157d8ecd64530269bb10badcf1d3e144f04e593 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 03:08:29 +0000 Subject: [PATCH 2/7] fix(*): revise only the menus a session reply reported Backends of one agent each hold the record they were built from, and a relearn on one does not reach the others' copies. Writing a backend's whole copy back meant a second backend, whose reply reported only modes, restored the model menu the first had just refreshed. SnapshotStore.revise now applies just the fields the reply reported, onto the row as read inside the file lock, and still stands down when that row carries another measurement stamp. The stale flag of the caller's record is kept on what it returns, since re-reading menus is not a handshake. A redundant model_menu_measured clause is dropped: an unmeasured menu is always empty, so any reported menu already differs. --- raven/acp_client/capabilities.py | 53 +++++++++++++++++++------------- tests/test_subagent_acp.py | 37 ++++++++++++++++++++++ 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/raven/acp_client/capabilities.py b/raven/acp_client/capabilities.py index 98e9abf50..fd44b994d 100644 --- a/raven/acp_client/capabilities.py +++ b/raven/acp_client/capabilities.py @@ -396,32 +396,37 @@ def merge(current: str | None) -> tuple[str, None]: except OSError as exc: # a snapshot is a cache, never a dependency logger.warning("acp capability snapshot write failed (not remembered): {}", exc) - def revise(self, snapshot: CapabilitySnapshot) -> bool: - """Store ``snapshot`` unless a newer measurement has replaced the row it was read from. - - For a write that re-measured part of a record and carries the rest of it - unchanged: the stored row must still hold the measurement stamp the - caller started from. A Test or the boot backfill that recorded in the - meantime is newer evidence about everything, and writing the older - record back over it would undo that verdict and its fingerprint. The - same "newest measurement wins" rule the boot backfill applies to its own - late verdicts. ``False`` when the write stood down. + def revise(self, snapshot: CapabilitySnapshot, **changes: Any) -> CapabilitySnapshot | None: + """Apply ``changes`` to the stored record ``snapshot`` was read from, and return the result. + + For a write that re-measured some fields of a record, so only those are + written, onto the row as it stands inside the lock: another holder of + the same record may have revised a different field since ``snapshot`` + was read, and writing the caller's whole copy back would undo it. + + ``None`` -- nothing written -- when the stored row carries another + measurement stamp than ``snapshot``. A Test or the boot backfill that + recorded in the meantime is newer evidence about everything, the same + "newest measurement wins" rule the boot backfill applies to its own + late verdicts. With no stored row, ``snapshot`` itself is the base. """ - def merge(current: str | None) -> tuple[str | None, bool]: + def merge(current: str | None) -> tuple[str | None, CapabilitySnapshot | None]: rows = self._parse(current) stored = next((r for r in rows if r.get("agent") == snapshot.agent), None) if stored is not None and int(stored.get("measuredAtMs") or 0) != snapshot.measured_at_ms: - return None, False + return None, None + base = (CapabilitySnapshot.from_row(stored) if stored is not None else None) or snapshot + updated = replace(base, stale=snapshot.stale, **changes) rows = [r for r in rows if r.get("agent") != snapshot.agent] - rows.append(snapshot.to_row()) - return self._dump(rows), True + rows.append(updated.to_row()) + return self._dump(rows), updated try: return atomic_update(self._path, merge) except OSError as exc: # a snapshot is a cache, never a dependency logger.warning("acp capability snapshot write failed (not remembered): {}", exc) - return False + return None def load(self, configs: Sequence[Any], *, allow_stale: bool = False) -> dict[str, CapabilitySnapshot]: """Snapshots for these configs, keyed by agent name. @@ -720,9 +725,10 @@ def relearn_session_menus( The stored fingerprint and measurement stamp are kept rather than renewed. Only the menus were re-measured here, and claiming a fresh handshake would - clear a staleness flag the rest of the snapshot has not earned. The write - stands down when the stored row carries a newer stamp than ``snapshot`` - (``SnapshotStore.revise``), since this record would undo that measurement. + clear a staleness flag the rest of the snapshot has not earned. Only the + menus this response reported are written, and none when a newer + measurement has replaced the record ``snapshot`` was read from + (``SnapshotStore.revise``). ``None`` when there was nothing to learn, which includes a response carrying neither menu. Each menu is judged on its own, and an absent or empty one is @@ -739,14 +745,17 @@ def relearn_session_menus( if modes and modes != snapshot.available_modes: changes["available_modes"] = modes choices = () if _read_session_menu_unread(result) else _read_session_model_choices(result) - if choices and (choices != snapshot.model_choices or not snapshot.model_menu_measured): + if choices and choices != snapshot.model_choices: changes["model_choices"] = choices changes["model_menu_measured"] = True if not changes: return None - updated = replace(snapshot, **changes) - if not (store or SnapshotStore()).revise(updated): - logger.info("acp agent {!r}: a newer measurement is stored; session menus not re-recorded", snapshot.agent) + updated = (store or SnapshotStore()).revise(snapshot, **changes) + if updated is None: + logger.info( + "acp agent {!r}: session menus not re-recorded; a newer measurement stands or the store is unwritable", + snapshot.agent, + ) return None logger.info( "acp agent {!r}: re-recorded {} mode(s) and {} model(s) from a session response", diff --git a/tests/test_subagent_acp.py b/tests/test_subagent_acp.py index bfab803b1..6bbe4ac96 100644 --- a/tests/test_subagent_acp.py +++ b/tests/test_subagent_acp.py @@ -1068,6 +1068,21 @@ def test_a_session_response_rewrites_a_model_menu_the_probe_captured_long_ago(tm assert store.load([cfg])["a"].model_choices == _GROUPED_CHOICES +def test_a_relearned_record_measured_under_an_old_launch_config_stays_stale(tmp_path: Path) -> None: + """Re-reading the menus is not a handshake, so a record already flagged + stale against the agent's current launch config is handed back flagged.""" + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + store.record(replace(_with_model_choices(cfg, AcpModelChoice(value="old/retired")), fingerprint="an-older-config")) + stale = store.load([cfg], allow_stale=True)["a"] + assert stale.stale is True + + updated = relearn_session_menus(stale, _menu_reply(models=_GROUPED_MODELS), store=store) + + assert updated is not None and updated.stale is True + assert updated.fingerprint == "an-older-config" + + def test_a_response_without_a_model_menu_keeps_the_one_recorded(tmp_path: Path) -> None: """Silence is "this route did not report it", never "the agent has none". @@ -1196,6 +1211,28 @@ def test_a_relearn_from_an_older_record_does_not_overwrite_a_newer_measurement(t assert store.load([cfg])["a"] == newer +def test_a_relearn_writes_only_the_menu_its_reply_reported(tmp_path: Path) -> None: + """Backends of one agent each hold the record they were built from, and a + relearn on one does not reach the others' copies. A second backend whose + reply reported only modes must therefore leave the model menu the first + one stored alone, rather than writing back the one it was built with.""" + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + built_from = replace( + _with_modes(cfg, AcpMode(id="fast", description="old")), + model_choices=(AcpModelChoice(value="old/retired"),), + ) + store.record(built_from) + + assert relearn_session_menus(built_from, _menu_reply(models=_GROUPED_MODELS), store=store) is not None + other = relearn_session_menus(built_from, _session_result({"id": "deep", "description": "new"}), store=store) + + assert other is not None and other.model_choices == _GROUPED_CHOICES + reread = store.load([cfg])["a"] + assert reread.model_choices == _GROUPED_CHOICES + assert [m.id for m in reread.available_modes] == ["deep"] + + def test_has_model_menu_is_false_for_a_row_recorded_before_the_key_existed(tmp_path: Path) -> None: """A fabricated older-format row: everything ``record`` would have written, minus the ``modelChoices`` key that did not exist yet when it was recorded. From b941988be2c42ccf6392aa3e07f42dc4c814be9a Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 03:10:01 +0000 Subject: [PATCH 3/7] docs(acp_client): say which session routes re-advertise the model menu Raven's own ACP server attaches configOptions to session/new only; session/load and the resume route carry the modes alone. The relearn docstring claimed both menus arrive on every route. --- raven/acp_client/capabilities.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/raven/acp_client/capabilities.py b/raven/acp_client/capabilities.py index fd44b994d..749826c47 100644 --- a/raven/acp_client/capabilities.py +++ b/raven/acp_client/capabilities.py @@ -711,8 +711,9 @@ def relearn_session_menus( """Re-record an agent's modes and model menu from a live session response, if they moved. Both menus are measured once at probe time and cached, but the agent - re-advertises them on every route into a session (``_with_modes`` in the - fork's ACP methods), so the response a dispatch already holds is newer + re-advertises them on its way into a session -- the modes on every route + (``_with_modes`` in the fork's ACP methods), the models at least on + ``session/new`` -- so the response a dispatch already holds is newer evidence than the snapshot. Without this the cache only ever moves when someone presses Test: ``snapshot_fingerprint`` covers how the agent is LAUNCHED, and an agent that reworded its own modes or renamed its models From ecec1de75685aae560804ff068f9eb6d430a926d Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 03:50:57 +0000 Subject: [PATCH 4/7] fix(acp_client): build the next menu relearn on a newer stored measurement When a relearn stood down because the stored row carried a newer measurement stamp, the backend kept the older record and never re-read the store, so every later reply was refused the same way until the backend was rebuilt. Hot-apply after a Test and the boot backfill rebuild it in-process, but a failed hot-apply or another process sharing the snapshot file left it refusing for good. SnapshotStore.revise now returns the record it stood down for, together with whether it wrote, and still writes nothing in that case: the reply may predate that measurement, so its menus are not taken over it. The relearn hands that record back, the backend adopts it through its existing hook, and the next reply is judged against the stamp actually stored. The handed-back record is flagged stale when it was measured under another launch config than the caller's. --- raven/acp_client/acp_agent.py | 3 ++ raven/acp_client/capabilities.py | 46 +++++++++++++------ tests/test_subagent_acp.py | 79 ++++++++++++++++++++++++++++++-- 3 files changed, 109 insertions(+), 19 deletions(-) diff --git a/raven/acp_client/acp_agent.py b/raven/acp_client/acp_agent.py index b0f132ab0..a7884390a 100644 --- a/raven/acp_client/acp_agent.py +++ b/raven/acp_client/acp_agent.py @@ -1260,6 +1260,9 @@ def _note_session_model(self, session_id: str | None, result: Any) -> None: def _relearn_menus(self, result: Any) -> None: """Take the modes and models a session response advertises over the cached ones. + Or, when a newer measurement has been stored since this backend was + built, take that record instead, so the next response builds on it. + Best-effort at every step. The menus are what the NEXT dispatch and the row's picker read, never something this turn depends on, so neither the write nor the table rebuild may cost the turn that happened to carry the diff --git a/raven/acp_client/capabilities.py b/raven/acp_client/capabilities.py index 749826c47..12aeb5fef 100644 --- a/raven/acp_client/capabilities.py +++ b/raven/acp_client/capabilities.py @@ -396,31 +396,42 @@ def merge(current: str | None) -> tuple[str, None]: except OSError as exc: # a snapshot is a cache, never a dependency logger.warning("acp capability snapshot write failed (not remembered): {}", exc) - def revise(self, snapshot: CapabilitySnapshot, **changes: Any) -> CapabilitySnapshot | None: - """Apply ``changes`` to the stored record ``snapshot`` was read from, and return the result. + def revise(self, snapshot: CapabilitySnapshot, **changes: Any) -> tuple[CapabilitySnapshot, bool] | None: + """Apply ``changes`` to the stored record ``snapshot`` was read from. For a write that re-measured some fields of a record, so only those are written, onto the row as it stands inside the lock: another holder of the same record may have revised a different field since ``snapshot`` was read, and writing the caller's whole copy back would undo it. - ``None`` -- nothing written -- when the stored row carries another - measurement stamp than ``snapshot``. A Test or the boot backfill that - recorded in the meantime is newer evidence about everything, the same - "newest measurement wins" rule the boot backfill applies to its own - late verdicts. With no stored row, ``snapshot`` itself is the base. + Nothing is written when the stored row carries another measurement + stamp than ``snapshot``. A Test or the boot backfill that recorded in + the meantime is newer evidence about everything, the same "newest + measurement wins" rule the boot backfill applies to its own late + verdicts. With no stored row, ``snapshot`` itself is the base. + + Returns the record now stored and whether this call wrote it, so a + caller that stood down holds the newer record and builds its next + revision on that stamp rather than being refused again for as long as + it lives. Flagged stale when it was measured under another launch + config than ``snapshot``, or when ``snapshot`` already was: for the + caller's launch it proves no more than that. ``None`` when the store + could not be written, or holds only a row that cannot be read back. """ - def merge(current: str | None) -> tuple[str | None, CapabilitySnapshot | None]: + def merge(current: str | None) -> tuple[str | None, tuple[CapabilitySnapshot, bool] | None]: rows = self._parse(current) stored = next((r for r in rows if r.get("agent") == snapshot.agent), None) if stored is not None and int(stored.get("measuredAtMs") or 0) != snapshot.measured_at_ms: - return None, None + newer = CapabilitySnapshot.from_row(stored) + if newer is None: + return None, None + return None, (replace(newer, stale=snapshot.stale or newer.fingerprint != snapshot.fingerprint), False) base = (CapabilitySnapshot.from_row(stored) if stored is not None else None) or snapshot updated = replace(base, stale=snapshot.stale, **changes) rows = [r for r in rows if r.get("agent") != snapshot.agent] rows.append(updated.to_row()) - return self._dump(rows), updated + return self._dump(rows), (updated, True) try: return atomic_update(self._path, merge) @@ -729,7 +740,9 @@ def relearn_session_menus( clear a staleness flag the rest of the snapshot has not earned. Only the menus this response reported are written, and none when a newer measurement has replaced the record ``snapshot`` was read from - (``SnapshotStore.revise``). + (``SnapshotStore.revise``). That newer record is returned instead, unrevised: + this response may predate it, so its menus are not taken over it, but the + caller's next response is then judged against the stamp actually stored. ``None`` when there was nothing to learn, which includes a response carrying neither menu. Each menu is judged on its own, and an absent or empty one is @@ -751,13 +764,16 @@ def relearn_session_menus( changes["model_menu_measured"] = True if not changes: return None - updated = (store or SnapshotStore()).revise(snapshot, **changes) - if updated is None: + revised = (store or SnapshotStore()).revise(snapshot, **changes) + if revised is None: + return None + updated, written = revised + if not written: logger.info( - "acp agent {!r}: session menus not re-recorded; a newer measurement stands or the store is unwritable", + "acp agent {!r}: session menus not re-recorded over a newer measurement; building on that one next", snapshot.agent, ) - return None + return updated logger.info( "acp agent {!r}: re-recorded {} mode(s) and {} model(s) from a session response", snapshot.agent, diff --git a/tests/test_subagent_acp.py b/tests/test_subagent_acp.py index 6bbe4ac96..90ac5ff8e 100644 --- a/tests/test_subagent_acp.py +++ b/tests/test_subagent_acp.py @@ -1192,9 +1192,21 @@ def test_a_relearn_from_an_older_record_does_not_overwrite_a_newer_measurement(t a backend built from the record before it. That backend's relearn holds the older record whole, and writing it back would undo the newer verdict, fingerprint included -- so it stands down when the stamp it was built from - is no longer the one stored. The newer measurement brought its own menus.""" + is no longer the one stored. The newer measurement brought its own menus, + and what comes back is that record, unrevised, for the caller to build on.""" cfg = stub_config("a") store = SnapshotStore(path=tmp_path / "caps.json") + older, newer = _older_and_newer(cfg, store) + stored_bytes = (tmp_path / "caps.json").read_bytes() + + assert relearn_session_menus(older, _menu_reply(models=_GROUPED_MODELS), store=store) == newer + assert relearn_session_menus(older, _session_result({"id": "deep"}), store=store) == newer + assert store.load([cfg])["a"] == newer + assert (tmp_path / "caps.json").read_bytes() == stored_bytes, "standing down writes nothing at all" + + +def _older_and_newer(cfg: Any, store: SnapshotStore) -> tuple[CapabilitySnapshot, CapabilitySnapshot]: + """A record a backend was built from, and the newer measurement stored over it.""" older = _with_model_choices(cfg, AcpModelChoice(value="old/retired")) store.record(older) newer = replace( @@ -1205,10 +1217,69 @@ def test_a_relearn_from_an_older_record_does_not_overwrite_a_newer_measurement(t model_choices=(AcpModelChoice(value="test/fresh"),), ) store.record(newer) + return older, newer - assert relearn_session_menus(older, _menu_reply(models=_GROUPED_MODELS), store=store) is None - assert relearn_session_menus(older, _session_result({"id": "deep"}), store=store) is None - assert store.load([cfg])["a"] == newer + +def test_a_relearn_that_stood_down_builds_on_the_newer_record_next_time(tmp_path: Path) -> None: + """Standing down must not become standing down forever. + + The record handed back is the newer one, so the next reply is judged + against the stamp actually stored and lands -- onto that record, keeping + its verdict, and writing only the menu the reply reported. Without it, a + backend no rebuild reached (a failed hot-apply, another process sharing the + store) refused every reply until it was replaced. + """ + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + older, newer = _older_and_newer(cfg, store) + + base = relearn_session_menus(older, _menu_reply(models=_GROUPED_MODELS), store=store) + assert base == newer + again = relearn_session_menus(base, _menu_reply(models=_GROUPED_MODELS), store=store) + + assert again is not None and again.model_choices == _GROUPED_CHOICES + stored = store.load([cfg])["a"] + assert stored == replace(newer, model_choices=_GROUPED_CHOICES) + assert (stored.status, stored.detail, stored.measured_at_ms) == ("attention", newer.detail, newer.measured_at_ms) + + +def test_a_newer_record_from_another_launch_config_is_handed_back_stale(tmp_path: Path) -> None: + """The newer measurement may be of a config the caller was not launched + with -- a Test after an edit. Handed to a caller holding the older record, + it is not proof about that caller's launch, so it comes back flagged.""" + cfg = stub_config("a") + store = SnapshotStore(path=tmp_path / "caps.json") + older, newer = _older_and_newer(cfg, store) + edited = replace(newer, fingerprint="after-an-edit", measured_at_ms=newer.measured_at_ms + 1) + store.record(edited) + + handed = relearn_session_menus(older, _menu_reply(models=_GROUPED_MODELS), store=store) + + assert handed is not None and handed.stale is True + assert replace(handed, stale=False) == edited + assert store.load([cfg], allow_stale=True)["a"] == replace(edited, stale=True) + + +def test_a_backend_that_stood_down_refreshes_on_its_next_reply(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """End to end through the backend's own hook and the real store: the first + reply after a newer measurement is refused and the backend takes that + record on; the second refreshes the stored menu under the newer stamp.""" + path = tmp_path / "caps.json" + monkeypatch.setattr("raven.acp_client.capabilities.default_snapshot_path", lambda: path) + cfg = stub_config("a") + older, newer = _older_and_newer(cfg, SnapshotStore(path=path)) + backend = AcpAgentBackend(name="a", command="true", snapshot=older) + rebuilds: list[int] = [] + backend.bind_caps_listener(lambda: rebuilds.append(1)) + + backend._relearn_menus(_menu_reply(models=_GROUPED_MODELS)) + assert backend._snapshot == newer + assert SnapshotStore(path=path).load([cfg])["a"] == newer + + backend._relearn_menus(_menu_reply(models=_GROUPED_MODELS)) + assert SnapshotStore(path=path).load([cfg])["a"] == replace(newer, model_choices=_GROUPED_CHOICES) + assert backend._snapshot.model_choices == _GROUPED_CHOICES + assert rebuilds == [1, 1] def test_a_relearn_writes_only_the_menu_its_reply_reported(tmp_path: Path) -> None: From 52c105ed064579ef1de8c9c2bb8994faf19bfd1a Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 03:50:57 +0000 Subject: [PATCH 5/7] docs: name the menu relearn by its current name in the mode-tier spec The measured table keeps its line numbers, which are those of the revision it is pinned to, and notes the name that revision used. --- docs/specs/2026-09-03-subagent-mode-tiers-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/specs/2026-09-03-subagent-mode-tiers-design.md b/docs/specs/2026-09-03-subagent-mode-tiers-design.md index 3d1de8cf1..b2bf826ea 100644 --- a/docs/specs/2026-09-03-subagent-mode-tiers-design.md +++ b/docs/specs/2026-09-03-subagent-mode-tiers-design.md @@ -57,7 +57,7 @@ Against `origin/refactor/raven_v0_2_0` at `9bc06091`. | Step | Where | |---|---| -| A sub-agent's modes are read from its session response | `raven/acp_client/capabilities.py:428` `_read_session_modes`, re-recorded by `:451` `relearn_session_modes` | +| A sub-agent's modes are read from its session response | `raven/acp_client/capabilities.py:428` `_read_session_modes`, re-recorded by `:451` `relearn_session_menus` (named `relearn_session_modes` at that revision) | | They become the agent's advertised menu | `raven/agent/subagent/backends/__init__.py:135` -> `AgentMeta.modes`, read back by `raven/agent/subagent/manager.py:944` `agent_modes` | | The menu is offered to the dispatching model | `raven/agent/subagent/spawn_tool.py:195` - one enum over every agent's ids, per-agent text in the description. Withdrawn by this change | | One resolver serves every dispatch lane | `raven/agent/subagent/manager.py:958` `resolve_mode`: `requested` > the instance's standing override > `None`. `requested` is withdrawn too | From a224ddc2b7baf09e30cc51f4da165501d25a0f86 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 12:17:57 +0800 Subject: [PATCH 6/7] test(*): quote acp stub commands on windows Use the platform command quoting helper for Python and stub paths. Keep the RPC dispatch fixture at a path with spaces so unquoted script arguments fail on every test host, while the shared ACP fixture supports spaced checkouts. --- tests/test_rpc_subagents.py | 7 +++++-- tests/test_subagent_acp.py | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_rpc_subagents.py b/tests/test_rpc_subagents.py index 882157715..963763a0f 100644 --- a/tests/test_rpc_subagents.py +++ b/tests/test_rpc_subagents.py @@ -25,6 +25,7 @@ subagents_test, subagents_test_cancel, ) +from raven.utils.commands import command_quote @pytest.fixture @@ -3207,19 +3208,21 @@ async def test_list_shows_the_model_menu_a_dispatch_relearned( age, so before a dispatch could refresh it the card went on offering the ids the agent held at its last Test. One dispatch now carries the agent's current menu to the card, under the same ``agent`` rule as before. + A spaced stub path exercises launch quoting on every test host. """ from raven.acp_client.capabilities import AcpModelChoice, CapabilitySnapshot, SnapshotStore, snapshot_fingerprint from raven.acp_client.pool import close_pool from raven.agent.subagent.backends import build_third_party_backend from raven.config.schema import SubagentsConfig - stub = Path(__file__).with_name("acp_stub_server.py") + stub = tmp_path / "acp stub server.py" + stub.write_bytes(Path(__file__).with_name("acp_stub_server.py").read_bytes()) raw = json.loads(config_path.read_text()) raw["subagents"]["agents"].append( { "name": "Stubby", "kind": "acp", - "command": f"{sys.executable} {stub}", + "command": f"{command_quote(sys.executable)} {command_quote(str(stub))}", "env": {"ACP_STUB_MODE": "ok"}, "description": "d", "enabled": True, diff --git a/tests/test_subagent_acp.py b/tests/test_subagent_acp.py index 90ac5ff8e..d5d360a82 100644 --- a/tests/test_subagent_acp.py +++ b/tests/test_subagent_acp.py @@ -47,6 +47,7 @@ from raven.agent.subagent.registry import _row_for from raven.config.schema import SubagentsConfig, ThirdPartyAcpSubagentConfig, ThirdPartyCliSubagentConfig from raven.config.update_subagents import reject_unsupported_acp_fields +from raven.utils.commands import command_quote from tests._windows_dirs import scratch_root, windows_removal _STUB = Path(__file__).with_name("acp_stub_server.py") @@ -55,7 +56,7 @@ def stub_config(name: str = "stub", *, mode: str = "ok", **kw: Any) -> ThirdPartyAcpSubagentConfig: return ThirdPartyAcpSubagentConfig( name=name, - command=f"{sys.executable} {_STUB}", + command=f"{command_quote(sys.executable)} {command_quote(str(_STUB))}", env={"ACP_STUB_MODE": mode}, ready_timeout_ms=kw.pop("ready_timeout_ms", 15000), **kw, From 1759eed0445d03aedcda0f605c0f7983d040aec2 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 21 Oct 2026 05:29:54 +0000 Subject: [PATCH 7/7] test(acp_client): pin the two ways a menu revision writes nothing A newer stored row this build cannot read back is neither written over nor handed to the backend, and a snapshot file that cannot be written logs once and reports nothing learned. Both branches were reachable and right but unexercised, which left the PR's changed lines under the diff coverage threshold. --- tests/test_subagent_acp.py | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/test_subagent_acp.py b/tests/test_subagent_acp.py index d5d360a82..9ce5cc057 100644 --- a/tests/test_subagent_acp.py +++ b/tests/test_subagent_acp.py @@ -1283,6 +1283,56 @@ def test_a_backend_that_stood_down_refreshes_on_its_next_reply(tmp_path: Path, m assert rebuilds == [1, 1] +def test_a_newer_row_this_build_cannot_read_is_neither_overwritten_nor_adopted(tmp_path: Path) -> None: + """A newer measurement written by a Raven that knows a status this one does + not: its stamp still says it is newer, so nothing is written over it, and + it cannot be loaded, so there is no record to hand back either. The + backend keeps what it holds and rebuilds nothing.""" + cfg = stub_config("a") + path = tmp_path / "caps.json" + older = _with_model_choices(cfg, AcpModelChoice(value="old/retired")) + SnapshotStore(path=path).record(older) + unreadable = {**older.to_row(), "status": "a-status-from-a-later-release", "measuredAtMs": 2} + path.write_text(json.dumps({"version": 1, "snapshots": [unreadable]}), encoding="utf-8") + stored_bytes = path.read_bytes() + + assert relearn_session_menus(older, _menu_reply(models=_GROUPED_MODELS), store=SnapshotStore(path=path)) is None + assert path.read_bytes() == stored_bytes + + backend = AcpAgentBackend(name="a", command="true", snapshot=older) + rebuilds: list[int] = [] + backend.bind_caps_listener(lambda: rebuilds.append(1)) + with patch("raven.acp_client.capabilities.default_snapshot_path", lambda: path): + backend._relearn_menus(_menu_reply(models=_GROUPED_MODELS)) + assert backend._snapshot is older + assert rebuilds == [] + assert path.read_bytes() == stored_bytes + + +def test_a_snapshot_file_that_cannot_be_written_costs_only_the_refresh(tmp_path: Path) -> None: + """The store is a cache: a write that fails is said once in the log and + the relearn reports nothing learned, rather than raising into the turn or + handing the backend a record that was never stored.""" + cfg = stub_config("a") + blocker = tmp_path / "not-a-directory" + blocker.write_text("a file where the store's directory should be", encoding="utf-8") + store = SnapshotStore(path=blocker / "caps.json") + stored = _with_model_choices(cfg, AcpModelChoice(value="old/retired")) + + with _loguru_capture("WARNING") as logs: + assert relearn_session_menus(stored, _menu_reply(models=_GROUPED_MODELS), store=store) is None + assert any("acp capability snapshot write failed" in line for line in logs), logs + assert blocker.read_text(encoding="utf-8") == "a file where the store's directory should be" + + backend = AcpAgentBackend(name="a", command="true", snapshot=stored) + rebuilds: list[int] = [] + backend.bind_caps_listener(lambda: rebuilds.append(1)) + with patch("raven.acp_client.capabilities.default_snapshot_path", lambda: blocker / "caps.json"): + backend._relearn_menus(_menu_reply(models=_GROUPED_MODELS)) + assert backend._snapshot is stored + assert rebuilds == [] + + def test_a_relearn_writes_only_the_menu_its_reply_reported(tmp_path: Path) -> None: """Backends of one agent each hold the record they were built from, and a relearn on one does not reach the others' copies. A second backend whose