From a4327c536b4e8624aa48a538f7b805aa17c81236 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Sun, 11 Oct 2026 16:21:15 +0000 Subject: [PATCH 1/3] fix: replace obsolete subagent authentication verdicts A new non-authentication failure could leave an old needs_auth flag persisted, and a stale snapshot could still disable the edited agent's connection control. Carry current failure flags when preserving prior capabilities, recheck previously failed rows on subsequent boots, and omit stale authentication verdicts from the listing RPC. --- raven/agent/subagent/probe.py | 28 +++++++---- raven/rpc/methods/subagents.py | 4 +- tests/test_rpc_subagents.py | 13 +++-- tests/test_subagent_probe.py | 86 ++++++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 13 deletions(-) diff --git a/raven/agent/subagent/probe.py b/raven/agent/subagent/probe.py index 8f6923fbf..789cb7ea7 100644 --- a/raven/agent/subagent/probe.py +++ b/raven/agent/subagent/probe.py @@ -1190,7 +1190,7 @@ def capabilities_wanted(cfg: Any) -> bool: def _test_record(snapshot: Any, previous: Any) -> Any: - """What a manual test writes down: its verdict always, its capabilities only + """What a new measurement writes down: its verdict always, its capabilities only when it reached them. A verify that fails before ``session/new`` -- a cold shim start, a machine @@ -1215,6 +1215,8 @@ def _test_record(snapshot: Any, previous: Any) -> Any: stale=False, status=snapshot.status, detail=snapshot.detail, + needs_auth=snapshot.needs_auth, + unfetched=snapshot.unfetched, measured_at_ms=snapshot.measured_at_ms, elapsed_ms=snapshot.elapsed_ms, ) @@ -1357,7 +1359,7 @@ async def _verify_missing_snapshots(manager: Any, rows: list[Any], *, configured name = getattr(row, "name", "") or "" outdated_menu = snapshot is not None and has_model_menu is not None and not has_model_menu(name) # Three reasons to re-measure besides staleness: a record written - # from before the model menu (above), a credential refusal, and one + # from before the model menu (above), a failed verification, and one # of raven's own that came back with an empty menu. Staleness asks # whether the launch config moved, and neither signing in nor # configuring a provider moves it -- so a recorded refusal never goes @@ -1365,16 +1367,24 @@ async def _verify_missing_snapshots(manager: Any, rows: list[Any], *, configured # across every restart after the sign-in that cured it. refused = getattr(snapshot, "needs_auth", False) menuless_own = _own_row_missing_its_menu(snapshot, name) - if snapshot is not None and not snapshot.stale and not outdated_menu and not refused and not menuless_own: + failed_before = snapshot is not None and snapshot.status != "ready" + if ( + snapshot is not None + and not snapshot.stale + and not outdated_menu + and not refused + and not menuless_own + and not failed_before + ): continue result = await verify_agent(cfg, **_launch_kw(cfg)) _note_menu_re_measured(result, name) - # A pass, or a refusal the agent explained. Every other failure stays - # unrecorded on purpose: a timeout or a crashed adapter is a fact - # about this minute, and a snapshot of one would label a working - # agent broken until somebody happened to press Test. - if result.status == "ready" or getattr(result, "needs_auth", False): - store.record(result) + # Replace an old failure with what this attempt actually measured. + # Failed rows are retried next boot, so a timeout is not a permanent + # verdict. A formerly working row still keeps its old capabilities. + if result.status == "ready" or getattr(result, "needs_auth", False) or failed_before: + record = _test_record(result, snapshot) if snapshot is not None and result.status != "ready" else result + store.record(record) recorded = True logger.info("acp agent {!r}: auto-verify {}", name, result.status) except Exception as exc: # noqa: BLE001 - a failed verify must not sink the rest diff --git a/raven/rpc/methods/subagents.py b/raven/rpc/methods/subagents.py index 8fff3383a..910c1ed65 100644 --- a/raven/rpc/methods/subagents.py +++ b/raven/rpc/methods/subagents.py @@ -360,7 +360,9 @@ async def _rows(*, probe: bool = True) -> list[dict]: # things. Always present, never omitted -- a client cannot tell # a missing key from a false one, and one day it will mean # "this server predates the field". - "needs_auth": bool(getattr(snapshot, "needs_auth", False)), + # Old capabilities remain useful after a config edit; an old + # authentication verdict must not disable the edited row. + "needs_auth": bool(getattr(snapshot, "needs_auth", False)) and not getattr(snapshot, "stale", False), "mcps": list(getattr(cfg, "mcps", None) or []), "allow_mcp_secrets": bool(getattr(cfg, "allow_mcp_secrets", False)), "last_test_ok": None if last is None else last.ok, diff --git a/tests/test_rpc_subagents.py b/tests/test_rpc_subagents.py index b24e18bde..e98d187b6 100644 --- a/tests/test_rpc_subagents.py +++ b/tests/test_rpc_subagents.py @@ -2740,8 +2740,9 @@ async def test_a_reserved_name_entry_is_reported_once_and_as_ignored(config_path assert rows[0]["enabled"] is True +@pytest.mark.parametrize("stale", [False, True]) async def test_a_row_carries_the_credential_verdict_its_snapshot_measured( - config_path: Path, monkeypatch: pytest.MonkeyPatch + config_path: Path, monkeypatch: pytest.MonkeyPatch, stale: bool ) -> None: """The agents page draws `Unauthorized` off this and nothing else. @@ -2758,14 +2759,20 @@ def snapshot_for(cfg: object) -> CapabilitySnapshot | None: if getattr(cfg, "name", "") != "Codex": return None return CapabilitySnapshot( - agent="Codex", fingerprint="f", status="attention", detail="no session", measured_at_ms=0, needs_auth=True + agent="Codex", + fingerprint="f", + status="attention", + detail="no session", + measured_at_ms=0, + needs_auth=True, + stale=stale, ) monkeypatch.setattr(mod, "acp_snapshot_for", snapshot_for, raising=False) monkeypatch.setattr("raven.agent.subagent.probe.acp_snapshot_for", snapshot_for) rows = {row["name"]: row for row in (await subagents_list({"probe": False}))["rows"]} - assert rows["Codex"]["needs_auth"] is True + assert rows["Codex"]["needs_auth"] is not stale # Every other row says so too, rather than leaving the key out: a missing key # and a false one read the same to a client, right up until one of them means # "this server is too old to know". A cli row and an openai row have no diff --git a/tests/test_subagent_probe.py b/tests/test_subagent_probe.py index 494747b9a..023675084 100644 --- a/tests/test_subagent_probe.py +++ b/tests/test_subagent_probe.py @@ -1661,3 +1661,89 @@ def _fake_executable(tmp_path: Path, name: str) -> Path: exe.write_text("#!/bin/sh\nexit 0\n") exe.chmod(0o755) return exe + + +@pytest.mark.parametrize( + "status,detail", + [ + ("unknown", "unsupported session configuration"), + ("attention", "session/new timed out"), + ("missing", "transport disconnected"), + ], +) +async def test_boot_replaces_an_auth_verdict_and_retries_until_ready(tmp_path, monkeypatch, status, detail): + from dataclasses import replace + from unittest.mock import AsyncMock + + import raven.acp_client.capabilities as caps + + cfg = ThirdPartyAcpSubagentConfig(name="measured", command="measured --acp") + monkeypatch.setattr(caps, "default_snapshot_path", lambda: tmp_path / "caps.json") + monkeypatch.setattr(probe_mod, "_launch_kw", lambda cfg: {}) + old = caps.CapabilitySnapshot( + agent=cfg.name, + fingerprint=caps.snapshot_fingerprint(cfg), + status="attention", + detail="sign in", + measured_at_ms=1, + needs_auth=True, + can_load=True, + model_choices=(caps.AcpModelChoice(value="a", name="A"),), + ) + store = caps.SnapshotStore() + store.record(old) + failed = caps.CapabilitySnapshot( + agent=cfg.name, + fingerprint=old.fingerprint, + status=status, + detail=detail, + measured_at_ms=2, + needs_auth=False, + ) + verified = AsyncMock(side_effect=[failed, replace(old, status="ready", needs_auth=False, measured_at_ms=3)]) + monkeypatch.setattr(caps, "verify_agent", verified) + rows = [_FakeRow(cfg.name, config=cfg)] + manager = _FakeManager(rows, with_refresh=True) + await probe_mod._verify_missing_snapshots(manager, rows) + saved = store.load([cfg])[cfg.name] + assert not saved.needs_auth + assert not saved.usable + assert (saved.status, saved.detail) == (status, detail) + assert saved.can_load and saved.model_choices == old.model_choices + await probe_mod._verify_missing_snapshots(manager, rows) + assert store.load([cfg])[cfg.name].usable + await probe_mod._verify_missing_snapshots(manager, rows) + assert verified.await_count == 2 + assert manager.refresh_calls == 2 + + +@pytest.mark.parametrize("needs_auth,unfetched", [(True, False), (False, True), (False, False)]) +@pytest.mark.parametrize("previous_status", ["ready", "attention"]) +def test_manual_test_merges_current_failure_flags_with_old_capabilities(needs_auth, unfetched, previous_status): + from dataclasses import replace + + from raven.acp_client.capabilities import CapabilitySnapshot + + old = CapabilitySnapshot( + agent="agent", + fingerprint="old", + status=previous_status, + detail="old refusal", + measured_at_ms=1, + needs_auth=previous_status == "attention", + can_load=True, + ) + measured = replace( + old, + status="attention", + fingerprint="new", + measured_at_ms=2, + detail="current refusal", + can_load=False, + needs_auth=needs_auth, + unfetched=unfetched, + ) + saved = probe_mod._test_record(measured, old) + assert saved.can_load + assert saved.fingerprint == "new" + assert (saved.needs_auth, saved.unfetched) == (needs_auth, unfetched) From 64fce10f28d8f7ab72bf01b29712f44badf97551 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Sun, 11 Oct 2026 17:15:15 +0000 Subject: [PATCH 2/3] test(agents): isolate research tool DNS resolution Use deterministic DNS beside the mocked HTTP transport while retaining the real URL validation. Explicit resolver-failure tests keep their own overrides, and the strict idle ceiling remains enabled. --- tests/test_agents_research_tools.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_agents_research_tools.py b/tests/test_agents_research_tools.py index e72c2062b..33a45682a 100644 --- a/tests/test_agents_research_tools.py +++ b/tests/test_agents_research_tools.py @@ -51,6 +51,19 @@ _NO_LLM_GATES = {"verify": {"enabled": False}, "forceFinalize": {"enabled": False}} +@pytest.fixture(autouse=True) +def _public_dns(monkeypatch): + """Keep the real URL checks, with deterministic DNS beside the stubbed HTTP. + + Cases about resolution failure override this function-scoped fixture. + """ + + def resolve(host, port, *args, **kwargs): + return [(socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP, "", ("8.8.8.8", port or 0))] + + monkeypatch.setattr(socket, "getaddrinfo", resolve) + + def _ctx(tmp_path: Path, slice_: dict, *, provider: object | None = None) -> PluginContext: return PluginContext( config=slice_, From f75ce7c81871b5e29f7e70e2875f974a55bf3115 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Mon, 12 Oct 2026 11:38:15 +0000 Subject: [PATCH 3/3] docs(agent): say failed snapshots are verified again at every start The two docstrings still described verifying only missing or stale rows. A row whose last verification failed is measured again at every start, so a timeout is not a permanent verdict. --- raven/agent/subagent/probe.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/raven/agent/subagent/probe.py b/raven/agent/subagent/probe.py index 789cb7ea7..803bdd110 100644 --- a/raven/agent/subagent/probe.py +++ b/raven/agent/subagent/probe.py @@ -1292,7 +1292,7 @@ def _unconfigured_acp_preset_rows(configured: set[str], *, path: str | None) -> def schedule_snapshot_verification(manager: Any) -> asyncio.Task | None: - """Verify every enabled acp agent that has no fresh capability snapshot. + """Verify every enabled acp agent whose capability snapshot is missing, stale or a failure. Before this, the snapshot -- the record an acp row's ``stateful`` and ``can_resume`` are read from -- was written only by the UI Test button and a @@ -1300,8 +1300,9 @@ def schedule_snapshot_verification(manager: Any) -> asyncio.Task | None: the ``/new-instance`` picker hid those agents until someone opened a page and clicked. Backgrounded so a slow adapter (a first ``npx`` fetch) cannot delay startup; fire-and-forget, because a failure leaves the row stateless - with the snapshot's own detail rather than breaking the boot. Verification - costs no model tokens (handshake plus a throwaway ``session/new``). + with the snapshot's own detail rather than breaking the boot, and is + measured again at the next start. Verification costs no model tokens + (handshake plus a throwaway ``session/new``). """ global _SCHEDULED if _SCHEDULED: @@ -1328,7 +1329,7 @@ def schedule_snapshot_verification(manager: Any) -> asyncio.Task | None: async def _verify_missing_snapshots(manager: Any, rows: list[Any], *, configured: set[str] | None = None) -> None: - """One verification per missing or stale row, sequentially, never raising. + """One verification per row whose snapshot is missing, stale or a failure, sequentially, never raising. Sequential, not concurrent: every acp verify spawns a child process, and a machine with a slow adapter among several would otherwise launch them all at