Models that know about each other, and have a self

Three features sharing one idea: a model here started from nothing every
conversation and had no notion that anything else existed.

THE ROSTER. `chat.roster_block` builds one line per model this *person* can
reach -- through `permissions.models_visible_to`, never the table -- and
`{{model_roster}}` carries it, gated on the `friend` family for the reason the
memories block is gated on `memory`: a list of peers a model cannot talk to is
context spent on nothing, and one checkbox is then the whole switch. New
`Model.notes` column, a column and not a `capabilities_json` key for the reason
`context_length` and `reasoning_efforts` both carry.

ASKING A FRIEND. A second entry point in `services/subagent.py` rather than a
second module, so one place still owns the bounds and the lifecycle. `_create_
child` takes the friend's (model_id, connection_id) *pair*, because Model is
unique on both and an id alone does not say which endpoint. Three things differ
from a helper: the effort is the friend's own default and never the parent's (the
1.3.0 bug by another door -- the vocabularies differ and a level a model does not
take raises inside its chat template), the chat is ordinary even when the asker's
is an agent chat, and `scope_json["role"]` marks it so `core.friend` speaks
instead of `core.subagent`. `friend` joins the unattended withdrawal set: a
friend that could ask a friend is the same unbounded fan-out in politer clothes.
Budget, concurrency and quota are shared with helpers, so one reply cannot spend
the allowance twice.

PERSONALITY. One table, two roles, `owner_id IS NULL` the discriminator: the
model's own persona, and its read of one person. Keyed on the model's *text* id
with no foreign key, because "Test & refresh" deletes a model the endpoint has
stopped listing and a personality must not be collateral. `PersonaRevision`
copies SkillRevision, and so does the argument: the safety story for a model
rewriting itself is a record and a way back, not a gate. The reflection is shown
to the person it is about, in their own settings, which is the whole of why
keeping one is acceptable. `persona` is withdrawn from any unattended chat --
a helper's task, a friend's question and a schedule's instruction are all words
nobody watched being written.

Two bugs found while reading for this, both silent:

`review_model_id` stored a `Model` primary key, so a refresh taken while an
endpoint was not listing that model unset the administrator's choice -- and
`_reviewer` then fell back to the chat's own model, so pictures were judged by
a model nobody chose. Now the text id, with the primary key still accepted.

`_messages_after` used a bare `>` on `created_at`, so a row sharing the edited
turn's microsecond survived a rewind -- and `_send` writes a user turn and its
placeholder back to back, which is exactly that tie. Deliberately NOT
`thread_tail`'s `(created_at, id)` tiebreak: ids are random UUIDs, so that
settles a tie by coin toss. A tie now reads as "later", which is the safe
direction for an operation whose purpose is to discard what follows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tests/test_agent_policy.py

@@ -270,6 +270,14 @@ def test_the_builtins_that_change_things_say_so():
         # class as a note. Plan mode meaning "look but do not touch" has to mean
         # this too, even though what it touches is a page rather than a machine.
         "report_write",
+        # Its own character and its own read of the person. Writes for the same
+        # reason `report_write` is one, and more strongly: these outlive the
+        # conversation, are carried into every later one, and change how it
+        # behaves rather than only what is recorded. Being in this set is also
+        # what makes `scope_json["write"] = False` withdraw them, which is how a
+        # read-only helper is kept from rewriting who it is.
+        "persona_write",
+        "impression_write",
     }
 
 

tests/test_chat.py

@@ -652,6 +652,44 @@ def test_editing_rewinds_and_discards_later_messages(
     assert remaining[1].complete is False
 
 
+def test_a_rewind_takes_a_message_written_in_the_same_microsecond(
+    client: TestClient, db, registered, make_chat
+):
+    """`_messages_after` compared timestamps with a bare `>`, so a row sharing the
+    edited turn's microsecond was never "after" it and survived the rewind -- an
+    orphan below the message being edited, in the transcript and in every later
+    request. `_send` writes a user turn and its assistant placeholder back to
+    back, so that pair is precisely what ties.
+
+    Not fixed with `thread_tail`'s `(created_at, id)` tiebreak: `Message.id` is a
+    random UUID, so that would settle a tie by coin toss. A tie is read as
+    "later" instead, which is the safe direction for an operation whose purpose
+    is to discard what follows.
+    """
+    _add_connection(db)
+    chat_id = make_chat()
+    _exchange(client, db, chat_id, "first")
+    _exchange(client, db, chat_id, "second")
+
+    rows = db.scalars(select(Message).order_by(Message.created_at)).all()
+    edited = rows[0]
+    # Every later row now shares the edited turn's timestamp exactly.
+    for row in rows[1:]:
+        row.created_at = edited.created_at
+    db.commit()
+
+    client.post(
+        f"/api/chats/{chat_id}/messages/{edited.id}/edit", data={"content": "first, revised"}
+    )
+
+    db.expire_all()
+    remaining = db.scalars(select(Message).order_by(Message.created_at, Message.id)).all()
+    assert [m.role for m in remaining] == ["user", "assistant"], (
+        "a message sharing the edited turn's microsecond survived the rewind"
+    )
+    assert remaining[0].content == "first, revised"
+
+
 def test_the_edit_form_says_how_much_will_be_lost(client: TestClient, db, registered, make_chat):
     _add_connection(db)
     chat_id = make_chat()

tests/test_friend.py

@@ -0,0 +1,382 @@
+"""Putting a question to one of the other models, and getting its answer back.
+
+Shares its machinery with `subagent_run` on purpose, so most of what is asserted
+here is the *differences* — which model answers, with whose reasoning effort, in
+what kind of chat, and what it may not do in turn. The generation loop is stubbed
+exactly as `test_subagent.py` stubs it; what matters is the chat the friend is
+given.
+"""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+from sqlalchemy import select
+
+from lembas.db.models import (
+    KIND_AGENT,
+    KIND_CHAT,
+    ROLE_USER,
+    Chat,
+    Connection,
+    Group,
+    Model,
+    User,
+)
+from lembas.services import chat as chat_service
+from lembas.services import settings_store
+from lembas.services import subagent as subagent_service
+from lembas.services import tools as tools_service
+from lembas.services.crypto import encrypt
+
+
+@pytest.fixture(autouse=True)
+def asking_allowed(db, registered):
+    """The instance switch on, the permission granted, and three models to ask.
+
+    The gates get their own test below, which asserts both directions.
+    """
+    settings_store.update(db, {"enabled": True}, key=settings_store.SUBAGENTS)
+    settings_store.update(db, {"default_permissions": {"tools.friend": True}})
+    connection = Connection(
+        name="Test", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("")
+    )
+    db.add(connection)
+    db.commit()
+    for index, (name, label, note) in enumerate(
+        [
+            ("test-model", "The asker", ""),
+            ("big-model", "Big", "70B, good at maths"),
+            ("small-model", "Small", ""),
+        ]
+    ):
+        db.add(
+            Model(
+                connection_id=connection.id,
+                model_id=name,
+                display_name=label,
+                notes=note,
+                position=index,
+                capabilities_json={"tools": True},
+            )
+        )
+    db.commit()
+    subagent_service.clear()
+    yield
+    subagent_service.clear()
+
+
+def _user(db) -> User:
+    return db.scalars(select(User).order_by(User.created_at)).first()
+
+
+def _chat(db, **kwargs) -> Chat:
+    chat = Chat(user_id=_user(db).id, title="t", model_id="test-model", **kwargs)
+    db.add(chat)
+    db.commit()
+    return chat
+
+
+class _Fake:
+    def __init__(self, spawned: int = 0):
+        self.subagents = spawned
+
+
+def _spawn(monkeypatch, *, answer: str = "I disagree, and here is why.", finish: bool = True):
+    from lembas.db.models import ROLE_ASSISTANT, ROLE_USER
+    from lembas.db.session import session_scope
+
+    seen: dict[str, str] = {}
+
+    async def fake_wake(chat_id: str, content: str, *, model_id: str = "") -> str:
+        seen["chat_id"] = chat_id
+        seen["turn"] = content
+        with session_scope() as db:
+            child = db.get(Chat, chat_id)
+            chat_service.create_message(db, child, ROLE_USER, content)
+            reply = chat_service.create_message(db, child, ROLE_ASSISTANT, answer)
+            seen["message_id"] = reply.id
+        return seen["message_id"]
+
+    monkeypatch.setattr("lembas.services.wake.wake_chat", fake_wake)
+    monkeypatch.setattr("lembas.services.generation.running_for", lambda chat_id: None)
+    return seen
+
+
+async def _ask(db, chat: Chat, args: dict, *, generation=None):
+    """Through `resolve_tools`, never by hand — what may be run is what was
+    offered, and a hand-built context falls back to the import-time registry,
+    which has never held this tool."""
+    from lembas.services import generation as generation_service
+
+    user = _user(db)
+    resolved = tools_service.resolve_tools(db, chat, user)
+    context = tools_service.context_for(db, user, chat, tools=resolved)
+    fake = generation if generation is not None else _Fake()
+    original = generation_service.running_for
+
+    def running_for(chat_id):
+        return fake if chat_id == chat.id else original(chat_id)
+
+    generation_service.running_for = running_for
+    try:
+        return await tools_service.run_tool(context, "ask_friend", json.dumps(args))
+    finally:
+        generation_service.running_for = original
+
+
+# --- Whose chat it is ---------------------------------------------------------
+async def test_the_friend_answers_as_itself_not_as_the_asking_model(db, monkeypatch):
+    """The whole feature. `generation` resolves the endpoint from the child chat
+    row, so the model on that row is the one that answers."""
+    parent = _chat(db)
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+
+    await _ask(db, parent, {"model": "big-model", "question": "Is this right?"})
+
+    child = db.get(Chat, seen["chat_id"])
+    assert child.model_id == "big-model"
+    assert child.parent_chat_id == parent.id
+    assert child.unattended is True
+    assert child.temporary is True
+
+
+async def test_the_friend_can_be_named_by_its_label_as_well_as_its_id(db, monkeypatch):
+    """The roster prints both, so a model will sometimes type back the pretty
+    one. Refusing that is a round spent on a spelling."""
+    parent = _chat(db)
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+
+    await _ask(db, parent, {"model": "Big", "question": "Is this right?"})
+
+    assert db.get(Chat, seen["chat_id"]).model_id == "big-model"
+
+
+async def test_the_friend_does_not_inherit_the_askers_reasoning_effort(db, monkeypatch):
+    """The 1.3.0 bug with a new door: the vocabularies differ per model, and an
+    effort a model does not take is rendered into its chat template and raises
+    there. `high` from the asker must not follow the question to a model whose
+    list says low/medium/xhigh."""
+    parent = _chat(db)
+    parent.params_json = {"reasoning_effort": "high"}
+    friend = db.scalar(select(Model).where(Model.model_id == "big-model"))
+    friend.reasoning_efforts = ["low", "medium", "xhigh"]
+    friend.params_json = {"reasoning_effort": "xhigh"}
+    db.commit()
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+
+    await _ask(db, parent, {"model": "big-model", "question": "Is this right?"})
+
+    child = db.get(Chat, seen["chat_id"])
+    assert chat_service.resolved_effort(child) == "xhigh"
+
+
+async def test_an_effort_the_friend_does_not_take_is_not_sent_at_all(db, monkeypatch):
+    parent = _chat(db)
+    friend = db.scalar(select(Model).where(Model.model_id == "big-model"))
+    friend.reasoning_efforts = ["low", "medium"]
+    friend.params_json = {"reasoning_effort": "high"}
+    db.commit()
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+
+    await _ask(db, parent, {"model": "big-model", "question": "?"})
+
+    assert chat_service.resolved_effort(db.get(Chat, seen["chat_id"])) == ""
+
+
+async def test_a_friend_of_an_agent_chat_is_not_given_the_machine(db, monkeypatch):
+    """A peer is asked what it thinks, not put to work. An agent chat's harness
+    is about the box it is working on, and handing that to somebody asked a
+    question invites it to plan around a shell it has not got."""
+    parent = _chat(db, kind=KIND_AGENT, project_dir="/srv/app", ssh_profile_id="nope")
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+
+    await _ask(db, parent, {"model": "big-model", "question": "?"})
+
+    child = db.get(Chat, seen["chat_id"])
+    assert child.kind == KIND_CHAT
+    assert not child.ssh_profile_id
+    assert not child.project_dir
+    # And the consequence, which is the thing that actually matters: an
+    # ordinary chat resolves no agent tools, whatever the mode column says.
+    offered = tools_service.resolve_tools(db, child, _user(db))
+    assert not [name for name in offered.by_name if name.startswith(("shell_", "file_"))]
+
+
+# --- What it may not do -------------------------------------------------------
+async def test_a_friend_cannot_ask_a_friend(db, monkeypatch):
+    """Otherwise one question is a fan-out with no bound anybody set. Both halves:
+    the family is withdrawn from the offered set, and the runner refuses a call
+    that arrived by any other route."""
+    parent = _chat(db)
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+    await _ask(db, parent, {"model": "big-model", "question": "?"})
+    child = db.get(Chat, seen["chat_id"])
+
+    offered = tools_service.resolve_tools(db, child, _user(db))
+    assert "ask_friend" not in offered.by_name
+    assert "subagent_run" not in offered.by_name
+    assert "ask_user" not in offered.by_name
+
+    # And the runner's own guard, reached by offering it the tool anyway --
+    # which is what "a call that arrived by some other route" means. Two halves,
+    # because the withdrawal is the one a prompt cannot argue with and this is
+    # the one that holds if the withdrawal is ever got round.
+    forced = tools_service.ToolSet(tuple(subagent_service.friend_tool_defs()))
+    context = tools_service.context_for(db, _user(db), child, tools=forced)
+    outcome = await tools_service.run_tool(
+        context, "ask_friend", json.dumps({"model": "small-model", "question": "?"})
+    )
+    assert "may not pass it on" in outcome.content
+
+
+async def test_a_friend_cannot_rewrite_its_own_personality(db, monkeypatch):
+    """A question is written by a model that may have been reading a page, and
+    the persona is carried into every conversation it will ever have."""
+    settings_store.update(db, {"default_permissions": {"tools.persona": True}})
+    parent = _chat(db)
+    seen = _spawn(monkeypatch)
+    settings_store.update(db, {"keep_transcript": True}, key=settings_store.SUBAGENTS)
+    await _ask(db, parent, {"model": "big-model", "question": "?"})
+
+    offered = tools_service.resolve_tools(db, db.get(Chat, seen["chat_id"]), _user(db))
+    assert "persona_write" not in offered.by_name
+    assert "impression_write" not in offered.by_name
+    # And it is genuinely on for the chat somebody is present in.
+    assert "persona_write" in tools_service.resolve_tools(db, parent, _user(db)).by_name
+
+
+# --- Refusals that name what could have been asked ----------------------------
+async def test_an_unknown_model_is_refused_with_the_list_of_real_ones(db, monkeypatch):
+    """The name arrives in a tool call, so it is model-written input. A refusal
+    that does not say what the valid answers are costs another round."""
+    parent = _chat(db)
+    _spawn(monkeypatch)
+
+    outcome = await _ask(db, parent, {"model": "gpt-9", "question": "?"})
+
+    assert outcome.event["status"] == "error"
+    assert "big-model" in outcome.content
+    assert "small-model" in outcome.content
+
+
+async def test_asking_itself_is_refused_in_those_words(db, monkeypatch):
+    parent = _chat(db)
+    _spawn(monkeypatch)
+    outcome = await _ask(db, parent, {"model": "test-model", "question": "?"})
+    assert "That is you" in outcome.content
+
+
+async def test_a_model_the_reader_cannot_use_is_neither_listed_nor_reachable(db, monkeypatch):
+    """A roster is filtered through what this account can see, so naming a
+    restricted model must fail for the same reason it is absent — not by a
+    second, looser check."""
+    group = Group(name="Wheel")
+    db.add(group)
+    restricted = db.scalar(select(Model).where(Model.model_id == "big-model"))
+    restricted.public = False
+    restricted.groups = [group]
+    db.commit()
+    user = _user(db)
+    user.role = ROLE_USER
+    db.commit()
+    parent = _chat(db)
+    _spawn(monkeypatch)
+
+    assert "big-model" not in chat_service.roster_block(db, user, exclude="test-model")
+    outcome = await _ask(db, parent, {"model": "big-model", "question": "?"})
+    assert outcome.event["status"] == "error"
+    assert "no model called" in outcome.content
+
+
+async def test_an_empty_question_is_refused_before_anything_is_created(db, monkeypatch):
+    parent = _chat(db)
+    seen = _spawn(monkeypatch)
+    outcome = await _ask(db, parent, {"model": "big-model", "question": "   "})
+    assert outcome.event["status"] == "error"
+    assert "chat_id" not in seen, "a chat was created for a call that could not work"
+
+
+# --- The budget ---------------------------------------------------------------
+async def test_questions_and_helpers_share_one_allowance(db, monkeypatch):
+    """Two counters would let one reply spend both. `Generation.subagents` is the
+    only object that knows what "this reply" means."""
+    parent = _chat(db)
+    _spawn(monkeypatch)
+    settings_store.update(db, {"max_per_reply": 1}, key=settings_store.SUBAGENTS)
+
+    generation = _Fake(spawned=1)
+    outcome = await _ask(
+        db, parent, {"model": "big-model", "question": "?"}, generation=generation
+    )
+
+    assert outcome.event["status"] == "error"
+    assert "already used its 1 helpers" in outcome.content
+
+
+async def test_a_successful_question_spends_one_of_the_allowance(db, monkeypatch):
+    parent = _chat(db)
+    _spawn(monkeypatch)
+    generation = _Fake()
+    await _ask(db, parent, {"model": "big-model", "question": "?"}, generation=generation)
+    assert generation.subagents == 1
+
+
+# --- The gates ----------------------------------------------------------------
+def test_the_tool_needs_the_permission_and_the_instance_switch(db):
+    parent = _chat(db)
+    user = _user(db)
+    assert "ask_friend" in tools_service.resolve_tools(db, parent, user).by_name
+
+    settings_store.update(db, {"enabled": False}, key=settings_store.SUBAGENTS)
+    assert "ask_friend" not in tools_service.resolve_tools(db, parent, user).by_name
+
+    settings_store.update(db, {"enabled": True}, key=settings_store.SUBAGENTS)
+    # An administrator bypasses every permission, so the permission half can
+    # only be asserted on somebody who is not one.
+    user.role = ROLE_USER
+    settings_store.update(db, {"default_permissions": {"tools.friend": False}})
+    db.commit()
+    assert "ask_friend" not in tools_service.resolve_tools(db, parent, user).by_name
+
+
+def test_the_model_switch_turns_it_off_for_that_model_alone(db):
+    parent = _chat(db)
+    asker = db.scalar(select(Model).where(Model.model_id == "test-model"))
+    asker.capabilities_json = {"tools": True, "tool_friend": False}
+    db.commit()
+    assert "ask_friend" not in tools_service.resolve_tools(db, parent, _user(db)).by_name
+
+
+# --- The answer ---------------------------------------------------------------
+async def test_the_answer_comes_back_named_and_marked_as_an_opinion(db, monkeypatch):
+    """A model handing on another's answer as its own is the failure worth
+    wording against, so the tool result says whose it is."""
+    parent = _chat(db)
+    _spawn(monkeypatch, answer="No. The second premise is wrong.")
+
+    outcome = await _ask(db, parent, {"model": "big-model", "question": "Is this right?"})
+
+    assert outcome.event["status"] == "ok"
+    assert "Big answered" in outcome.content
+    assert "The second premise is wrong." in outcome.content
+    assert "opinion" in outcome.content
+    assert outcome.event["why"] == "Big"
+
+
+async def test_the_question_says_who_is_asking_and_that_nobody_is_reading(db, monkeypatch):
+    parent = _chat(db)
+    seen = _spawn(monkeypatch)
+    await _ask(db, parent, {"model": "big-model", "question": "Is this right?", "context": "ctx"})
+
+    assert "test-model" in seen["turn"]
+    assert "Nobody is reading" in seen["turn"]
+    assert "Is this right?" in seen["turn"]
+    assert "ctx" in seen["turn"]

tests/test_images_chat.py

@@ -295,3 +295,111 @@ def test_a_queued_turn_is_not_lost_when_it_was_forced(db, user_id, vision_chat):
 
     assert chats_api._reply_in_flight(db, vision_chat) is True
     assert db.scalar(select(Attachment)) is None
+
+
+# --- Which model reviews what was drawn ---------------------------------------
+#
+# The reviewer is named in the instance settings, and it used to be named by the
+# `Model` row's primary key. "Test & refresh" on the connection screen deletes
+# any model the endpoint has stopped listing and recreates it when it comes back
+# with a new primary key -- so one refresh taken while an endpoint happened to be
+# loading something else silently unset the administrator's choice. It did not
+# fail: `_reviewer` falls back to the chat's own model, so the picture was
+# reviewed by a different model than the one chosen, with nothing saying so.
+def _reviewer_of(db, chat, settings: dict):
+    from lembas.db.models import User
+    from lembas.services import tools as tools_service
+    from lembas.services.images import tool as image_tool
+
+    user = db.get(User, chat.user_id)
+    context = tools_service.context_for(db, user, chat, tools=tools_service.ToolSet())
+    context.image_config = settings
+    return image_tool._reviewer(context)
+
+
+def test_the_reviewer_is_named_by_the_models_own_id(db, vision_chat):
+    db.add(
+        Model(
+            connection_id=vision_chat.connection_id,
+            model_id="reviewer",
+            capabilities_json={"vision": True},
+        )
+    )
+    db.commit()
+
+    resolved = _reviewer_of(
+        db, vision_chat, {"review_enabled": True, "review_model_id": "reviewer"}
+    )
+
+    assert resolved is not None
+    assert resolved[1] == "reviewer"
+
+
+def test_the_reviewer_survives_its_row_being_deleted_and_remade(db, vision_chat):
+    """The refresh case, end to end: the row goes, an identical one arrives with
+    a different primary key, and the choice still resolves."""
+    db.add(
+        Model(
+            connection_id=vision_chat.connection_id,
+            model_id="reviewer",
+            capabilities_json={"vision": True},
+        )
+    )
+    db.commit()
+    settings = {"review_enabled": True, "review_model_id": "reviewer"}
+    assert _reviewer_of(db, vision_chat, settings)[1] == "reviewer"
+
+    row = db.scalar(select(Model).where(Model.model_id == "reviewer"))
+    connection_id = row.connection_id
+    db.delete(row)
+    db.commit()
+    db.add(
+        Model(
+            connection_id=connection_id,
+            model_id="reviewer",
+            capabilities_json={"vision": True},
+        )
+    )
+    db.commit()
+
+    assert _reviewer_of(db, vision_chat, settings)[1] == "reviewer"
+
+
+def test_a_primary_key_stored_by_an_older_release_still_resolves(db, vision_chat):
+    """The value written before the id was the rule is a primary key, and an
+    instance that never touches the setting again must keep working."""
+    db.add(
+        Model(
+            connection_id=vision_chat.connection_id,
+            model_id="reviewer",
+            capabilities_json={"vision": True},
+        )
+    )
+    db.commit()
+    row = db.scalar(select(Model).where(Model.model_id == "reviewer"))
+
+    resolved = _reviewer_of(
+        db, vision_chat, {"review_enabled": True, "review_model_id": row.id}
+    )
+
+    assert resolved is not None
+    assert resolved[1] == "reviewer"
+
+
+def test_the_admin_page_offers_the_models_own_id_as_the_value(client, db, vision_chat):
+    """The other half. Storing the primary key is what created the problem, so
+    the form must not put one back."""
+    db.add(
+        Model(
+            connection_id=vision_chat.connection_id,
+            model_id="reviewer",
+            display_name="Reviewer",
+            capabilities_json={"vision": True},
+        )
+    )
+    db.commit()
+
+    page = client.get("/admin/images").text
+    row = db.scalar(select(Model).where(Model.model_id == "reviewer"))
+    assert 'value="reviewer"' in page
+    assert f'value="{row.id}"' not in page

tests/test_migrations.py

@@ -29,7 +29,7 @@ from lembas.db.migrations import ensure_fts, sync_schema
 from lembas.db.session import get_engine
 
 # Tables that did not exist at 0.8.1. `sync_schema` has to create them.
-OLD_TABLES = ("chunks", "push_subscriptions", "usage")
+OLD_TABLES = ("chunks", "push_subscriptions", "usage", "personas", "persona_revisions")
 
 # Columns added to tables that already existed, and therefore already had rows.
 # These are the interesting half: a new *table* is empty by definition, but a
@@ -40,6 +40,12 @@ OLD_COLUMNS = (
     ("chats", "unattended"),
     ("reports", "unread_notified"),
     ("groups", "limits_json"),
+    # What the other models are told about this one. A Text column with a scalar
+    # default, so the backfill is the easy kind -- listed because the hard kind
+    # (`reasoning_efforts`, below) was not caught by anything until it broke a
+    # live instance, and a column absent from this list is a column the migration
+    # tests do not exercise.
+    ("models", "notes"),
 )
 
 

tests/test_personality.py

@@ -0,0 +1,450 @@
+"""A model's own character, and what it makes of the person in front of it.
+
+Two things in one table, and the discriminator is a nullable column — so the
+assertions that matter most are about the boundary between them: an instance-wide
+persona must not be reachable as somebody's reflection, and one account's
+reflection must never be visible or deletable by another. A model-written note
+about a person that the person cannot read is the thing this must not become.
+
+The safety story for self-modification is a record and a way back rather than a
+gate, which is `SkillRevision`'s argument; the revision tests are where that is
+pinned.
+"""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+from sqlalchemy import select
+
+from lembas.db.models import (
+    AUTHOR_MODEL,
+    AUTHOR_USER,
+    ROLE_USER,
+    Chat,
+    Connection,
+    Model,
+    Persona,
+    User,
+)
+from lembas.services import harness as harness_service
+from lembas.services import personas as personas_service
+from lembas.services import settings_store
+from lembas.services import tools as tools_service
+from lembas.services.crypto import encrypt
+
+
+@pytest.fixture(autouse=True)
+def personality_allowed(db, registered):
+    settings_store.update(db, {"default_permissions": {"tools.persona": True}})
+    connection = Connection(
+        name="Test", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("")
+    )
+    db.add(connection)
+    db.commit()
+    for index, name in enumerate(("test-model", "other-model")):
+        db.add(
+            Model(
+                connection_id=connection.id,
+                model_id=name,
+                display_name=name,
+                position=index,
+                capabilities_json={"tools": True},
+            )
+        )
+    db.commit()
+
+
+def _user(db) -> User:
+    return db.scalars(select(User).order_by(User.created_at)).first()
+
+
+def _second_user(db) -> User:
+    """A row directly, the way `test_sharing.py` makes its three accounts."""
+    from lembas.security.passwords import hash_password
+
+    user = User(
+        name="Sam", email="s@example.test", password_hash=hash_password("x"), role="user"
+    )
+    db.add(user)
+    db.commit()
+    return user
+
+
+def _chat(db, model_id: str = "test-model", user: User | None = None) -> Chat:
+    chat = Chat(user_id=(user or _user(db)).id, title="t", model_id=model_id)
+    db.add(chat)
+    db.commit()
+    return chat
+
+
+async def _run(db, chat: Chat, name: str, args: dict):
+    user = db.get(User, chat.user_id)
+    resolved = tools_service.resolve_tools(db, chat, user)
+    context = tools_service.context_for(db, user, chat, tools=resolved)
+    return await tools_service.run_tool(context, name, json.dumps(args))
+
+
+# --- The two halves are not the same row --------------------------------------
+def test_a_persona_and_a_reflection_are_separate_rows_for_one_model(db):
+    user = _user(db)
+    personas_service.write(db, model_key="test-model", owner=None, content="I am terse.")
+    personas_service.write(db, model_key="test-model", owner=user, content="They test things.")
+
+    assert personas_service.block(db, "test-model", None) == "I am terse."
+    assert personas_service.block(db, "test-model", user) == "They test things."
+
+
+def test_a_missing_persona_does_not_fall_back_to_a_reflection(db):
+    """They answer different questions. A fallback between them would put "what
+    it makes of you" where "who it is" belongs, in the first person."""
+    user = _user(db)
+    personas_service.write(db, model_key="test-model", owner=user, content="They test things.")
+    assert personas_service.block(db, "test-model", None) == ""
+
+
+def test_each_model_keeps_its_own_read_of_the_same_person(db):
+    user = _user(db)
+    personas_service.write(db, model_key="test-model", owner=user, content="Impatient.")
+    personas_service.write(db, model_key="other-model", owner=user, content="Thorough.")
+
+    assert personas_service.block(db, "test-model", user) == "Impatient."
+    assert personas_service.block(db, "other-model", user) == "Thorough."
+
+
+def test_one_accounts_reflection_is_invisible_to_another(db):
+    """The whole reason the reflection is keyed on the person and not only on the
+    model. On an instance with two accounts, inheriting somebody else's is both
+    wrong and a disclosure."""
+    first = _user(db)
+    second = _second_user(db)
+    personas_service.write(db, model_key="test-model", owner=first, content="Writes tests.")
+
+    assert personas_service.block(db, "test-model", second) == ""
+    assert [row.content for row in personas_service.reflections_for(db, second)] == []
+    assert [row.content for row in personas_service.reflections_for(db, first)] == [
+        "Writes tests."
+    ]
+
+
+# --- Writing, keeping, and going back -----------------------------------------
+def test_every_change_keeps_what_was_there(db):
+    personas_service.write(db, model_key="test-model", owner=None, content="First.")
+    personas_service.write(
+        db, model_key="test-model", owner=None, content="Second.", note="thought again"
+    )
+
+    row = personas_service.get(db, "test-model", None)
+    assert row.content == "Second."
+    assert [r.content for r in row.revisions] == ["First."]
+    assert row.revisions[0].note == "thought again"
+
+
+def test_writing_the_same_text_again_keeps_no_revision(db):
+    """Otherwise a model that rewrites itself identically every turn fills the
+    history and pushes the real "before" out of it."""
+    personas_service.write(db, model_key="test-model", owner=None, content="Same.")
+    personas_service.write(db, model_key="test-model", owner=None, content="Same.")
+    assert personas_service.get(db, "test-model", None).revisions == []
+
+
+def test_reverting_keeps_the_text_it_replaced(db):
+    """An undo that cannot be undone is a second way to lose the same work."""
+    personas_service.write(db, model_key="test-model", owner=None, content="First.")
+    personas_service.write(db, model_key="test-model", owner=None, content="Second.")
+    row = personas_service.get(db, "test-model", None)
+
+    personas_service.revert(db, row, row.revisions[0])
+
+    # The session is built with `expire_on_commit=False`, so a committed change
+    # is not visible through an object already loaded here until it is expired.
+    db.expire_all()
+    row = personas_service.get(db, "test-model", None)
+    assert row.content == "First."
+    assert "Second." in [r.content for r in row.revisions]
+    assert row.author == AUTHOR_USER
+
+
+def test_the_history_is_bounded(db):
+    for index in range(personas_service.MAX_REVISIONS + 8):
+        personas_service.write(db, model_key="test-model", owner=None, content=f"v{index}")
+    db.expire_all()
+    row = personas_service.get(db, "test-model", None)
+    assert len(row.revisions) <= personas_service.MAX_REVISIONS
+
+
+def test_an_over_long_text_is_trimmed_rather_than_refused(db):
+    """`memories.py`'s rule: a write the model could not have known was too long
+    should not cost it the turn."""
+    row = personas_service.write(
+        db, model_key="test-model", owner=None, content="x" * 5000
+    )
+    assert len(row.content) == personas_service.MAX_PERSONA_CHARS
+
+
+def test_a_reflection_is_held_to_the_shorter_limit(db):
+    row = personas_service.write(
+        db, model_key="test-model", owner=_user(db), content="y" * 5000
+    )
+    assert len(row.content) == personas_service.MAX_VIEW_CHARS
+
+
+def test_the_row_survives_the_model_row_being_replaced(db):
+    """Keyed on the model's own id and not on the `Model` primary key, because
+    "Test & refresh" deletes a model the endpoint has stopped listing and gives
+    it a new primary key when it returns. A personality must not be collateral."""
+    personas_service.write(db, model_key="test-model", owner=None, content="I am terse.")
+    row = db.scalar(select(Model).where(Model.model_id == "test-model"))
+    connection_id = row.connection_id
+    db.delete(row)
+    db.commit()
+    db.add(Model(connection_id=connection_id, model_id="test-model"))
+    db.commit()
+
+    assert personas_service.block(db, "test-model", None) == "I am terse."
+
+
+# --- What the tools write -----------------------------------------------------
+async def test_persona_write_can_only_rewrite_the_answering_model(db):
+    """There is deliberately no argument naming a model: the key is the model
+    this reply is being written by, so a call cannot reach another one's."""
+    chat = _chat(db, "test-model")
+    outcome = await _run(db, chat, "persona_write", {"content": "I am blunt.", "why": "learnt"})
+
+    assert outcome.event["status"] == "ok"
+    assert personas_service.block(db, "test-model", None) == "I am blunt."
+    assert personas_service.block(db, "other-model", None) == ""
+
+
+async def test_persona_write_is_recorded_as_the_models_own_work(db):
+    chat = _chat(db)
+    await _run(db, chat, "persona_write", {"content": "Mine."})
+    assert personas_service.get(db, "test-model", None).author == AUTHOR_MODEL
+
+
+async def test_an_empty_persona_write_is_refused_rather_than_erasing(db):
+    """It replaces rather than appends, so an empty call would be a wipe — and a
+    model that has been talked into one turn of nonsense should not be able to
+    end its own character in it."""
+    personas_service.write(db, model_key="test-model", owner=None, content="I am terse.")
+    chat = _chat(db)
+
+    outcome = await _run(db, chat, "persona_write", {"content": "  "})
+
+    assert outcome.event["status"] == "error"
+    assert personas_service.block(db, "test-model", None) == "I am terse."
+
+
+async def test_impression_write_is_keyed_on_the_person_as_well_as_the_model(db):
+    chat = _chat(db)
+    await _run(db, chat, "impression_write", {"content": "They want the short answer."})
+
+    user = _user(db)
+    assert personas_service.block(db, "test-model", user) == "They want the short answer."
+    # Not the model's own persona, which is the row next to it.
+    assert personas_service.block(db, "test-model", None) == ""
+
+
+async def test_an_empty_impression_write_clears_it(db):
+    """The opposite of the persona, on purpose: "I have no standing view of this
+    person" is a legitimate state, and "I have no character" is not."""
+    chat = _chat(db)
+    await _run(db, chat, "impression_write", {"content": "Something."})
+    await _run(db, chat, "impression_write", {"content": ""})
+    assert personas_service.block(db, "test-model", _user(db)) == ""
+
+
+async def test_the_tool_is_offered_only_with_the_capability_and_the_permission(db):
+    chat = _chat(db)
+    user = _user(db)
+    assert "persona_write" in tools_service.resolve_tools(db, chat, user).by_name
+
+    model = db.scalar(select(Model).where(Model.model_id == "test-model"))
+    model.capabilities_json = {"tools": True, "tool_persona": False}
+    db.commit()
+    assert "persona_write" not in tools_service.resolve_tools(db, chat, user).by_name
+
+    model.capabilities_json = {"tools": True}
+    user.role = ROLE_USER
+    settings_store.update(db, {"default_permissions": {"tools.persona": False}})
+    db.commit()
+    assert "persona_write" not in tools_service.resolve_tools(db, chat, user).by_name
+
+
+# --- What reaches the prompt --------------------------------------------------
+def _values(db, chat: Chat, *, families: list[str]) -> dict[str, str]:
+    offered = [
+        tool.schema
+        for tool in tools_service.registry(db).values()
+        if tools_service.gate_of(tool.family) in families
+    ]
+    return harness_service.context_variables(db, db.get(User, chat.user_id), offered, chat)
+
+
+def test_both_variables_are_gated_on_the_family(db):
+    """A model that may not keep either has no business being handed them, and
+    the query should not happen at all on an instance that does not use this."""
+    user = _user(db)
+    personas_service.write(db, model_key="test-model", owner=None, content="I am terse.")
+    personas_service.write(db, model_key="test-model", owner=user, content="Impatient.")
+    chat = _chat(db)
+
+    without = _values(db, chat, families=["memory"])
+    assert without["persona"] == ""
+    assert without["person_view"] == ""
+
+    with_it = _values(db, chat, families=["persona"])
+    assert with_it["persona"] == "I am terse."
+    assert with_it["person_view"] == "Impatient."
+
+
+def test_a_switched_off_persona_reads_as_absent(db):
+    row = personas_service.write(db, model_key="test-model", owner=None, content="I am terse.")
+    row.enabled = False
+    db.commit()
+    assert personas_service.block(db, "test-model", None) == ""
+
+
+def test_the_fragments_vanish_when_there_is_nothing_to_say(db):
+    chat = _chat(db)
+    preamble = harness_service.compose(
+        db,
+        _user(db),
+        [
+            tool.schema
+            for tool in tools_service.registry(db).values()
+            if tools_service.gate_of(tool.family) == "persona"
+        ],
+        chat,
+    )
+    assert "Who you are" not in preamble
+    assert "What you have made of them" not in preamble
+
+
+def test_the_fragments_carry_the_texts_when_there_are_some(db):
+    user = _user(db)
+    personas_service.write(db, model_key="test-model", owner=None, content="I argue back.")
+    personas_service.write(db, model_key="test-model", owner=user, content="Likes brevity.")
+    chat = _chat(db)
+
+    preamble = harness_service.compose(
+        db,
+        user,
+        [
+            tool.schema
+            for tool in tools_service.registry(db).values()
+            if tools_service.gate_of(tool.family) == "persona"
+        ],
+        chat,
+    )
+    assert "I argue back." in preamble
+    assert "Likes brevity." in preamble
+    # The persona comes before the impression: a fact the person stated should be
+    # read before an opinion the model formed about them.
+    assert preamble.index("I argue back.") < preamble.index("Likes brevity.")
+
+
+# --- The screens --------------------------------------------------------------
+def test_the_person_can_read_and_delete_what_a_model_makes_of_them(client, db, registered):
+    """The whole reason writing one is acceptable. A model-written note about
+    somebody that they cannot see is not something this should hold."""
+    user = _user(db)
+    personas_service.write(db, model_key="test-model", owner=user, content="Wants brevity.")
+
+    page = client.get("/settings")
+    assert "Wants brevity." in page.text
+    assert "What models make of you" in page.text
+
+    row = personas_service.reflections_for(db, user)[0]
+    client.post(f"/api/library/reflections/{row.id}/delete", follow_redirects=False)
+    db.expire_all()
+    assert personas_service.reflections_for(db, user) == []
+
+
+def test_nobody_can_delete_somebody_elses_reflection(client, db):
+    second = _second_user(db)
+    row = personas_service.write(
+        db, model_key="test-model", owner=second, content="Theirs."
+    )
+
+    response = client.post(f"/api/library/reflections/{row.id}/delete", follow_redirects=False)
+
+    assert response.status_code == 404
+    db.expire_all()
+    assert personas_service.get(db, "test-model", second) is not None
+
+
+def test_a_models_own_persona_cannot_be_deleted_from_the_settings_page(client, db):
+    """`owner_id IS NULL` is the instance's, not this person's. An id from that
+    half arriving at the reader's route must be refused on ownership rather than
+    found by existence."""
+    row = personas_service.write(db, model_key="test-model", owner=None, content="Instance.")
+
+    response = client.post(f"/api/library/reflections/{row.id}/delete", follow_redirects=False)
+
+    assert response.status_code == 404
+    db.expire_all()
+    assert personas_service.block(db, "test-model", None) == "Instance."
+
+
+def test_an_administrator_can_read_write_and_revert_a_persona(client, db):
+    model = db.scalar(select(Model).where(Model.model_id == "test-model"))
+
+    client.post(
+        f"/admin/models/{model.id}/persona",
+        data={"content": "I am terse."},
+        follow_redirects=False,
+    )
+    client.post(
+        f"/admin/models/{model.id}/persona",
+        data={"content": "I am not terse at all."},
+        follow_redirects=False,
+    )
+    db.expire_all()
+    row = personas_service.get(db, "test-model", None)
+    assert row.content == "I am not terse at all."
+    assert row.author == AUTHOR_USER
+
+    page = client.get(f"/admin/models/{model.id}/edit")
+    assert "I am not terse at all." in page.text
+    assert "Earlier personalities" in page.text
+
+    client.post(
+        f"/admin/models/{model.id}/persona/revert",
+        data={"revision_id": row.revisions[0].id},
+        follow_redirects=False,
+    )
+    db.expire_all()
+    assert personas_service.get(db, "test-model", None).content == "I am terse."
+
+
+def test_a_revision_of_another_model_cannot_be_restored_onto_this_one(client, db):
+    """Checked against this persona rather than merely existing, or an id from
+    another model's history transplants its personality."""
+    personas_service.write(db, model_key="other-model", owner=None, content="Theirs first.")
+    personas_service.write(db, model_key="other-model", owner=None, content="Theirs second.")
+    personas_service.write(db, model_key="test-model", owner=None, content="Mine.")
+    foreign = personas_service.get(db, "other-model", None).revisions[0]
+    model = db.scalar(select(Model).where(Model.model_id == "test-model"))
+
+    response = client.post(
+        f"/admin/models/{model.id}/persona/revert",
+        data={"revision_id": foreign.id},
+        follow_redirects=False,
+    )
+
+    assert response.status_code == 404
+    db.expire_all()
+    assert personas_service.block(db, "test-model", None) == "Mine."
+
+
+def test_clearing_the_persona_from_the_admin_page_removes_it(client, db):
+    model = db.scalar(select(Model).where(Model.model_id == "test-model"))
+    personas_service.write(db, model_key="test-model", owner=None, content="I am terse.")
+
+    client.post(f"/admin/models/{model.id}/persona", data={"content": ""}, follow_redirects=False)
+
+    db.expire_all()
+    assert personas_service.get(db, "test-model", None) is None
+    assert db.scalars(select(Persona)).all() == []

tests/test_roster.py

@@ -0,0 +1,175 @@
+"""The list of other models a model is given, and what decides it is there.
+
+The roster is one `{{variable}}` and one fragment, so the interesting assertions
+are about *absence*: it is missing on a single-model instance, missing for a
+model that may not ask anyone anything, and missing a model this account cannot
+reach. A list that is merely wrong would be bad; a list naming something the
+reader has no access to is a leak and a dead end at once, because asking it
+anything is refused by the same check.
+"""
+
+from __future__ import annotations
+
+import pytest
+from sqlalchemy import select
+
+from lembas.db.models import ROLE_USER, Chat, Connection, Group, Model, User
+from lembas.services import chat as chat_service
+from lembas.services import harness as harness_service
+from lembas.services import settings_store
+from lembas.services.crypto import encrypt
+
+
+@pytest.fixture(autouse=True)
+def three_models(db, registered):
+    settings_store.update(db, {"enabled": True}, key=settings_store.SUBAGENTS)
+    settings_store.update(db, {"default_permissions": {"tools.friend": True}})
+    connection = Connection(
+        name="Test", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("")
+    )
+    db.add(connection)
+    db.commit()
+    rows = [
+        ("test-model", "The asker", "", ""),
+        ("big-model", "Big", "Long reasoning problems", "70B, Q4, MMLU 82"),
+        ("small-model", "Small", "Quick summaries", ""),
+    ]
+    for index, (name, label, description, notes) in enumerate(rows):
+        db.add(
+            Model(
+                connection_id=connection.id,
+                model_id=name,
+                display_name=label,
+                description=description,
+                notes=notes,
+                position=index,
+                capabilities_json={"tools": True},
+            )
+        )
+    db.commit()
+
+
+def _user(db) -> User:
+    return db.scalars(select(User).order_by(User.created_at)).first()
+
+
+def _chat(db, model_id: str = "test-model") -> Chat:
+    chat = Chat(user_id=_user(db).id, title="t", model_id=model_id)
+    db.add(chat)
+    db.commit()
+    return chat
+
+
+def _offered(db, families: list[str]) -> list[dict]:
+    """Tool schemas for the families named, built from the real definitions so a
+    family that stops existing takes these tests with it rather than passing on
+    a hand-written string."""
+    from lembas.services import tools as tools_service
+
+    return [
+        tool.schema
+        for tool in tools_service.registry(db).values()
+        if tools_service.gate_of(tool.family) in families
+    ]
+
+
+def _values(db, chat: Chat, *, families: list[str]) -> dict[str, str]:
+    return harness_service.context_variables(db, _user(db), _offered(db, families), chat)
+
+
+def _preamble(db, chat: Chat, *, families: list[str]) -> str:
+    """The whole harness, through the path a request actually takes."""
+    return harness_service.compose(db, _user(db), _offered(db, families), chat)
+
+
+def test_every_other_model_is_listed_with_its_id(db):
+    block = chat_service.roster_block(db, _user(db), exclude="test-model")
+    assert "big-model" in block
+    assert "small-model" in block
+    assert "Big" in block
+
+
+def test_the_asking_model_is_not_in_its_own_roster(db):
+    block = chat_service.roster_block(db, _user(db), exclude="test-model")
+    assert "test-model" not in block
+
+
+def test_the_description_and_the_notes_both_reach_it(db):
+    """Two fields on purpose: the description says what a model is for and is
+    also shown to people, the notes say what it *is* and are for this alone. A
+    model choosing whom to ask wants both."""
+    block = chat_service.roster_block(db, _user(db), exclude="test-model")
+    assert "Long reasoning problems" in block
+    assert "70B, Q4, MMLU 82" in block
+
+
+def test_a_model_this_account_cannot_reach_is_absent(db):
+    group = Group(name="Wheel")
+    db.add(group)
+    restricted = db.scalar(select(Model).where(Model.model_id == "big-model"))
+    restricted.public = False
+    restricted.groups = [group]
+    user = _user(db)
+    user.role = ROLE_USER
+    db.commit()
+
+    block = chat_service.roster_block(db, user, exclude="test-model")
+    assert "big-model" not in block
+    assert "small-model" in block
+
+
+def test_a_disabled_model_is_absent(db):
+    off = db.scalar(select(Model).where(Model.model_id == "small-model"))
+    off.enabled = False
+    db.commit()
+    assert "small-model" not in chat_service.roster_block(db, _user(db), exclude="test-model")
+
+
+def test_the_block_is_bounded(db):
+    """Every model an instance has multiplies this, and the harness has a budget
+    the whole of it shares."""
+    connection = db.scalars(select(Connection)).first()
+    for index in range(60):
+        db.add(
+            Model(
+                connection_id=connection.id,
+                model_id=f"filler-{index}",
+                display_name=f"Filler {index}",
+                notes="x" * 400,
+                position=10 + index,
+            )
+        )
+    db.commit()
+
+    block = chat_service.roster_block(db, _user(db), exclude="test-model")
+    assert len(block) <= chat_service.MAX_ROSTER_CHARS + chat_service.MAX_ROSTER_ENTRY
+    assert len(block.splitlines()) <= chat_service.MAX_ROSTER_MODELS
+
+
+# --- Whether it is sent at all ------------------------------------------------
+def test_the_variable_is_empty_for_a_model_that_cannot_ask_anyone(db):
+    """Gated on the family, exactly as the memories block is gated on memory. A
+    list of peers a model cannot reach is context spent on nothing, and it is why
+    the roster and the tool are one switch rather than two."""
+    chat = _chat(db)
+    assert _values(db, chat, families=["memory"])["model_roster"] == ""
+    assert _values(db, chat, families=["friend"])["model_roster"] != ""
+
+
+def test_the_fragment_vanishes_on_a_single_model_instance(db):
+    """`requires` rather than a conditional in the text: a heading above an empty
+    list reads as "there is nobody", which is a different and wrong claim."""
+    for extra in db.scalars(select(Model).where(Model.model_id != "test-model")):
+        db.delete(extra)
+    db.commit()
+
+    chat = _chat(db)
+    assert _values(db, chat, families=["friend"])["model_roster"] == ""
+    assert "The other models here" not in _preamble(db, chat, families=["friend"])
+
+
+def test_the_fragment_carries_the_list_when_there_is_one(db):
+    chat = _chat(db)
+    assembled = _preamble(db, chat, families=["friend"])
+    assert "The other models here" in assembled
+    assert "big-model" in assembled

tests/test_tool_activity.py

@@ -165,13 +165,22 @@ def test_a_custom_tools_own_label_still_wins():
     assert "Weather" in html
 
 
-def test_every_builtin_and_agent_tool_has_a_label_and_an_icon():
+def test_every_builtin_and_agent_tool_has_a_label_and_an_icon(db):
     """A property, not markup. A tool added without an entry renders its own
-    function name at somebody, which is the state this replaced."""
-    names = [tool.name for tool in tools_service.REGISTRY.values()]
+    function name at somebody, which is the state this replaced.
+
+    Through `registry(db)` rather than `REGISTRY`, because the latter holds only
+    the tools built at import time: the scheduling, subagent, ask-a-friend and
+    image tools are all built by a function and were invisible here. Three of
+    them had labels only because somebody remembered, which is the arrangement
+    this test exists to replace.
+    """
+    names = [tool.name for tool in tools_service.registry(db).values()]
     names += [tool.name for tool in agent_tools.tool_defs()]
     # plan_submit is filtered out of tool_defs() outside Plan mode.
     names.append("plan_submit")
+    for expected in ("subagent_run", "ask_friend", "schedule_create", "image_generate"):
+        assert expected in names, f"{expected} is not in the registry; this test went blind"
     missing = [name for name in names if name not in tool_labels.LABELS]
     assert not missing, f"no label for {missing}"
     missing = [name for name in names if name not in tool_labels.ICONS]