diff --git a/raven/agent/subagent/probe.py b/raven/agent/subagent/probe.py index 8f6923fbf..803bdd110 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, ) @@ -1290,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 @@ -1298,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: @@ -1326,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 @@ -1357,7 +1360,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 +1368,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_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_, 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)