fix(scopes): guard scope membership config, move checks to the mutation point

Addresses a second review pass against 10655792.

1. Scope membership configuration was directly user-mutable.
   `PUT /sessions/{id}/peers/{peer_id}/config` had no scope guard at all, and
   `crud.set_peer_config` resolved the peer only to discard the row. Confirmed:
   posting `{"observe_others": false, "observe_me": true}` for a scope returned
   204 and persisted, which silently stops all fan-out into the scope and makes
   Honcho form a representation *of* a scope — neither of which is reachable
   through the facade. Deterministic, no race required. Now checked on the row
   `get_peer` already returns, so it costs nothing and cannot race.

2. Empty and over-long names were still 500s. Removing the charset pattern from
   `PeerSpec` fixed one trap but left its length bounds, and request-bound peer
   names carry no length limits of their own — so `peer_id: ""` or a 513-char
   name reached `PeerSpec(...)` and raised a raw pydantic ValidationError that
   the catch-all turned into a 500. `PeerSpec` now carries no constraints at all
   (matching its documented purpose) and every rule for a new name lives in
   `_validate_new_peer_names` on the insert path.

3. Resolved-row protection generalized. The previous pass applied it only to
   membership upserts, leaving check-then-use windows elsewhere: peer update
   could have a concurrently-created scope's configuration replaced wholesale
   (create-path validation does not fire for a peer that now exists), the chat
   observer get-or-create could resolve a fresh scope as its observer, and the
   generic session-peer removal could silently detach a scope from its sessions.
   Each now inspects the resolved peer immediately before acting; the redundant
   name-level guard on the update route is dropped in favor of the race-free one.

`remove_peers_from_session` grows an internal `_allow_scope_peers` flag because
the scopes facade ends membership through that same path and must not be blocked
by its own guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/crud/peer.py

@@ -28,6 +28,9 @@ from src.utils.types import GetOrCreateResult
 
 logger = getLogger(__name__)
 
+# Matches the peers.name CHECK constraint and PeerCreate's max_length.
+PEER_NAME_MAX_LENGTH = 512
+
 PEER_CACHE_KEY_TEMPLATE = "v2:workspace:{workspace_name}:peer:{peer_name}"
 PEER_LOCK_PREFIX = f"{get_cache_namespace()}:lock:v2"
 
@@ -58,6 +61,12 @@ def _validate_new_peer_names(names: list[str]) -> None:
     scopes_util.validate_no_scope_peer_names(
         names, action="Use the scopes routes to create scopes."
     )
+    too_long = sorted({n for n in names if len(n) > PEER_NAME_MAX_LENGTH})
+    if too_long:
+        raise ValidationException(
+            f"Peer name(s) must be at most {PEER_NAME_MAX_LENGTH} characters"
+        )
+    # RESOURCE_NAME_PATTERN's `+` already rejects the empty name.
     offenders = sorted({n for n in names if not re.fullmatch(RESOURCE_NAME_PATTERN, n)})
     if offenders:
         raise ValidationException(
@@ -386,6 +395,17 @@ async def update_peer(
     )
     honcho_peer = peers_result.resource[0]
 
+    # Refuse a real scope on the row just resolved, not on the name beforehand:
+    # this route replaces `configuration` wholesale, and a name-level check leaves
+    # a window in which a concurrently-created scope is resolved as existing (so
+    # create-path validation never fires) and then overwritten. An existing
+    # *unflagged* peer in the reserved namespace is an ordinary peer and passes.
+    if scopes_util.is_scope_peer(honcho_peer.name, honcho_peer.internal_metadata):
+        raise ValidationException(
+            f"Peer '{peer_name}' is a scope."
+            + " Use the scopes routes to manage scopes."
+        )
+
     needs_update = False
 
     if peer.metadata is not None and honcho_peer.h_metadata != peer.metadata:

src/crud/scope.py

@@ -353,4 +353,6 @@ async def remove_session_from_scope(
         workspace_name=workspace_name,
         session_name=session_name,
         peer_names={scope_peer_name(scope_name)},
+        # This *is* the supported path for ending scope membership.
+        _allow_scope_peers=True,
     )

src/crud/session.py

@@ -34,7 +34,7 @@ from src.utils.scopes import is_scope_peer, scope_peer_name
 from src.utils.types import GetOrCreateResult
 from src.vector_store import get_external_vector_store
 
-from .peer import get_or_create_peers, get_peer
+from .peer import get_or_create_peers, get_peer, reject_scope_peers
 from .scope import SCOPE_MEMBERSHIP_CONFIG, get_or_create_scopes
 from .workspace import get_or_create_workspace
 
@@ -822,6 +822,8 @@ async def remove_peers_from_session(
     workspace_name: str,
     session_name: str,
     peer_names: set[str],
+    *,
+    _allow_scope_peers: bool = False,
 ) -> bool:
     """
     Remove specified peers from a session.
@@ -831,16 +833,32 @@ async def remove_peers_from_session(
         workspace_name: Name of the workspace
         session_name: Name of the session
         peer_names: Set of peer names to remove from the session
+        _allow_scope_peers: Internal. Set only by the scopes facade, which ends
+            scope membership through this same path and must not be blocked by
+            the guard below.
 
     Returns:
         True if peers were removed successfully
 
     Raises:
         ResourceNotFoundException: If the session does not exist
+        ValidationException: If any named peer is a scope
     """
     # Verify session exists
     await get_session(db, session_name, workspace_name)
 
+    # Scope membership is ended through the scopes routes, which also reconcile
+    # the scope's copies. Checked here rather than in the route so it is adjacent
+    # to the UPDATE below, leaving no window for a concurrently-created scope to
+    # be silently detached from its sessions.
+    if not _allow_scope_peers:
+        await reject_scope_peers(
+            db,
+            workspace_name,
+            peer_names,
+            action="Scope membership is managed via the scopes routes.",
+        )
+
     # Soft delete specified session peers by setting left_at timestamp
     update_stmt = (
         update(models.SessionPeer)
@@ -1227,10 +1245,18 @@ async def set_peer_config(
 
     Raises:
         ObserverException: If the update would exceed the observer limit
+        ValidationException: If the peer is a scope
     """
     # First, get the session and peer to ensure they exist
     await get_session(db, session_name, workspace_name)
-    await get_peer(db, workspace_name, peer_name)
+    peer = await get_peer(db, workspace_name, peer_name)
+
+    # A scope's membership config is the facade's, not the caller's: setting
+    # observe_others=false silently stops all fan-out into the scope, and
+    # observe_me=true makes Honcho form a representation *of* a scope, which
+    # never happens by design. Checked on the row just resolved above, so there
+    # is no check-then-use window and no extra query.
+    _reject_resolved_scope_peers([peer])
 
     # Check if a SessionPeer entry already exists
     stmt = (

src/routers/peers.py

@@ -30,7 +30,11 @@ from src.telemetry import prometheus_metrics
 from src.telemetry.events import EmbeddingCallPurpose, GetContextEvent, emit
 from src.utils.filter import extract_session_allowlist
 from src.utils.schema_conversion import json_response_schema_to_pydantic
-from src.utils.scopes import is_scope_peer_name, validate_no_scope_peer_names
+from src.utils.scopes import (
+    is_scope_peer,
+    is_scope_peer_name,
+    validate_no_scope_peer_names,
+)
 from src.utils.search import search
 from src.utils.types import embedding_call_purpose
 
@@ -138,16 +142,13 @@ async def update_peer(
 ):
     """Update a Peer's metadata and/or configuration.
 
-    Three-way on the reserved namespace: a real scope is refused here (this route
-    replaces `configuration` wholesale, so it must never touch a facade-managed
-    peer); an existing *unflagged* peer that merely occupies the namespace is a
-    normal peer and updates fine; and a reserved-prefix name that does not exist
-    is refused by ``get_or_create_peers``' create-path validation rather than
-    being minted.
+    Three-way on the reserved namespace, all enforced inside ``crud.update_peer``
+    on the resolved row so there is no check-then-use window: a real scope is
+    refused (this route replaces `configuration` wholesale, so it must never touch
+    a facade-managed peer); an existing *unflagged* peer that merely occupies the
+    namespace is a normal peer and updates fine; and a reserved-prefix name that
+    does not exist is refused by create-path validation rather than being minted.
     """
-    await crud.reject_scope_peers(
-        db, workspace_id, [peer_id], action="Use the scopes routes to manage scopes."
-    )
     updated_peer = await crud.update_peer(
         db, workspace_name=workspace_id, peer_name=peer_id, peer=peer
     )
@@ -279,6 +280,15 @@ async def chat(
             workspace_name=workspace_id,
             peers=[schemas.PeerSpec(name=peer_id)],
         )
+        # Re-check on the resolved row: the name-level check above ran before the
+        # peer was resolved, so a scope created in between would be picked up here
+        # as existing and used as the chat observer.
+        observer = peers_result.resource[0]
+        if is_scope_peer(observer.name, observer.internal_metadata):
+            raise ValidationException(
+                "No representation is formed of a scope, so a scope cannot be a "
+                + "chat observer or target."
+            )
         await peer_db.commit()
     await peers_result.post_commit()
 

src/schemas/api.py

@@ -179,9 +179,16 @@ class PeerSpec(PeerBase):
     author, an existing row — including pre-``d429de0e5338`` legacy names containing
     '.' and every ``scope.``-prefixed peer name. Re-validating those turns a lookup
     into a raw pydantic ValidationError, i.e. an HTTP 500.
+
+    Carries **no** constraints at all, deliberately. Length limits here were the
+    same trap as the charset pattern: request-bound peer names (message authors,
+    session peer-map keys) have no length bound of their own, so an empty or
+    over-long name reached ``PeerSpec(...)`` and raised internally — again a 500.
+    Every rule for a *new* name lives in ``crud.peer._validate_new_peer_names``,
+    which runs on the insert path only.
     """
 
-    name: Annotated[str, Field(min_length=1, max_length=512)]
+    name: str
     metadata: _SanitizedMetadata | None = None
     configuration: dict[str, Any] | None = None
 

tests/routes/test_scopes.py

@@ -965,3 +965,109 @@ async def test_resolved_scope_peer_rejected_at_membership_upsert(
             ),
             workspace_name=test_workspace.name,
         )
+
+
+def test_set_peer_config_cannot_disable_a_scope(
+    client: TestClient, sample_data: tuple[Workspace, Peer]
+):
+    """A scope's membership config belongs to the facade, not the caller.
+
+    `observe_others=false` would silently stop all fan-out into the scope, and
+    `observe_me=true` would make Honcho form a representation *of* a scope.
+    """
+    test_workspace, _ = sample_data
+    scope_name = str(generate_nanoid())
+    assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
+    backing = scope_peer_name(scope_name)
+    session_name = _create_session(client, test_workspace.name)
+    assert (
+        client.post(
+            f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
+            json={"session_ids": [session_name]},
+        ).status_code
+        == 200
+    )
+
+    response = client.put(
+        f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers/{backing}/config",
+        json={"observe_others": False, "observe_me": True},
+    )
+    assert response.status_code == 422, response.text
+
+    # Observer semantics intact
+    current = client.get(
+        f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers/{backing}/config"
+    )
+    assert current.status_code == 200
+    assert current.json() == {"observe_me": False, "observe_others": True}
+
+
+async def test_unflagged_squatter_config_still_settable(
+    client: TestClient,
+    db_session: AsyncSession,
+    sample_data: tuple[Workspace, Peer],
+):
+    """The set_peer_config guard is flag-based, so a squatter is unaffected."""
+    test_workspace, _ = sample_data
+    squatter = scope_peer_name(str(generate_nanoid()))
+    db_session.add(models.Peer(workspace_name=test_workspace.name, name=squatter))
+    await db_session.commit()
+
+    session_name = _create_session(client, test_workspace.name)
+    assert (
+        client.post(
+            f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers",
+            json={squatter: {}},
+        ).status_code
+        == 200
+    )
+
+    response = client.put(
+        f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers/{squatter}/config",
+        json={"observe_others": False, "observe_me": True},
+    )
+    assert response.status_code == 204, response.text
+
+
+def test_generic_session_remove_cannot_detach_a_scope(
+    client: TestClient, sample_data: tuple[Workspace, Peer]
+):
+    """Scope membership must end through the scopes routes, which reconcile."""
+    test_workspace, _ = sample_data
+    scope_name = str(generate_nanoid())
+    assert _create_scope(client, test_workspace.name, scope_name).status_code == 201
+    session_name = _create_session(client, test_workspace.name)
+    assert (
+        client.post(
+            f"/v3/workspaces/{test_workspace.name}/scopes/{scope_name}/sessions",
+            json={"session_ids": [session_name]},
+        ).status_code
+        == 200
+    )
+
+    response = client.request(
+        "DELETE",
+        f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/peers",
+        json=[scope_peer_name(scope_name)],
+    )
+    assert response.status_code == 422, response.text
+
+
+@pytest.mark.parametrize("bad_name", ["", "a" * 513])
+def test_degenerate_peer_names_are_422_not_500(
+    client: TestClient, sample_data: tuple[Workspace, Peer], bad_name: str
+):
+    """Empty and over-long author names must not reach PeerSpec and 500.
+
+    Request-bound peer names carry no length bound of their own, so these used to
+    raise a raw pydantic ValidationError inside crud, which the catch-all handler
+    turned into a 500.
+    """
+    test_workspace, _ = sample_data
+    session_name = _create_session(client, test_workspace.name)
+
+    response = client.post(
+        f"/v3/workspaces/{test_workspace.name}/sessions/{session_name}/messages",
+        json={"messages": [{"peer_id": bad_name, "content": "hello"}]},
+    )
+    assert response.status_code == 422, response.text