fix: replace obsolete subagent authentication verdicts

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,16 +1292,17 @@ 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
     ``probe: true`` listing, so a fresh install reported zero stateful rows and
     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,24 +1360,32 @@ 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
             # stale, and the row it came from would go on saying "Unauthorized"
             # 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

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,

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_,

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

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)