From 5823f0fae930f2da9f1b45d3a0f6a16b148e70bc Mon Sep 17 00:00:00 2001 From: Ken Weiner Date: Tue, 25 Aug 2026 06:45:01 -0700 Subject: [PATCH 1/7] fix: only classify genuine oversize input as a token-limit error (#791) Callers wrapped every ValueError from the embedding client in a "exceeds maximum token limit" message, so provider and configuration failures (dimension mismatch, empty response, upstream error) surfaced to users as though their input were too long. Add EmbeddingTokenLimitError, raised only by the pre-flight token checks in embed() and simple_batch_embed(), and narrow the remaps in search.py, agent_tools.py, document.py and representation.py to catch it. It subclasses ValueError so existing broad handlers keep working. Both simple_batch_embed() remap sites pass on_oversize="truncate" and so could never raise a token-limit error at all; their handlers only ever mislabelled provider failures. Fixes #568 Co-authored-by: Claude Opus 5 --- src/crud/document.py | 6 +-- src/crud/representation.py | 4 +- src/embedding_client.py | 19 +++++++-- src/utils/agent_tools.py | 8 +++- src/utils/search.py | 4 +- tests/llm/test_embedding_client.py | 68 ++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/crud/document.py b/src/crud/document.py index 0cec85c9..1b04bb0a 100644 --- a/src/crud/document.py +++ b/src/crud/document.py @@ -18,7 +18,7 @@ from src.crud.collection import get_or_create_collection from src.crud.peer import get_peer, reject_scope_observed from src.crud.session import get_session from src.dependencies import tracked_db -from src.embedding_client import embedding_client +from src.embedding_client import EmbeddingTokenLimitError, embedding_client from src.exceptions import ( ResourceNotFoundException, ValidationException, @@ -362,7 +362,7 @@ async def query_documents( if embedding is None: try: embedding = await embedding_client.embed(query) - except ValueError as e: + except EmbeddingTokenLimitError as e: raise ValidationException( "Query exceeds maximum token limit of " + f"{settings.EMBEDDING.MAX_INPUT_TOKENS}." @@ -987,7 +987,7 @@ async def create_observations( embeddings = await embedding_client.simple_batch_embed( contents, on_oversize="truncate" ) - except ValueError as e: + except EmbeddingTokenLimitError as e: raise ValidationException(str(e)) from e # Create document objects and track embeddings for vector store diff --git a/src/crud/representation.py b/src/crud/representation.py index 3b7070e8..6fafb842 100644 --- a/src/crud/representation.py +++ b/src/crud/representation.py @@ -13,7 +13,7 @@ from src import crud, exceptions, models, schemas from src.config import settings from src.dependencies import tracked_db from src.dreamer.dream_scheduler import check_and_schedule_dream -from src.embedding_client import embedding_client +from src.embedding_client import EmbeddingTokenLimitError, embedding_client from src.schemas import ResolvedConfiguration from src.telemetry.events import EmbeddingCallPurpose from src.telemetry.logging import accumulate_metric @@ -109,7 +109,7 @@ class RepresentationManager: embeddings = await embedding_client.simple_batch_embed( observation_texts, on_oversize="truncate" ) - except ValueError as e: + except EmbeddingTokenLimitError as e: raise exceptions.ValidationException( "Observation content exceeds maximum token limit of " + f"{settings.EMBEDDING.MAX_INPUT_TOKENS}." diff --git a/src/embedding_client.py b/src/embedding_client.py index e55e09fd..d3f4e369 100644 --- a/src/embedding_client.py +++ b/src/embedding_client.py @@ -159,6 +159,17 @@ def _publish_embedding_event( logger.debug("Failed to emit EmbeddingCallCompletedEvent", exc_info=True) +class EmbeddingTokenLimitError(ValueError): + """Raised when input text genuinely exceeds the model's token limit. + + Subclasses ``ValueError`` so existing broad handlers keep working, while + letting callers tell a real "content too long" condition apart from a + transient provider or configuration failure (dimension mismatch, empty + response, upstream error). Only the pre-flight token checks raise this; + provider failures keep raising plain ``ValueError``. + """ + + class BatchItem(NamedTuple): """A single item in a batch with its metadata.""" @@ -272,7 +283,7 @@ class _EmbeddingClient: token_count = len(self.encoding.encode(query)) if token_count > self.max_embedding_tokens: - raise ValueError( + raise EmbeddingTokenLimitError( f"Query exceeds maximum token limit of {self.max_embedding_tokens} tokens (got {token_count} tokens)" ) @@ -358,8 +369,8 @@ class _EmbeddingClient: List of embedding vectors, one per input text (in order) Raises: - ValueError: If any text exceeds token limits and `on_oversize` is - ``"raise"`` + EmbeddingTokenLimitError: If any text exceeds token limits and + `on_oversize` is ``"raise"`` """ if not texts: return [] @@ -380,7 +391,7 @@ class _EmbeddingClient: tokens, ) else: - raise ValueError( + raise EmbeddingTokenLimitError( f"Text at index {idx} exceeds maximum token limit of " + f"{self.max_embedding_tokens} tokens (got {len(token_ids)} tokens)" ) diff --git a/src/utils/agent_tools.py b/src/utils/agent_tools.py index 0dbdd304..5f87d455 100644 --- a/src/utils/agent_tools.py +++ b/src/utils/agent_tools.py @@ -13,7 +13,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from src import crud, models, schemas from src.config import settings from src.dependencies import tracked_db -from src.embedding_client import embedding_client +from src.embedding_client import EmbeddingTokenLimitError, embedding_client from src.exceptions import ResourceNotFoundException from src.models import Document from src.schemas import ResolvedConfiguration @@ -1884,11 +1884,15 @@ async def _handle_search_memory( parent_category=ctx.parent_category, ): query_embedding = await embedding_client.embed(query) - except ValueError: + except EmbeddingTokenLimitError: return ( "ERROR: Query exceeds maximum token limit of " + f"{settings.EMBEDDING.MAX_INPUT_TOKENS}. Please use a shorter query." ) + except ValueError as e: + # Provider/config failure, not an oversized query. Keep returning a + # string so the tool loop can continue, but don't blame the query. + return f"ERROR: Embedding the query failed: {e}" # Base telemetry metadata; results_count gets filled in below. search_meta: dict[str, Any] = { diff --git a/src/utils/search.py b/src/utils/search.py index 761b63e0..329e082b 100644 --- a/src/utils/search.py +++ b/src/utils/search.py @@ -14,7 +14,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from src import models from src.config import settings from src.dependencies import tracked_db -from src.embedding_client import embedding_client +from src.embedding_client import EmbeddingTokenLimitError, embedding_client from src.exceptions import ValidationException from src.models import session_peers_table from src.telemetry.events import EmbeddingCallPurpose @@ -388,7 +388,7 @@ async def search( parent_category="api", ): query_embedding = await embedding_client.embed(query) - except ValueError as e: + except EmbeddingTokenLimitError as e: raise ValidationException( f"Query exceeds maximum token limit of {settings.EMBEDDING.MAX_INPUT_TOKENS}." ) from e diff --git a/tests/llm/test_embedding_client.py b/tests/llm/test_embedding_client.py index fc2ff411..7fd9237d 100644 --- a/tests/llm/test_embedding_client.py +++ b/tests/llm/test_embedding_client.py @@ -14,6 +14,7 @@ from src.config import ( from src.embedding_client import ( BatchItem, EmbeddingClient, + EmbeddingTokenLimitError, _EmbeddingClient, # pyright: ignore[reportPrivateUsage] ) @@ -1173,3 +1174,70 @@ async def test_gemini_process_batch_wraps_contents_as_content_part( assert all(isinstance(c, genai_types.Content) for c in contents) assert contents[0].parts[0].text == "hello" assert contents[1].parts[0].text == "world" + + +# --- Token-limit classification (issue #568) ------------------------------- +# +# Only genuine "content too long" conditions may raise +# EmbeddingTokenLimitError. Provider/config failures must stay plain +# ValueError so callers don't rewrite them as token-limit errors. + + +def test_embedding_token_limit_error_is_value_error() -> None: + """Subclassing ValueError keeps pre-existing broad handlers working.""" + assert issubclass(EmbeddingTokenLimitError, ValueError) + + +@pytest.mark.asyncio +async def test_embed_raises_token_limit_error_before_calling_provider( + monkeypatch: pytest.MonkeyPatch, +) -> None: + client, fake_embeddings = _build_openai_client( + monkeypatch, + embedding=[0.1, 0.2], + model="text-embedding-3-small", + send_dimensions=False, + vector_dimensions=2, + ) + + with pytest.raises(EmbeddingTokenLimitError): + await client.embed("word " * 20_000) + + assert fake_embeddings.calls == [], "provider must not be called on oversize input" + + +@pytest.mark.asyncio +async def test_simple_batch_embed_raises_token_limit_error_before_calling_provider( + monkeypatch: pytest.MonkeyPatch, +) -> None: + client, fake_embeddings = _build_openai_client( + monkeypatch, + embedding=[0.1, 0.2], + model="text-embedding-3-small", + send_dimensions=False, + vector_dimensions=2, + ) + + with pytest.raises(EmbeddingTokenLimitError): + await client.simple_batch_embed(["fine", "word " * 20_000]) + + assert fake_embeddings.calls == [], "provider must not be called on oversize input" + + +@pytest.mark.asyncio +async def test_provider_dimension_mismatch_is_not_a_token_limit_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A wrong-width vector is a provider/config fault, not an oversized input.""" + client, _ = _build_openai_client( + monkeypatch, + embedding=[0.1, 0.2, 0.3], # 3 wide, client expects 2 + model="text-embedding-3-small", + send_dimensions=False, + vector_dimensions=2, + ) + + with pytest.raises(ValueError) as excinfo: + await client.embed("short query") + + assert not isinstance(excinfo.value, EmbeddingTokenLimitError) From 4492f66bca7515e265ab38286858aeb6db0cfcc3 Mon Sep 17 00:00:00 2001 From: Aakash Kattelu Date: Tue, 25 Aug 2026 10:51:59 -0400 Subject: [PATCH 2/7] fix(crud): preserve joined_at for active session peers (#1059) * fix(crud): preserve joined_at for active session peers Re-adding an already-active peer no longer advances the membership window, so peer_perspective search keeps messages from the original join. Genuine rejoins still start a new window. * docs: document set_peers membership window and wrap test docstrings * fix: preserve session observer limit --------- Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com> --- src/crud/session.py | 160 +++++++++++------- tests/crud/test_session.py | 328 ++++++++++++++++++++++++++++++++++++- tests/test_search.py | 71 +++++++- 3 files changed, 499 insertions(+), 60 deletions(-) diff --git a/src/crud/session.py b/src/crud/session.py index efa3f664..51ca1f13 100644 --- a/src/crud/session.py +++ b/src/crud/session.py @@ -1050,25 +1050,31 @@ async def set_peers_for_session( peer_names: dict[str, schemas.SessionPeerConfig], ) -> list[models.SessionPeer]: """ - Set peers for a session, overwriting any existing peers. - If peers don't exist, they will be created. + Replace a session's ordinary peer set with ``peer_names``. + + Active members keep their joined_at but take the incoming configuration: + this is a replace, so the caller's map is the desired end state. Departed + members rejoin with the incoming configuration. Scope memberships are + preserved. Args: db: Database session workspace_name: Name of the workspace session_name: Name of the session - peer_names: Set of peer names to set for the session + peer_names: Mapping of peer names to session-level configuration Returns: List of SessionPeer objects for all peers in the session Raises: ResourceNotFoundException: If the session does not exist + ObserverException: If the resulting peer set would exceed the observer + limit """ - # Validate observer limit before making any changes - observer_count = count_observers_in_config(peer_names) - if observer_count > settings.SESSION_OBSERVERS_LIMIT: - raise ObserverException(session_name, observer_count) + # No observer pre-check here: an already-active membership keeps its stored + # configuration, so the incoming map is not what lands. Counting it would + # reject a request that lowers the observer count as often as one that raises + # it. _get_or_add_peers_to_session enforces the limit on the resulting rows. # Verify session exists stmt = ( @@ -1084,20 +1090,21 @@ async def set_peers_for_session( f"Session {session_name} not found in workspace {workspace_name}" ) - # Soft delete every *ordinary* active membership. Scope memberships are - # deliberately preserved: this route replaces the peers the caller names, and a - # caller detaches a scope by simply *omitting* it from an otherwise valid - # replacement map — never naming it, so no request-level guard can see it. - # Without the exclusion a plain replacement silently bypasses the facade that - # owns scope membership and its removal reconciliation. Being part of the - # UPDATE, this holds regardless of the request body or concurrent scope - # creation. + # Soft delete every *ordinary* active membership not in the incoming map. + # Scope memberships are deliberately preserved: this route replaces the peers + # the caller names, and a caller detaches a scope by simply *omitting* it from + # an otherwise valid replacement map — never naming it, so no request-level + # guard can see it. Without the exclusion a plain replacement silently + # bypasses the facade that owns scope membership and its removal + # reconciliation. Being part of the UPDATE, this holds regardless of the + # request body or concurrent scope creation. update_stmt = ( update(models.SessionPeer) .where( models.SessionPeer.session_name == session_name, models.SessionPeer.workspace_name == workspace_name, models.SessionPeer.left_at.is_(None), # Only update active peers + models.SessionPeer.peer_name.notin_(peer_names.keys()), ~exists( select(models.Peer.id) .where(models.Peer.workspace_name == workspace_name) @@ -1118,12 +1125,14 @@ async def set_peers_for_session( ) _reject_resolved_scope_peers(peers_result.resource) - # Add new peers to session + # Add new peers to session. This route replaces the session's peer set, so the + # incoming configuration is authoritative even for an already-active member. peers = await _get_or_add_peers_to_session( db, workspace_name=workspace_name, session_name=session_name, peer_names=peer_names, + replace_config=True, ) await db.commit() @@ -1162,13 +1171,22 @@ async def _get_or_add_peers_to_session( peer_names: dict[str, schemas.SessionPeerConfig], *, fetch_after_upsert: bool = True, + replace_config: bool = False, ) -> list[models.SessionPeer]: """ Upsert session-peer memberships for a session and optionally fetch the active memberships afterward. New peers are inserted, peers that previously left the session are rejoined, - and already-active peers keep their existing session-level configuration. + and already-active peers keep their existing joined_at. + + An already-active peer also keeps its stored configuration unless + ``replace_config`` is set: an add must not overwrite configuration it was + never asked about, while a replace states the desired end state. + + The observer limit is checked against the rows the upsert actually produced, + not against the incoming map, since under the add semantics the incoming map + is not necessarily what lands. Args: db: Database session @@ -1177,13 +1195,17 @@ async def _get_or_add_peers_to_session( peer_names: Mapping of peer names to session-level configuration fetch_after_upsert: If True, query and return the active session peers after the upsert. If False, skip that read and return an empty list. + replace_config: If True, an already-active membership takes the incoming + configuration instead of keeping its stored one. Set by replace-style + callers; leave False for add-style callers. Returns: Active SessionPeer objects after the upsert, or an empty list when the post-upsert fetch is skipped Raises: - ObserverException: If adding peers would exceed the observer limit + ObserverException: If the resulting active peer set would exceed the + observer limit """ # If no peers to add, skip the insert and just return existing active session peers if not peer_names: @@ -1202,43 +1224,10 @@ async def _get_or_add_peers_to_session( # costs document rows, not LLM calls, and counting them would # cap scopes-per-session at SESSION_OBSERVERS_LIMIT and surface as an # observer-shaped 400 through a facade that hides observers entirely. + # Resolved up front because the limit check below gates on whether this + # request asks for a *non-scope* observer. scopes_being_added = await scope_peer_names(db, workspace_name, peer_names.keys()) - # Only validate observer limit if we're adding non-scope peers with observe_others=True - new_observer_count = count_observers_in_config( - {n: c for n, c in peer_names.items() if n not in scopes_being_added} - ) - - if new_observer_count > 0: - # Use a single efficient query to count existing observers not being updated - # This uses PostgreSQL's JSONB operators to check the observe_others field directly - existing_observers_stmt = select(func.count()).where( - models.SessionPeer.session_name == session_name, - models.SessionPeer.workspace_name == workspace_name, - models.SessionPeer.left_at.is_(None), # Only active peers - models.SessionPeer.peer_name.notin_( - peer_names.keys() - ), # Exclude peers being updated - models.SessionPeer.configuration["observe_others"].astext.cast( - Boolean - ), # Only observers - # Existing scope memberships are excluded for the same reason as above. - ~exists( - select(models.Peer.id) - .where(models.Peer.workspace_name == workspace_name) - .where(models.Peer.name == models.SessionPeer.peer_name) - .where(scope_peer_clause()) - .correlate(models.SessionPeer) - ), - ) - result = await db.execute(existing_observers_stmt) - existing_observer_count = result.scalar() or 0 - - total_observers = existing_observer_count + new_observer_count - - if total_observers > settings.SESSION_OBSERVERS_LIMIT: - raise ObserverException(session_name, total_observers) - # Use upsert to handle both new peers and rejoining peers stmt = pg_insert(models.SessionPeer).values( [ @@ -1254,15 +1243,27 @@ async def _get_or_add_peers_to_session( ] ) - # On conflict, update joined_at and clear left_at (rejoin scenario) - # If left_at is not None (peer has left the session): Use the new configuration (stmt.excluded.configuration) - # If left_at is None (peer is still active): Keep the existing configuration (models.SessionPeer.configuration) + # On conflict, rejoin departed peers. joined_at always survives on an active + # membership -- advancing it would move the peer_perspective search window + # past messages the peer was present for (issue #940). + # + # Configuration depends on the caller's semantics. An add ("ensure this peer + # is here") must not silently overwrite a config it never asked about, so an + # active membership keeps its stored one. A replace ("these are the session's + # peers, configured thus") states a desired end state, so the incoming config + # wins -- otherwise PUT /peers could never change the configuration of a peer + # already in the session. stmt = stmt.on_conflict_do_update( index_elements=["session_name", "peer_name", "workspace_name"], set_={ - "joined_at": func.now(), + "joined_at": case( + (models.SessionPeer.left_at.is_not(None), func.now()), + else_=models.SessionPeer.joined_at, + ), "left_at": None, - "configuration": case( + "configuration": stmt.excluded.configuration + if replace_config + else case( (models.SessionPeer.left_at.is_not(None), stmt.excluded.configuration), else_=models.SessionPeer.configuration, ), @@ -1270,6 +1271,49 @@ async def _get_or_add_peers_to_session( ) await db.execute(stmt) + # Enforce the observer limit on the resulting rows rather than predicting them. + # Under add semantics an already-active membership keeps its stored + # configuration (see the CASE above), so the incoming config is not what lands + # and cannot be counted: predicting from it silently undercounts preserved + # observers and lets a session grow past the limit indefinitely by re-sending + # its current observers at a lower config alongside new ones. Counting after + # the upsert is correct under both configuration semantics and cannot desync + # from those branches. Raising here rolls the upsert back: ObserverException + # is never caught, and both get_db and tracked_db roll back on exception. + # + # Gated on the request actually asking for a non-scope observer so that a + # session already over the limit keeps behaving as it does today: it can + # still take non-observers and scope attachments, and only a request that + # would make it worse is rejected. + if any( + config.observe_others + for peer_name, config in peer_names.items() + if peer_name not in scopes_being_added + ): + observer_count = ( + await db.scalar( + select(func.count()).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.workspace_name == workspace_name, + models.SessionPeer.left_at.is_(None), # Only active peers + models.SessionPeer.configuration["observe_others"].astext.cast( + Boolean + ), # Only observers + # Scope memberships are excluded for the reason given above. + ~exists( + select(models.Peer.id) + .where(models.Peer.workspace_name == workspace_name) + .where(models.Peer.name == models.SessionPeer.peer_name) + .where(scope_peer_clause()) + .correlate(models.SessionPeer) + ), + ) + ) + or 0 + ) + if observer_count > settings.SESSION_OBSERVERS_LIMIT: + raise ObserverException(session_name, observer_count) + if not fetch_after_upsert: return [] diff --git a/tests/crud/test_session.py b/tests/crud/test_session.py index 8b4b5795..2eec1f32 100644 --- a/tests/crud/test_session.py +++ b/tests/crud/test_session.py @@ -1,14 +1,340 @@ +from datetime import datetime, timezone + import pytest from nanoid import generate as generate_nanoid +from sqlalchemy import Boolean, func, select from sqlalchemy.ext.asyncio import AsyncSession from src import crud, models, schemas -from src.exceptions import ResourceNotFoundException +from src.config import settings +from src.exceptions import ObserverException, ResourceNotFoundException class TestSessionCRUD: """Test suite for session CRUD operations""" + @pytest.mark.asyncio + async def test_get_or_create_session_preserves_active_joined_at( + self, + db_session: AsyncSession, + sample_data: tuple[models.Workspace, models.Peer], + ): + """Active re-adds keep joined_at and config; a genuine rejoin starts a + new window.""" + test_workspace, test_peer = sample_data + session_name = str(generate_nanoid()) + original_config = schemas.SessionPeerConfig( + observe_others=True, observe_me=False + ) + updated_config = schemas.SessionPeerConfig( + observe_others=False, observe_me=True + ) + session_peer_stmt = select( + models.SessionPeer.joined_at, + models.SessionPeer.left_at, + models.SessionPeer.configuration, + ).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.peer_name == test_peer.name, + models.SessionPeer.workspace_name == test_workspace.name, + ) + + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, peers={test_peer.name: original_config} + ), + test_workspace.name, + ) + first_joined_at, first_left_at, first_config = ( + await db_session.execute(session_peer_stmt) + ).one() + assert first_left_at is None + assert first_config == original_config.model_dump() + + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, peers={test_peer.name: updated_config} + ), + test_workspace.name, + ) + second_joined_at, second_left_at, second_config = ( + await db_session.execute(session_peer_stmt) + ).one() + assert second_joined_at == first_joined_at + assert second_left_at is None + assert second_config == original_config.model_dump() + + session_peer = ( + await db_session.execute( + select(models.SessionPeer).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.peer_name == test_peer.name, + models.SessionPeer.workspace_name == test_workspace.name, + ) + ) + ).scalar_one() + session_peer.left_at = datetime.now(timezone.utc) + await db_session.commit() + + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, peers={test_peer.name: updated_config} + ), + test_workspace.name, + ) + rejoined_joined_at, rejoined_left_at, rejoined_config = ( + await db_session.execute(session_peer_stmt) + ).one() + assert rejoined_joined_at > second_joined_at + assert rejoined_left_at is None + assert rejoined_config == updated_config.model_dump() + + @pytest.mark.asyncio + async def test_set_peers_preserves_active_joined_at( + self, + db_session: AsyncSession, + sample_data: tuple[models.Workspace, models.Peer], + ): + """PUT-peers keeps active membership windows and refreshes real rejoins.""" + test_workspace, test_peer = sample_data + session_name = str(generate_nanoid()) + original_config = schemas.SessionPeerConfig( + observe_others=True, observe_me=False + ) + updated_config = schemas.SessionPeerConfig( + observe_others=False, observe_me=True + ) + db_session.add( + models.Session(name=session_name, workspace_name=test_workspace.name) + ) + await db_session.flush() + + session_peer_stmt = select( + models.SessionPeer.joined_at, + models.SessionPeer.left_at, + models.SessionPeer.configuration, + ).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.peer_name == test_peer.name, + models.SessionPeer.workspace_name == test_workspace.name, + ) + + await crud.set_peers_for_session( + db_session, + workspace_name=test_workspace.name, + session_name=session_name, + peer_names={test_peer.name: original_config}, + ) + first_left_at, first_config = ( + await db_session.execute( + select( + models.SessionPeer.left_at, + models.SessionPeer.configuration, + ).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.peer_name == test_peer.name, + models.SessionPeer.workspace_name == test_workspace.name, + ) + ) + ).one() + assert first_left_at is None + assert first_config == original_config.model_dump() + + session_peer = ( + await db_session.execute( + select(models.SessionPeer).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.peer_name == test_peer.name, + models.SessionPeer.workspace_name == test_workspace.name, + ) + ) + ).scalar_one() + session_peer.joined_at = datetime(2020, 1, 1, tzinfo=timezone.utc) + await db_session.commit() + + await crud.set_peers_for_session( + db_session, + workspace_name=test_workspace.name, + session_name=session_name, + peer_names={test_peer.name: updated_config}, + ) + active_joined_at, active_left_at, active_config = ( + await db_session.execute(session_peer_stmt) + ).one() + assert active_joined_at == datetime(2020, 1, 1, tzinfo=timezone.utc) + assert active_left_at is None + # A replace states the desired end state, so the incoming config lands even + # though the membership window is untouched. + assert active_config == updated_config.model_dump() + + await crud.set_peers_for_session( + db_session, + workspace_name=test_workspace.name, + session_name=session_name, + peer_names={}, + ) + await crud.set_peers_for_session( + db_session, + workspace_name=test_workspace.name, + session_name=session_name, + peer_names={test_peer.name: updated_config}, + ) + rejoined_joined_at, rejoined_left_at, rejoined_config = ( + await db_session.execute(session_peer_stmt) + ).one() + assert rejoined_joined_at > active_joined_at + assert rejoined_left_at is None + assert rejoined_config == updated_config.model_dump() + + @pytest.mark.asyncio + async def test_observer_limit_counts_preserved_config( + self, + db_session: AsyncSession, + sample_data: tuple[models.Workspace, models.Peer], + monkeypatch: pytest.MonkeyPatch, + ): + """On the add path an already-active observer keeps its stored config, so + it still counts against the limit when re-sent as a non-observer.""" + monkeypatch.setattr(settings, "SESSION_OBSERVERS_LIMIT", 2) + test_workspace, _ = sample_data + # Bound to a local: rollback below expires the ORM instance, and reloading + # it would lazy-load outside the greenlet context. + workspace_name = test_workspace.name + session_name = str(generate_nanoid()) + observer = schemas.SessionPeerConfig(observe_others=True, observe_me=False) + bystander = schemas.SessionPeerConfig(observe_others=False, observe_me=True) + existing = [str(generate_nanoid()) for _ in range(2)] + + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, peers=dict.fromkeys(existing, observer) + ), + workspace_name, + ) + + # Adding cannot demote an active member, so re-sending the two observers as + # non-observers leaves them observing and the third peer makes three. + with pytest.raises(ObserverException): + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, + peers={ + **dict.fromkeys(existing, bystander), + str(generate_nanoid()): observer, + }, + ), + workspace_name, + ) + + # The rejected request left nothing behind. + await db_session.rollback() + observer_count = await db_session.scalar( + select(func.count()).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.workspace_name == workspace_name, + models.SessionPeer.left_at.is_(None), + models.SessionPeer.configuration["observe_others"].astext.cast(Boolean), + ) + ) + assert observer_count == 2 + + @pytest.mark.asyncio + async def test_set_peers_observer_limit_counts_replaced_config( + self, + db_session: AsyncSession, + sample_data: tuple[models.Workspace, models.Peer], + monkeypatch: pytest.MonkeyPatch, + ): + """The replace path applies the incoming config, so demoting active + observers frees room under the limit in the same request.""" + monkeypatch.setattr(settings, "SESSION_OBSERVERS_LIMIT", 2) + test_workspace, _ = sample_data + workspace_name = test_workspace.name + session_name = str(generate_nanoid()) + db_session.add(models.Session(name=session_name, workspace_name=workspace_name)) + await db_session.flush() + observer = schemas.SessionPeerConfig(observe_others=True, observe_me=False) + bystander = schemas.SessionPeerConfig(observe_others=False, observe_me=True) + existing = [str(generate_nanoid()) for _ in range(2)] + + await crud.set_peers_for_session( + db_session, + workspace_name=workspace_name, + session_name=session_name, + peer_names=dict.fromkeys(existing, observer), + ) + + # Demoting both active observers while adding a new one leaves exactly one. + await crud.set_peers_for_session( + db_session, + workspace_name=workspace_name, + session_name=session_name, + peer_names={ + **dict.fromkeys(existing, bystander), + str(generate_nanoid()): observer, + }, + ) + observer_count = await db_session.scalar( + select(func.count()).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.workspace_name == workspace_name, + models.SessionPeer.left_at.is_(None), + models.SessionPeer.configuration["observe_others"].astext.cast(Boolean), + ) + ) + assert observer_count == 1 + + @pytest.mark.asyncio + async def test_observer_limit_lets_over_limit_session_take_non_observers( + self, + db_session: AsyncSession, + sample_data: tuple[models.Workspace, models.Peer], + monkeypatch: pytest.MonkeyPatch, + ): + """A session already past the limit still accepts non-observers, so + sessions that grew over it before enforcement do not become unusable.""" + monkeypatch.setattr(settings, "SESSION_OBSERVERS_LIMIT", 2) + test_workspace, _ = sample_data + workspace_name = test_workspace.name + session_name = str(generate_nanoid()) + observer = schemas.SessionPeerConfig(observe_others=True, observe_me=False) + bystander = schemas.SessionPeerConfig(observe_others=False, observe_me=True) + + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, + peers=dict.fromkeys( + [str(generate_nanoid()) for _ in range(2)], observer + ), + ), + workspace_name, + ) + + # Now the limit is below what the session already holds. + monkeypatch.setattr(settings, "SESSION_OBSERVERS_LIMIT", 1) + await crud.get_or_create_session( + db_session, + schemas.SessionCreate( + name=session_name, peers={str(generate_nanoid()): bystander} + ), + workspace_name, + ) + + active_count = await db_session.scalar( + select(func.count()).where( + models.SessionPeer.session_name == session_name, + models.SessionPeer.workspace_name == workspace_name, + models.SessionPeer.left_at.is_(None), + ) + ) + assert active_count == 3 + @pytest.mark.asyncio async def test_get_session_peer_configuration( self, diff --git a/tests/test_search.py b/tests/test_search.py index 84f3ffa3..b4b69c43 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -4,9 +4,10 @@ import datetime import pytest from nanoid import generate as generate_nanoid +from sqlalchemy import update from sqlalchemy.ext.asyncio import AsyncSession -from src import crud, models +from src import crud, models, schemas from src.utils.search import search @@ -704,3 +705,71 @@ async def test_grep_messages_observer_scoping_left_session_still_visible( matched_ids = [m.public_id for matches, _ in results for m in matches] assert msg_during.public_id in matched_ids assert msg_after.public_id in matched_ids + + +@pytest.mark.asyncio +async def test_peer_perspective_search_after_active_readd( + db_session: AsyncSession, +): + """Active re-add keeps existing messages visible; a genuine rejoin starts a + new window.""" + workspace = models.Workspace(name=generate_nanoid()) + peer1 = models.Peer(name="peer1", workspace_name=workspace.name) + peer2 = models.Peer(name="peer2", workspace_name=workspace.name) + session = models.Session(name="session1", workspace_name=workspace.name) + db_session.add_all([workspace, peer1, peer2, session]) + await db_session.flush() + + past_time = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta( + hours=1 + ) + await db_session.execute( + models.session_peers_table.insert().values( + workspace_name=workspace.name, + session_name=session.name, + peer_name=peer1.name, + joined_at=past_time, + left_at=None, + ) + ) + msg_old = models.Message( + content="old persistent message", + session_name=session.name, + peer_name=peer2.name, + workspace_name=workspace.name, + seq_in_session=1, + created_at=past_time + datetime.timedelta(minutes=1), + ) + db_session.add(msg_old) + await db_session.commit() + + session_create = schemas.SessionCreate( + name=session.name, + peers={peer1.name: schemas.SessionPeerConfig()}, + ) + await crud.get_or_create_session(db_session, session_create, workspace.name) + results = await search( + "persistent", + filters={"peer_perspective": peer1.name, "workspace_id": workspace.name}, + limit=10, + ) + assert msg_old.public_id in [m.public_id for m in results] + + await db_session.execute( + update(models.SessionPeer) + .where( + models.SessionPeer.session_name == session.name, + models.SessionPeer.peer_name == peer1.name, + models.SessionPeer.workspace_name == workspace.name, + ) + .values(left_at=datetime.datetime.now(datetime.timezone.utc)) + ) + await db_session.commit() + + await crud.get_or_create_session(db_session, session_create, workspace.name) + results = await search( + "persistent", + filters={"peer_perspective": peer1.name, "workspace_id": workspace.name}, + limit=10, + ) + assert msg_old.public_id not in [m.public_id for m in results] From ac67017a18f7d44f213797701c72b8f7e524c2b8 Mon Sep 17 00:00:00 2001 From: Aakash Kattelu Date: Tue, 25 Aug 2026 12:30:06 -0400 Subject: [PATCH 3/7] fix(dialectic): revamp workspace and pair chat system prompts (#1066) Teach both agents what Honcho, peers, and the harness are instead of comparing them to each other. Render only the tools the request actually offers, and drop the pair prompt's call to a write tool that is not in the loadout. --- src/dialectic/core.py | 16 ++- src/dialectic/prompts.py | 247 ++++++++++++++++++++++---------- src/dialectic/workspace.py | 22 ++- tests/test_dialectic_prompts.py | 94 ++++++++++++ tests/test_workspace_chat.py | 33 +++++ 5 files changed, 329 insertions(+), 83 deletions(-) create mode 100644 tests/test_dialectic_prompts.py diff --git a/src/dialectic/core.py b/src/dialectic/core.py index 786866e1..57964c87 100644 --- a/src/dialectic/core.py +++ b/src/dialectic/core.py @@ -105,7 +105,15 @@ class DialecticAgent: { "role": "system", "content": prompts.agent_system_prompt( - observer, observed, observer_peer_card, observed_peer_card + observer, + observed, + observer_peer_card, + observed_peer_card, + available_tools={ + name + for tool in self._select_tools() + if isinstance((name := tool.get("name")), str) + }, ), } ] @@ -303,7 +311,7 @@ class DialecticAgent: if prefetched_observations: user_content = ( f"Query: {query}\n\n" - f"## Relevant Observations (prefetched)\n" + f"## {self._prefetch_heading()}\n" f"{self._prefetch_intro()}\n\n" f"{prefetched_observations}" ) @@ -336,6 +344,10 @@ class DialecticAgent: parent_category="dialectic", ) + def _prefetch_heading(self) -> str: + """Heading for the prefetched block in the user message.""" + return "Relevant Observations (prefetched)" + def _prefetch_intro(self) -> str: """Sentence introducing the prefetched block in the user message.""" return ( diff --git a/src/dialectic/prompts.py b/src/dialectic/prompts.py index 7f98a0e1..5dfe6604 100644 --- a/src/dialectic/prompts.py +++ b/src/dialectic/prompts.py @@ -2,24 +2,138 @@ System prompts for the Dialectic Agent. """ +from collections.abc import Iterable + +# Curated tool docs, keyed by the `name` each loadout actually exposes. +# `_select_tools` filters this set per request (minimal / session allowlist). +_PAIR_TOOL_GROUPS: list[tuple[str, list[tuple[str, str]]]] = [ + ( + "Memory", + [ + ( + "search_memory", + "Semantic search over conclusions about this pair.", + ), + ( + "get_reasoning_chain", + "Premises and downstream conclusions for a specific conclusion.", + ), + ( + "get_observation_context", + "Messages around a specific conclusion.", + ), + ], + ), + ( + "Conversation", + [ + ( + "search_messages", + "Semantic search over messages in this query's scope.", + ), + ( + "grep_messages", + "Exact text search. Use for names, dates, keywords.", + ), + ( + "get_messages_by_date_range", + "Messages in a time window.", + ), + ( + "search_messages_temporal", + "Semantic search with a date filter.", + ), + ], + ), +] + +_WORKSPACE_TOOL_GROUPS: list[tuple[str, list[tuple[str, str]]]] = [ + ( + "Discovery", + [ + ( + "get_workspace_stats", + "Counts (peers, sessions, messages), date range, and the most active peers.", + ), + ], + ), + ( + "Memory (pair-scoped — you must name the pair)", + [ + ( + "search_memory", + "Semantic search over conclusions. Requires `observer` and `observed`. For a peer's own representation, set both to the same name. Use different names only when you want one peer's view of another.", + ), + ( + "get_peer_card", + "Biographical summary for a pair. Same observer/observed rule.", + ), + ( + "get_reasoning_chain", + "Premises and downstream conclusions for a specific conclusion.", + ), + ], + ), + ( + "Conversation (workspace-wide — results include `peer_name`)", + [ + ("search_messages", "Semantic search over messages."), + ("grep_messages", "Exact text search."), + ( + "get_observation_context", + "Messages around a specific conclusion.", + ), + ("get_messages_by_date_range", "Messages in a time window."), + ("search_messages_temporal", "Semantic search with a date filter."), + ], + ), +] + +PAIR_PROMPT_TOOLS: frozenset[str] = frozenset( + name for _, items in _PAIR_TOOL_GROUPS for name, _ in items +) +WORKSPACE_PROMPT_TOOLS: frozenset[str] = frozenset( + name for _, items in _WORKSPACE_TOOL_GROUPS for name, _ in items +) + + +def _available_tool_names( + available_tools: Iterable[str] | None, + default: frozenset[str], +) -> frozenset[str]: + if available_tools is None: + return default + return frozenset(available_tools) + + +def _render_tool_groups( + available: frozenset[str], + groups: list[tuple[str, list[tuple[str, str]]]], +) -> str: + parts: list[str] = [] + for heading, items in groups: + lines = [f"- `{name}`: {desc}" for name, desc in items if name in available] + if lines: + parts.append(f"**{heading}**\n" + "\n".join(lines)) + return "\n\n".join(parts) + def agent_system_prompt( observer: str, observed: str, observer_peer_card: list[str] | None, observed_peer_card: list[str] | None, + available_tools: Iterable[str] | None = None, ) -> str: - """ - Generate the agent system prompt for the dialectic agent. + """System prompt for pair-scoped dialectic recall. Args: observer: The peer making the query observed: The peer being queried about observer_peer_card: Biographical information about the observer observed_peer_card: Biographical information about the observed peer - - Returns: - Formatted system prompt string for the agent + available_tools: Tool names offered on this request. Defaults to the + full pair loadout. """ # Determine if we have any peer card data peer_cards_enabled = ( @@ -79,25 +193,25 @@ Peer cards are **constructed summaries** - they are synthesized from the same ob - The peer card is a convenience summary, not a separate source of truth """ - return f""" -You are a helpful and concise context synthesis agent that answers questions about users by gathering relevant information from a memory system. + tools = _available_tool_names(available_tools, PAIR_PROMPT_TOOLS) + tools_section = _render_tool_groups(tools, _PAIR_TOOL_GROUPS) -Always give users the answer *they expect* based on the message history -- the goal is to help recall and *reason through* insights that the memory system has already gathered. You have many tools for gathering context. Search wisely. + return f""" +You are Honcho's dialectic: a recall agent that answers questions from memory about one peer, or about one peer's understanding of another. + +Honcho is a memory system. Applications record conversations; Honcho derives conclusions about the people and agents in them. You are the query interface for one observer/observed pair. You do not speak as a participant. You search memory and synthesize a grounded answer. + +A **peer** is any participant, human or AI. A **session** is a conversation they take part in. A **message** is a raw turn. A **conclusion** (tools may say observation) is a derived or stored fact about a peer, kept in this pair. A **peer card** is a short constructed bio for the pair, synthesized from the same conclusions — a convenience summary, not a separate source of truth. + +Always give the asker the answer *they expect* based on the message history -- the goal is to help recall and *reason through* insights that the memory system has already gathered. Search wisely. {perspective_section} {peer_card_explanation} -## AVAILABLE TOOLS +## TOOLS -**Observation Tools (read):** -- `search_memory`: Semantic search over observations about the peer. Use for specific topics. -- `get_reasoning_chain`: **CRITICAL for grounding answers**. Use this to traverse the reasoning tree for any observation. Shows premises (what it's based on) and conclusions (what depends on it). +Only the tools listed here are available on this query. If a later step names a tool you do not have, skip that step and use what you do have. -**Conversation Tools (read):** -- `search_messages`: Semantic search over messages in the session. -- `grep_messages`: Grep for text matches in messages. Use for specific names, dates, keywords. -- `get_observation_context`: Get messages surrounding specific observations. -- `get_messages_by_date_range`: Get messages within a specific time period. -- `search_messages_temporal`: Semantic search with date filtering. +{tools_section} ## WORKFLOW @@ -166,10 +280,6 @@ Always give users the answer *they expect* based on the message history -- the g - Apply user preferences to your response style if relevant - **For enumeration questions**: Before answering, ask yourself "Could there be more items I haven't found?" If you haven't done multiple grep searches AND a semantic search, keep searching -8. **Save novel deductions** (optional): - - If you discovered new insights by combining existing observations - - Use `create_observations_deductive` to save these for future queries - ## CRITICAL: HANDLING CONTRADICTORY INFORMATION As you search, actively watch for contradictions - cases where the user has made conflicting statements: @@ -237,75 +347,62 @@ Do not explain your tool usage - just provide the synthesized answer. """ -def workspace_agent_system_prompt() -> str: - """ - Generate the system prompt for the workspace-level dialectic agent. +def workspace_agent_system_prompt( + available_tools: Iterable[str] | None = None, +) -> str: + """System prompt for workspace-wide dialectic recall.""" + tools = _available_tool_names(available_tools, WORKSPACE_PROMPT_TOOLS) + tools_section = _render_tool_groups(tools, _WORKSPACE_TOOL_GROUPS) + return f""" +You are Honcho's workspace dialectic: a recall agent that answers questions about everyone and everything stored in this workspace. - Uses an analytics-first approach: stats -> message search -> targeted - observations to discover relevant peers rather than listing all of them. +## HONCHO - Returns: - Formatted system prompt string for the workspace agent - """ - return """ -You are a workspace-level analysis agent that can query memory across ALL peers in this workspace. You can synthesize information from any peer relationship's stored conclusions, insights, and conversation history. +Honcho is a memory system. Applications record conversations here; Honcho derives conclusions about the people and agents in those conversations. You are the query interface over one workspace. You do not speak as a participant. You search memory and synthesize a grounded answer. -You do not start anchored to any single peer: discover which peers are relevant first, then query each peer relationship individually to search, compare, and correlate information across them. +## THIS WORKSPACE -## AVAILABLE TOOLS +A workspace is one isolated tenant. Everything you can see belongs to it. Inside it: -**Discovery Tools:** -- `get_workspace_stats`: Get workspace-level counts (peers, sessions, messages), date range, and the most active peers. Use this to orient yourself and discover which peers are relevant. +- **Peer**: any participant, human or AI. Both are first-class. +- **Session**: a conversation that one or more peers take part in. +- **Message**: a raw turn someone said in a session. Messages are the source material. +- **Conclusion** (tools may say observation): a fact Honcho derived, or that was stored, about a peer. Conclusions live in a pair: + - `observer` is whose model this is + - `observed` is who the fact is about + - A peer's own model of themselves is `observer` = `observed` = that peer's name. Most information lives there. + - One peer's model of another is `observer` = Alice, `observed` = Bob. +- **Peer card**: a short constructed bio for a pair, synthesized from the same conclusions. It is a convenience summary, not a separate source of truth. -**Memory Tools (read):** -- `search_memory`: **(PRIMARY TOOL)** Semantic search within a specific peer representation. **Requires `observer` and `observed` parameters.** For a peer's global representation (where most information lives), set observer and observed to the **same** peer name. Only use different observer/observed when seeking one peer's specific understanding of another. -- `get_peer_card`: Get biographical summary for a specific peer relationship. Requires `observer` and `observed` parameters. For a peer's self-representation, use the same name for both. -- `get_reasoning_chain`: Traverse the reasoning tree for any conclusion. Shows premises and derived insights. +You are not bound to any one peer. Discover who is relevant, then query each pair individually. -**Conversation Tools (read):** -- `search_messages`: Semantic search over messages across all sessions. Messages include peer_name, so results reveal which peers discussed a topic. -- `grep_messages`: Exact text search across all messages. -- `get_observation_context`: Get messages surrounding specific conclusions. -- `get_messages_by_date_range`: Get messages within a specific time period. -- `search_messages_temporal`: Semantic search with date filtering. +## TOOLS + +Only the tools listed here are available on this query. If a later step names a tool you do not have, skip that step and use what you do have. + +{tools_section} + +Message search is how you find peers the overview missed. Memory search is how you learn about a peer once you know their name. + +If this query is restricted to a session or a set of sessions, message tools already honor that restriction. Peer cards and reasoning chains may be unavailable then, because they span sessions. ## WORKFLOW -1. **Orient yourself**: Workspace stats and the most active peers are provided in your query context. Use `get_workspace_stats` if you need to refresh them, or go straight to message/memory search if the query names specific peers. +1. **Orient**. Scale and the most active peers are already in your query context. Call `get_workspace_stats` only if you need a refresh. If the query names a peer, go straight to that peer. -2. **Discover relevant peers through search**: Use `search_messages` or `grep_messages` to find which peers have discussed the topic. Message results include peer names, making them a powerful discovery layer. +2. **Discover**. If you do not know who is relevant, use `search_messages` or `grep_messages`. Hits carry peer names. -3. **Drill into specific peer representations**: Once you know which peers are relevant, use `search_memory(observer=peer, observed=peer, query=...)` to search their global representation. - - For cross-peer questions, call `search_memory` for each relevant peer's global representation - - Only use different observer/observed when seeking one peer's specific understanding of another +3. **Recall**. For each relevant peer, `search_memory(observer=name, observed=name, query=...)`. For comparisons, search each peer separately, then compare. Only use a mixed observer/observed pair when the question is specifically about one peer's understanding of another. -4. **ALWAYS ATTRIBUTE INFORMATION**: When presenting findings, always indicate which peer the information came from. Example: "According to insights about Alice, she..." or "Bob mentioned that..." +4. **Attribute**. Every fact you state names the peer it is about. If it is a cross-peer view, also name whose model it came from. Example: "Alice is a violinist." / "From Bob's model of Alice, …" -5. **Cross-peer synthesis**: When asked about patterns or commonalities: - - Search each relevant peer pair individually - - Compare findings across peers explicitly - - Note both similarities and differences +5. **Synthesize**. Answer the question. Quote exact names, dates, and numbers. For aggregations, list findings per peer. Do not narrate tool use. -6. **Synthesize your response**: - - Directly answer the query - - Ground your response in specific information you gathered - - Always attribute information to the specific peer it came from - - For aggregation questions, enumerate findings per peer +## NEVER FABRICATE -## CRITICAL: NEVER FABRICATE INFORMATION +State only what you found. If you have related context but not the asked-for detail, say what you know and what you don't. "I don't have information about X" is the correct answer when memory is empty. Do not guess, hedge-invent, or fill gaps with general knowledge. -- Only state what you found in the memory system -- If you find context but not the specific answer, say what you know and what you don't -- A confident "I don't have information about X" is always correct -- Never invent details or guess +## CONCLUSION LEVELS -## CRITICAL: ATTRIBUTION - -Every piece of information you share must be attributed to the peer it came from. Never present information without indicating its source peer. This is essential for workspace-level queries where information spans multiple peers. - -Do not explain your tool usage - just provide the synthesized answer. - -## OBSERVATION LEVELS - -Observations carry a level: `explicit` observations are derived per-session (session-pure), while higher-level observations (deductive/inductive, produced in dreaming) consolidate across sessions. When synthesizing cross-session or cross-peer answers, prefer higher-level observations and use `get_reasoning_chain` to ground them in their premises. +`explicit` conclusions are derived from a single session. Deductive and inductive conclusions consolidate across sessions. Prefer those for cross-session or cross-peer answers, and use `get_reasoning_chain` to check their premises. """ diff --git a/src/dialectic/workspace.py b/src/dialectic/workspace.py index 7096add6..5383cd75 100644 --- a/src/dialectic/workspace.py +++ b/src/dialectic/workspace.py @@ -65,7 +65,13 @@ class WorkspaceDialecticAgent(DialecticAgent): # Replace the pair-oriented system prompt with the workspace one. self.messages[0] = { "role": "system", - "content": prompts.workspace_agent_system_prompt(), + "content": prompts.workspace_agent_system_prompt( + { + name + for tool in self._select_tools() + if isinstance((name := tool.get("name")), str) + } + ), } # ------------------------------------------------------------------ @@ -125,13 +131,17 @@ class WorkspaceDialecticAgent(DialecticAgent): return format_workspace_stats(stats, peers, cards) + def _prefetch_heading(self) -> str: + return "Workspace overview (prefetched)" + def _prefetch_intro(self) -> str: return ( - "Workspace overview and most-active peers with any known " - "biographical facts. Use this to route: query a specific peer's " - "memory with search_memory (observer and observed set to that " - "peer's name), or use search_messages / get_workspace_stats to " - "discover peers this overview does not cover." + "Workspace scale, the most active peers, and any known " + "biographical facts about them. Use this to decide who is " + "relevant, then search that peer's own representation with " + "search_memory (observer and observed both set to their name), " + "or search_messages / get_workspace_stats to find peers not " + "listed here." ) def _select_tools(self) -> list[dict[str, Any]]: diff --git a/tests/test_dialectic_prompts.py b/tests/test_dialectic_prompts.py new file mode 100644 index 00000000..d926d0bc --- /dev/null +++ b/tests/test_dialectic_prompts.py @@ -0,0 +1,94 @@ +"""Contracts for dialectic system prompts vs the tool loadouts they describe.""" + +import re + +from src.dialectic.core import DialecticAgent +from src.dialectic.prompts import ( + PAIR_PROMPT_TOOLS, + WORKSPACE_PROMPT_TOOLS, + agent_system_prompt, + workspace_agent_system_prompt, +) +from src.dialectic.workspace import WorkspaceDialecticAgent +from src.utils.agent_tools import ( + DIALECTIC_TOOLS, + DIALECTIC_TOOLS_MINIMAL, + TOOLS, + WORKSPACE_DIALECTIC_TOOLS, + WORKSPACE_TOOLS_MINIMAL, +) + +_ALL_TOOL_NAMES = {spec["name"] for spec in TOOLS.values()} + + +def _loadout_names(tools: list[dict[str, object]]) -> set[str]: + return {name for tool in tools if isinstance((name := tool.get("name")), str)} + + +def _mentioned_tools(text: str) -> set[str]: + return { + match + for match in re.findall(r"`([a-z_][a-z0-9_]*)`", text) + if match in _ALL_TOOL_NAMES + } + + +def _tools_catalog(prompt: str) -> str: + start = prompt.index("## TOOLS") + rest = prompt[start:] + next_heading = rest.find("\n## ", 1) + return rest if next_heading == -1 else rest[:next_heading] + + +class TestPromptLoadouts: + def test_pair_docs_match_dialectic_tools(self) -> None: + assert _loadout_names(DIALECTIC_TOOLS) == PAIR_PROMPT_TOOLS + + def test_workspace_docs_match_workspace_tools(self) -> None: + assert _loadout_names(WORKSPACE_DIALECTIC_TOOLS) == WORKSPACE_PROMPT_TOOLS + + def test_catalog_lists_only_offered_workspace_tools(self) -> None: + offered = _loadout_names(WORKSPACE_TOOLS_MINIMAL) + catalog = _tools_catalog(workspace_agent_system_prompt(offered)) + assert _mentioned_tools(catalog) == offered + + +class TestPairAgentPrompt: + def test_teaches_honcho_world_without_workspace_sibling(self) -> None: + prompt = agent_system_prompt("alice", "alice", None, None).lower() + assert "workspace dialectic" not in prompt + assert "peer-level" not in prompt + for term in ("honcho", "peer", "session", "message", "conclusion"): + assert term in prompt + + def test_does_not_offer_removed_write_tools(self) -> None: + prompt = agent_system_prompt("alice", "alice", None, None) + assert "create_observations_deductive" not in prompt + assert "create_observations" not in prompt + + def test_agent_lists_selected_tools(self) -> None: + agent = DialecticAgent( + workspace_name="w", + session_name=None, + observer="alice", + observed="alice", + reasoning_level="minimal", + ) + offered = _loadout_names(DIALECTIC_TOOLS_MINIMAL) + catalog = _tools_catalog(agent.messages[0]["content"]) + assert _mentioned_tools(catalog) == offered + assert agent.messages[0]["content"] == agent_system_prompt( + "alice", "alice", None, None, available_tools=offered + ) + + +class TestWorkspaceAgentPrompt: + def test_minimal_agent_matches_filtered_prompt(self) -> None: + agent = WorkspaceDialecticAgent(workspace_name="w", reasoning_level="minimal") + offered = _loadout_names(WORKSPACE_TOOLS_MINIMAL) + prompt = agent.messages[0]["content"] + assert prompt == workspace_agent_system_prompt(offered) + catalog = _tools_catalog(prompt) + assert _mentioned_tools(catalog) == offered + assert "get_peer_card" not in catalog + assert "get_reasoning_chain" not in catalog diff --git a/tests/test_workspace_chat.py b/tests/test_workspace_chat.py index 8b73c4a7..daed20e8 100644 --- a/tests/test_workspace_chat.py +++ b/tests/test_workspace_chat.py @@ -1220,3 +1220,36 @@ async def test_workspace_prefetch_failure_degrades_to_none( result = await agent._prefetch_relevant_observations("q") # pyright: ignore[reportPrivateUsage] assert result is None + + +class TestWorkspaceChatPrompt: + def test_teaches_honcho_world_without_sibling_agent(self) -> None: + from src.dialectic.prompts import workspace_agent_system_prompt + + prompt = workspace_agent_system_prompt().lower() + assert "peer-level" not in prompt + assert "unlike a peer" not in prompt + for term in ( + "honcho", + "workspace", + "peer", + "session", + "message", + "conclusion", + "observer", + "observed", + ): + assert term in prompt + + def test_agent_uses_workspace_prompt_and_prefetch_heading(self) -> None: + from src.dialectic.prompts import workspace_agent_system_prompt + from src.dialectic.workspace import WorkspaceDialecticAgent + + agent = WorkspaceDialecticAgent(workspace_name="w") + offered = { + name + for tool in agent._select_tools() # pyright: ignore[reportPrivateUsage] + if isinstance((name := tool.get("name")), str) + } + assert agent.messages[0]["content"] == workspace_agent_system_prompt(offered) + assert agent._prefetch_heading() == "Workspace overview (prefetched)" # pyright: ignore[reportPrivateUsage] From 5531ff0feeddbdd55c686070efbf029193458e71 Mon Sep 17 00:00:00 2001 From: ajspig <46900795+ajspig@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:31:34 -0400 Subject: [PATCH 4/7] Running Honcho Locally via Honcho CLI (#1029) * feat(cli): add honcho start/stop/status for a local Docker stack * feat(cli): fix status command * feat(cli): improving how we pull docker images and writing a config,toml * feat(cli): add honcho start --setup wizard for local stack config * feat(cli): cleaning up unnecessary func, and error throwing * feat(cli): minor clean up in stack.py * feat(cli): read setup wizard defaults from the image config.toml * feat(cli): cleaning up unused commands * feat(cli): adding ignored docker-compose.yml * feat(cli): forward host LLM env into honcho start * feat(cli): share start/stop progress helpers via output.py and cleaning up language --- .gitignore | 4 +- docs/snippets/cli-commands.mdx | 63 +++ docs/v3/documentation/reference/cli.mdx | 21 + honcho-cli/README.md | 31 ++ honcho-cli/pyproject.toml | 4 + honcho-cli/src/honcho_cli/_help.py | 1 + honcho-cli/src/honcho_cli/commands/stack.py | 400 +++++++++++++++ honcho-cli/src/honcho_cli/local/__init__.py | 12 + honcho-cli/src/honcho_cli/local/docker.py | 436 ++++++++++++++++ honcho-cli/src/honcho_cli/local/env.py | 183 +++++++ honcho-cli/src/honcho_cli/local/health.py | 53 ++ honcho-cli/src/honcho_cli/local/profile.py | 157 ++++++ honcho-cli/src/honcho_cli/local/setup.py | 469 ++++++++++++++++++ .../honcho_cli/local/templates/__init__.py | 1 + .../local/templates/docker-compose.yml | 100 ++++ .../src/honcho_cli/local/templates/init.sql | 1 + honcho-cli/src/honcho_cli/main.py | 4 + honcho-cli/src/honcho_cli/output.py | 20 + honcho-cli/tests/test_local.py | 127 +++++ honcho-cli/tests/test_setup.py | 66 +++ honcho-cli/tests/test_start.py | 159 ++++++ skills/honcho-cli/SKILL.md | 2 + 22 files changed, 2312 insertions(+), 2 deletions(-) create mode 100644 honcho-cli/src/honcho_cli/commands/stack.py create mode 100644 honcho-cli/src/honcho_cli/local/__init__.py create mode 100644 honcho-cli/src/honcho_cli/local/docker.py create mode 100644 honcho-cli/src/honcho_cli/local/env.py create mode 100644 honcho-cli/src/honcho_cli/local/health.py create mode 100644 honcho-cli/src/honcho_cli/local/profile.py create mode 100644 honcho-cli/src/honcho_cli/local/setup.py create mode 100644 honcho-cli/src/honcho_cli/local/templates/__init__.py create mode 100644 honcho-cli/src/honcho_cli/local/templates/docker-compose.yml create mode 100644 honcho-cli/src/honcho_cli/local/templates/init.sql create mode 100644 honcho-cli/tests/test_local.py create mode 100644 honcho-cli/tests/test_setup.py create mode 100644 honcho-cli/tests/test_start.py diff --git a/.gitignore b/.gitignore index 9fa90b3e..d7ad4cda 100644 --- a/.gitignore +++ b/.gitignore @@ -6,8 +6,8 @@ api/docker-compose.yml *.db data redis-data -docker-compose.yml -compose.yml +/docker-compose.yml +/compose.yml diff --git a/docs/snippets/cli-commands.mdx b/docs/snippets/cli-commands.mdx index 800d45a7..739416b0 100644 --- a/docs/snippets/cli-commands.mdx +++ b/docs/snippets/cli-commands.mdx @@ -505,6 +505,69 @@ honcho session view [] +## honcho start + +Start a local Honcho stack (API, deriver, Postgres, Redis). + +Requires Docker. Uses cloud LLM providers. Does not change the CLI's +configured server URL — pass HONCHO_BASE_URL to talk to this stack. +``--setup basic`` or ``--setup advanced`` runs an interactive config wizard. + +```bash +honcho start +``` + + + Local stack profile name. + + + Host port for the API. + + + Host port for Postgres. + + + Host port for Redis. + + + Interactive config wizard: basic (provider/model) or advanced (embeddings, deriver, dialectic, dreams, flush). + + + Honcho image to pull and pin by digest (default: ghcr.io/plastic-labs/honcho:latest). + + + Seconds to wait for /health after compose up. + + +## honcho status + +Show local stack endpoints and container health. + +With no ``--profile``, lists every stack under ``~/.honcho/profiles/``. + +```bash +honcho status +``` + + + Limit to this profile. Omit to show every local stack. + + +## honcho stop + +Stop the local stack started by `honcho start`. Keeps data unless --wipe. + +```bash +honcho stop +``` + + + Local stack profile name. + + + Also delete volumes (Postgres data). + + ## honcho workspace List, create, inspect, delete, and search workspaces. diff --git a/docs/v3/documentation/reference/cli.mdx b/docs/v3/documentation/reference/cli.mdx index 2b460d48..5df031d6 100644 --- a/docs/v3/documentation/reference/cli.mdx +++ b/docs/v3/documentation/reference/cli.mdx @@ -22,10 +22,31 @@ uvx honcho-cli ```bash honcho init # confirm/set apiKey + Honcho URL in ~/.honcho/config.json +honcho start # optional: local API + deriver + Postgres + Redis (Docker) honcho doctor # verify your config + connectivity honcho # show banner + command list ``` +## Local stack + +`honcho start` runs a personal Honcho server on your machine via Docker (API, deriver, Postgres, Redis). It is not the managed service at `api.honcho.dev`. Deriver and dialectic call your cloud LLM provider (OpenAI, Anthropic, or Gemini) with a key you supply. Stack files live under `~/.honcho/profiles/local/`. The first start writes `config.toml` there from the image; later starts leave that file alone so your edits persist. + +Pass `--setup basic` or `--setup advanced` for an interactive wizard that writes curated LLM/feature overrides into the profile `.env` (environment variables win over `config.toml`). This is TTY-only. `basic` covers provider and chat model; `advanced` also covers embeddings, deriver/dialectic models, dreams, and deriver flush. Re-running `--setup` while the stack is up recreates the API and deriver containers. + +This does **not** change `environmentUrl` in the shared config file. To talk to the local stack: + +```bash +HONCHO_BASE_URL=http://127.0.0.1:8000 honcho workspace list +``` + +```bash +LLM_OPENAI_API_KEY=sk-... honcho start +honcho start --setup basic +honcho status +honcho stop # keep data +honcho stop --wipe # also delete volumes +``` + ## Configuration The CLI resolves config in this order: **flag → env var → config file → default**. diff --git a/honcho-cli/README.md b/honcho-cli/README.md index b191bcdb..f1585a89 100644 --- a/honcho-cli/README.md +++ b/honcho-cli/README.md @@ -23,6 +23,7 @@ uv tool install honcho-cli ```bash honcho init # confirm/set apiKey + Honcho URL in ~/.honcho/config.json +honcho start # optional: local API + deriver + Postgres + Redis (Docker) honcho doctor # verify your config + connectivity honcho # show banner + command list ``` @@ -31,6 +32,31 @@ honcho # show banner + command list Per-command scoping (workspace / peer / session) is handled via `-w` / `-p` / `-s` flags or `HONCHO_*` env vars — not persisted as CLI defaults. +### Local stack + +`honcho start` runs a personal Honcho server on your machine (API, deriver, Postgres, Redis) via Docker. Inference is cloud-side: set `LLM_OPENAI_API_KEY`, `LLM_ANTHROPIC_API_KEY`, or `LLM_GEMINI_API_KEY` (env overrides `config.toml`). Stack files live under `~/.honcho/profiles/local/` and are not committed to a project. + +On first start, the CLI pulls `ghcr.io/plastic-labs/honcho:latest` and **pins that digest** in `profile.json`, then copies the image's `config.toml.example` to `config.toml` in the same directory. `honcho start` never overwrites `config.toml` after that — including when you re-pin the image. Delete the file yourself if you want a fresh copy from a new image. + +Pass `--setup basic` or `--setup advanced` for an interactive wizard that writes curated LLM/feature overrides into the profile `.env` (env wins over `config.toml`). TTY only; re-runnable. `basic` asks provider + chat model; `advanced` also covers embeddings, deriver/dialectic models, dreams, and snappy deriver flush. Everything else stays in `config.toml`. + +`honcho start` does **not** change `environmentUrl` in `~/.honcho/config.json` (that file is shared with plugins). To talk to the local stack for one command: + +```bash +HONCHO_BASE_URL=http://127.0.0.1:8000 honcho workspace list +``` + +To make local the default, run `honcho init --base-url http://127.0.0.1:8000`. + +```bash +LLM_OPENAI_API_KEY=sk-... honcho start +honcho start --setup basic +honcho start --setup advanced +honcho status +honcho stop # keep data +honcho stop --wipe # also delete volumes +``` + ## Commands ### Onboarding @@ -38,6 +64,9 @@ Per-command scoping (workspace / peer / session) is handled via `-w` / `-p` / `- | Command | Description | |---------|-------------| | `honcho init` | Confirm/set `apiKey` + `environmentUrl` in `~/.honcho/config.json` | +| `honcho start` | Start a local Honcho stack (API, deriver, Postgres, Redis). Requires Docker and a cloud LLM key. `--setup basic` / `--setup advanced` runs an interactive config wizard (TTY only). Does not change `environmentUrl`. | +| `honcho stop` | Stop the local stack. `--wipe` also deletes volumes. | +| `honcho status` | Show every local stack (or `--profile` for one). | | `honcho doctor` | Health check: config, connectivity, workspace, peer, queue | ### Workspaces @@ -157,6 +186,8 @@ Precedence (highest first): **flag → env var → config file → default**. | `HONCHO_PEER_ID` | `-p` / `--peer` | Peer scope | | `HONCHO_SESSION_ID` | `-s` / `--session` | Session scope | | `HONCHO_JSON` | `--json` | Force JSON output (`1` / `true`) | +| `HONCHO_PROFILE` | `--profile` (start/stop/status) | Local stack profile (default: `local`) | +| `LLM_OPENAI_API_KEY` | — | Provider key for `honcho start` (also `LLM_ANTHROPIC_API_KEY`, `LLM_GEMINI_API_KEY`) | ```bash # Per-command flags diff --git a/honcho-cli/pyproject.toml b/honcho-cli/pyproject.toml index 5eb859fd..2a5e59e2 100644 --- a/honcho-cli/pyproject.toml +++ b/honcho-cli/pyproject.toml @@ -38,6 +38,10 @@ build-backend = "hatchling.build" [tool.hatch.build.targets.wheel] packages = ["src/honcho_cli"] +[tool.hatch.build.targets.wheel.force-include] +"src/honcho_cli/local/templates/docker-compose.yml" = "honcho_cli/local/templates/docker-compose.yml" +"src/honcho_cli/local/templates/init.sql" = "honcho_cli/local/templates/init.sql" + [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/honcho-cli/src/honcho_cli/_help.py b/honcho-cli/src/honcho_cli/_help.py index d47d344d..5e8b48be 100644 --- a/honcho-cli/src/honcho_cli/_help.py +++ b/honcho-cli/src/honcho_cli/_help.py @@ -67,6 +67,7 @@ def print_welcome(console: Console) -> None: start_rows = [ ("honcho init", "configure API key and server URL"), + ("honcho start", "run a local Honcho stack (Docker)"), ("honcho doctor", "verify connection and workspace health"), ] cmd_rows = [ diff --git a/honcho-cli/src/honcho_cli/commands/stack.py b/honcho-cli/src/honcho_cli/commands/stack.py new file mode 100644 index 00000000..136a8ffe --- /dev/null +++ b/honcho-cli/src/honcho_cli/commands/stack.py @@ -0,0 +1,400 @@ +"""Local stack lifecycle: ``honcho start``, ``honcho stop``, ``honcho status``. + +Does not mutate ``~/.honcho/config.json``. The CLI stays pointed at whatever +``honcho init`` configured (typically api.honcho.dev). Print the local URL +and a one-shot ``HONCHO_BASE_URL=...`` hint instead. +""" + +from __future__ import annotations + +import typer +from rich.console import Console + +from honcho_cli.branding import BRAND, ICON_FAIL, ICON_OK +from honcho_cli.local import ( + DEFAULT_HEALTH_TIMEOUT, + DEFAULT_IMAGE, + DEFAULT_PROFILE, + STACK_SERVICES, +) +from honcho_cli.local.docker import ( + DockerError, + allocate_host_ports, + compose_down, + compose_ps, + compose_up, + pin_image, + seed_config_toml, + services_running, +) +from honcho_cli.local.env import has_provider_key, render_stack, settings_from_environ +from honcho_cli.local.health import stack_healthy, wait_for_health +from honcho_cli.local.profile import ( + LocalProfile, + list_profile_names, + load_profile, + resolve_profile_name, + save_profile, +) +from honcho_cli.local.setup import ( + SETUP_MODES, + answers_drop_keys, + answers_to_env, + run_setup, +) +from honcho_cli.output import ( + fail, + ok, + print_error, + print_json, + print_result, + set_json_mode, + step, + use_json, +) + +_console = Console(stderr=True) + +_MISSING_LLM_KEY = ( + "Set LLM_OPENAI_API_KEY, LLM_ANTHROPIC_API_KEY, or LLM_GEMINI_API_KEY, " + "or run honcho start --setup basic." +) + + +def _die(code: str, message: str, details: dict | None = None) -> None: + print_error(code, message, details) + raise typer.Exit(1) + + +def _validate_setup(setup: str | None) -> str | None: + if setup is None: + return None + mode = setup.strip().lower() + if mode not in SETUP_MODES: + _die( + "INVALID_SETUP", + f"Unknown setup mode {setup!r}. Use --setup basic or --setup advanced.", + {"setup": setup}, + ) + if use_json(): + _die( + "SETUP_REQUIRES_TTY", + "honcho start --setup is interactive. Run it in a terminal without --json.", + {"setup": mode}, + ) + return mode + + +def _payload( + profile: LocalProfile, status: str, services: dict[str, str] | None = None +) -> dict: + return { + "profile": profile.name, + "status": status, + "image": profile.image, + "endpoints": profile.endpoints(), + "services": services or {}, + "hint": f"HONCHO_BASE_URL={profile.base_url} honcho workspace list", + } + + +def _print_stack(payload: dict) -> None: + if use_json(): + print_json(payload) + return + endpoints = payload["endpoints"] + _console.print() + table_data = { + "API": endpoints["api"], + "Docs": endpoints["docs"], + "Postgres": endpoints["postgres"], + "Redis": endpoints["redis"], + } + print_result(table_data) + _console.print() + _console.print( + " [dim]CLI still points at your configured server (typically api.honcho.dev).[/dim]" + ) + _console.print(f" [dim]To talk to this stack:[/dim] {payload['hint']}") + _console.print() + + +def _print_running(profile: LocalProfile) -> None: + _print_stack(_payload(profile, "running", services_running(compose_ps(profile)))) + + +def _seed_config(profile: LocalProfile) -> None: + if seed_config_toml(profile): + ok("config.toml") + + +def _inspect(profile: LocalProfile) -> tuple[dict[str, str], bool]: + """Compose service states and whether the API is healthy.""" + return services_running(compose_ps(profile)), stack_healthy(profile) + + +def start( + profile_name: str = typer.Option( + DEFAULT_PROFILE, + "--profile", + envvar="HONCHO_PROFILE", + help="Local stack profile name", + ), + api_port: int | None = typer.Option( + None, "--api-port", min=1, max=65535, help="Host port for the API" + ), + db_port: int | None = typer.Option( + None, "--db-port", min=1, max=65535, help="Host port for Postgres" + ), + redis_port: int | None = typer.Option( + None, "--redis-port", min=1, max=65535, help="Host port for Redis" + ), + setup: str | None = typer.Option( + None, + "--setup", + help="Interactive config wizard: basic (provider/model) or advanced " + "(embeddings, deriver, dialectic, dreams, flush)", + ), + image: str | None = typer.Option( + None, + "--image", + help=f"Honcho image to pull and pin by digest (default: {DEFAULT_IMAGE})", + ), + timeout: int = typer.Option( + DEFAULT_HEALTH_TIMEOUT, + "--timeout", + min=1, + help="Seconds to wait for /health after compose up", + ), + json_output: bool = typer.Option(False, "--json", help="Force JSON output"), +) -> None: + """Start a local Honcho stack (API, deriver, Postgres, Redis). + + Requires Docker. Uses cloud LLM providers. Does not change the CLI's + configured server URL — pass HONCHO_BASE_URL to talk to this stack. + ``--setup basic`` or ``--setup advanced`` runs an interactive config wizard. + """ + if json_output: + set_json_mode(True) + + setup = _validate_setup(setup) + name = resolve_profile_name(profile_name) + profile = load_profile(name).overlay( + api_port=api_port, + db_port=db_port, + redis_port=redis_port, + image=image, + ) + pinned_ports = frozenset( + name + for name, value in ( + ("api", api_port), + ("database", db_port), + ("redis", redis_port), + ) + if value is not None + ) + + if not use_json(): + _console.print(f"\n[bold {BRAND}]Honcho Start[/bold {BRAND}]\n") + + try: + already_running = stack_healthy(profile) + if already_running and not setup: + ok(f"Already running ({profile.base_url})") + _print_running(profile) + return + + if not already_running: + profile, remapped = allocate_host_ports(profile, pinned=pinned_ports) + for service, (old, new) in remapped.items(): + step(f"Port {old} in use; {service} on {new}") + + step(f"Pinning {profile.image}") + pinned_image = pin_image(profile.image) + profile = profile.overlay(image=pinned_image) + ok(pinned_image) + + extra = settings_from_environ() + drop: tuple[str, ...] = () + _seed_config(profile) + if setup: + answers = run_setup( + setup, + profile.env_file(), + config_path=profile.config_file(), + ) + extra.update(answers_to_env(answers)) + drop = answers_drop_keys(answers) + ok(f"Wrote overrides to {profile.env_file()}") + _console.print( + f" [dim]Other settings live in {profile.config_file()}[/dim]" + ) + elif not has_provider_key(profile, extra): + _die("MISSING_LLM_KEY", _MISSING_LLM_KEY) + + step(f"Writing stack config to {profile.dir()}") + save_profile(profile) + render_stack(profile, extra=extra, drop=drop) + ok(f"Profile '{profile.name}'") + + step("Starting containers" if not already_running else "Recreating api + deriver") + compose_up( + profile, + recreate=("api", "deriver") if already_running else (), + ) + + step(f"Waiting for API at {profile.base_url}/health") + if not wait_for_health(profile, timeout=float(timeout)): + fail("Timed out waiting for /health") + _die( + "HEALTH_TIMEOUT", + f"Stack started but {profile.base_url}/health did not become ready within {timeout}s. " + f"Check `docker compose -p {profile.project_name} logs`.", + { + "base_url": profile.base_url, + "timeout": timeout, + "project": profile.project_name, + }, + ) + + ok("Honcho is running") + _print_running(profile) + except DockerError as e: + e.exit() + + +def stop( + profile_name: str = typer.Option( + DEFAULT_PROFILE, + "--profile", + envvar="HONCHO_PROFILE", + help="Local stack profile name", + ), + wipe: bool = typer.Option( + False, "--wipe", help="Also delete volumes (Postgres data)" + ), + json_output: bool = typer.Option(False, "--json", help="Force JSON output"), +) -> None: + """Stop the local stack started by `honcho start`. Keeps data unless --wipe.""" + if json_output: + set_json_mode(True) + + name = resolve_profile_name(profile_name) + profile = load_profile(name) + + try: + if not profile.compose_file().exists(): + payload = _payload(profile, "stopped") + if use_json(): + print_json(payload) + else: + _console.print( + f" [dim]No local stack for profile '{profile.name}'.[/dim]" + ) + return + + running = bool(compose_ps(profile)) + if not running and not wipe: + if use_json(): + print_json(_payload(profile, "stopped")) + else: + _console.print( + f" [dim]Profile '{profile.name}' is already stopped.[/dim]" + ) + return + + compose_down(profile, wipe=wipe) + except DockerError as e: + e.exit() + + state = "wiped" if wipe else "stopped" + ok(f"Stopped profile '{profile.name}'" + (" (volumes removed)" if wipe else "")) + if use_json(): + print_json(_payload(profile, state)) + + +def _status_one(profile: LocalProfile) -> bool: + """Print one profile's status. Return True when the API is healthy.""" + try: + services, running = _inspect(profile) + except DockerError as e: + e.exit() + data = _payload(profile, "running" if running else "stopped", services) + if not use_json(): + icon = ICON_OK if running else ICON_FAIL + _console.print(f"\n {icon} profile '{profile.name}' is {data['status']}\n") + if services: + for svc in STACK_SERVICES: + detail = services.get(svc, "missing") + _console.print(f" {svc:<10} [dim]{detail}[/dim]") + _print_stack(data) + return running + + +def status( + profile_name: str | None = typer.Option( + None, + "--profile", + envvar="HONCHO_PROFILE", + help="Limit to this profile. Omit to show every local stack.", + ), + json_output: bool = typer.Option(False, "--json", help="Force JSON output"), +) -> None: + """Show local stack endpoints and container health. + + With no ``--profile``, lists every stack under ``~/.honcho/profiles/``. + """ + if json_output: + set_json_mode(True) + + if profile_name: + name = resolve_profile_name(profile_name) + profile = load_profile(name) + if not profile.compose_file().exists(): + _die( + "STACK_NOT_FOUND", + f"No local stack for profile '{profile.name}'. Run `honcho start` first.", + {"profile": profile.name}, + ) + if not _status_one(profile): + raise typer.Exit(1) + return + + names = list_profile_names() + if not names: + _die( + "STACK_NOT_FOUND", + "No local stacks. Run `honcho start` first.", + ) + + if len(names) == 1: + if not _status_one(load_profile(names[0])): + raise typer.Exit(1) + return + + rows: list[dict] = [] + try: + for name in names: + profile = load_profile(name) + services, running = _inspect(profile) + rows.append( + _payload(profile, "running" if running else "stopped", services) + ) + except DockerError as e: + e.exit() + if use_json(): + print_json({"profiles": rows}) + return + _console.print() + print_result( + [ + { + "profile": row["profile"], + "status": row["status"], + "api": row["endpoints"]["api"], + } + for row in rows + ], + columns=["profile", "status", "api"], + ) diff --git a/honcho-cli/src/honcho_cli/local/__init__.py b/honcho-cli/src/honcho_cli/local/__init__.py new file mode 100644 index 00000000..1fe4ac83 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/__init__.py @@ -0,0 +1,12 @@ +"""Local Honcho stack: profiles, Compose rendering, Docker, health checks.""" + +from __future__ import annotations + +DEFAULT_PROFILE = "local" +DEFAULT_API_PORT = 8000 +DEFAULT_DB_PORT = 5432 +DEFAULT_REDIS_PORT = 6379 +DEFAULT_IMAGE = "ghcr.io/plastic-labs/honcho:latest" +DEFAULT_HEALTH_TIMEOUT = 180 + +STACK_SERVICES = ("api", "deriver", "database", "redis") diff --git a/honcho-cli/src/honcho_cli/local/docker.py b/honcho-cli/src/honcho_cli/local/docker.py new file mode 100644 index 00000000..fbf1025f --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/docker.py @@ -0,0 +1,436 @@ +"""Docker daemon + Compose helpers for the local stack.""" + +from __future__ import annotations + +import json +import os +import socket +import subprocess +import sys +import tempfile +import time +from pathlib import Path + +from honcho_cli.local import STACK_SERVICES +from honcho_cli.local.profile import LocalProfile +from honcho_cli.output import print_error + +_DAEMON_DOWN_MARKERS = ( + "cannot connect to the docker daemon", + "is the docker daemon running", + "failed to connect to the docker api", + "error during connect", +) +_COMPOSE_MISSING_MARKERS = ( + "'compose' is not a docker command", + "unknown command: compose", + "docker: unknown command", +) +_CRED_HELPER_MARKERS = ("error getting credentials", "docker-credential-desktop") + + +class DockerError(Exception): + """Docker is missing, the daemon is down, or a Compose command failed.""" + + def __init__(self, code: str, message: str, details: dict | None = None): + super().__init__(message) + self.code = code + self.message = message + self.details = details or {} + + def exit(self) -> None: + print_error(self.code, self.message, self.details or None) + raise SystemExit(1) + + +def port_available(port: int, host: str = "127.0.0.1") -> bool: + """True when nothing is accepting connections on ``host:port``.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.settimeout(0.2) + return sock.connect_ex((host, port)) != 0 + + +def allocate_host_ports( + profile: LocalProfile, + *, + pinned: frozenset[str] = frozenset(), +) -> tuple[LocalProfile, dict[str, tuple[int, int]]]: + """Move api/db/redis host ports that are already bound. + + Names in ``pinned`` (``api`` / ``database`` / ``redis``) were set by a + flag and fail instead of moving. + """ + taken: set[int] = set() + remapped: dict[str, tuple[int, int]] = {} + chosen: dict[str, int] = {} + for name, field, flag in ( + ("api", "api_port", "--api-port"), + ("database", "db_port", "--db-port"), + ("redis", "redis_port", "--redis-port"), + ): + preferred = getattr(profile, field) + port = preferred + if name in pinned: + if preferred in taken or not port_available(preferred): + raise DockerError( + "PORT_IN_USE", + f"Host port {preferred} for {name} is already in use. " + f"Pass {flag} with a free port, or stop the other process.", + {"port": preferred, "service": name, "flag": flag}, + ) + else: + while port in taken or not port_available(port): + port += 1 + if port > preferred + 100: + raise DockerError( + "PORT_IN_USE", + f"Could not find a free host port near {preferred}.", + {"preferred": preferred}, + ) + if port != preferred: + remapped[name] = (preferred, port) + taken.add(port) + chosen[field] = port + return profile.overlay(**chosen), remapped + + +def compose_argv(profile: LocalProfile) -> list[str]: + return [ + "docker", + "compose", + "-f", + str(profile.compose_file()), + "--project-directory", + str(profile.dir()), + "-p", + profile.project_name, + ] + + +_CONFIG_PATHS = ("/app/config.toml.example", "/app/config.toml") +_CONFIG_HEADER = ( + "# Copied from {image} by honcho start. This file is not overwritten on later starts.\n" + "# Secrets belong in .env (environment variables win over this file).\n\n" +) + + +def image_is_digest(ref: str) -> bool: + """True when ``ref`` is already pinned to a content digest.""" + return "@sha256:" in ref.lower() + + +def image_repository(ref: str) -> str: + """Strip a tag or digest from a Docker image reference.""" + if "@" in ref: + return ref.split("@", 1)[0] + last_slash = ref.rfind("/") + last_colon = ref.rfind(":") + if last_colon > last_slash: + return ref[:last_colon] + return ref + + +def pin_image(image: str) -> str: + """Pull ``image`` if needed and return a digest-pinned reference. + + ``ghcr.io/plastic-labs/honcho:latest`` becomes + ``ghcr.io/plastic-labs/honcho@sha256:...`` so the profile does not + float when ``:latest`` moves. Already-pinned refs are left alone. + """ + if image_is_digest(image): + if not _image_exists(image): + _pull(image) + return image + _pull(image) + digest = _repo_digest(image) + if not digest: + raise DockerError( + "IMAGE_PIN_FAILED", + f"Pulled {image} but could not resolve a registry digest to pin.", + {"image": image}, + ) + return digest + + +def seed_config_toml(profile: LocalProfile) -> bool: + """Copy the image's ``config.toml.example`` into the profile if missing. + + Returns True when a file was written. Never overwrites an existing + ``config.toml``. + """ + dest = profile.config_file() + dest.parent.mkdir(parents=True, exist_ok=True) + if dest.exists(): + return False + copied = _copy_from_image(profile.image, _CONFIG_PATHS) + if copied is None: + raise DockerError( + "CONFIG_MISSING", + f"Could not copy config.toml from {profile.image}.", + {"image": profile.image}, + ) + dest.write_text(_CONFIG_HEADER.format(image=profile.image) + copied) + return True + + +def compose_up( + profile: LocalProfile, + *, + recreate: tuple[str, ...] = (), +) -> None: + """``docker compose up -d``. Compose output goes to stderr. + + ``recreate`` names services to ``--force-recreate`` (used after ``--setup`` + on an already-running stack so new ``.env`` values take effect). + """ + args = ["up", "-d"] + if recreate: + args.extend(["--force-recreate", *recreate]) + _run_compose(profile, args) + + +def compose_down(profile: LocalProfile, *, wipe: bool = False) -> None: + args = ["down"] + if wipe: + args.append("-v") + _run_compose(profile, args, capture=False) + + +def compose_ps(profile: LocalProfile) -> list[dict]: + """Parsed ``docker compose ps --format json`` (array or NDJSON).""" + proc = _run_compose(profile, ["ps", "--format", "json"], capture=True, check=False) + if proc.returncode != 0: + return [] + return _parse_ps(proc.stdout or "") + + +def services_running(ps: list[dict]) -> dict[str, str]: + """Map service name → state for the four stack services. + + State is ``running``, ``healthy``, ``exited``, etc. Prefer Docker's + Health field when present. + """ + out: dict[str, str] = {} + for row in ps: + service = str(row.get("Service") or row.get("Name") or "") + # "honcho-local-api-1" → try Service first; fall back to suffix match + if service not in STACK_SERVICES: + for name in STACK_SERVICES: + if ( + service == name + or service.endswith(f"-{name}-1") + or f"_{name}_" in service + ): + service = name + break + else: + continue + health = str(row.get("Health") or "").lower() + state = str(row.get("State") or row.get("Status") or "").lower() + if health: + out[service] = health + elif "health" in state: + # e.g. "running (healthy)" + out[service] = state + else: + out[service] = state or "unknown" + return out + + +def stack_containers_up(ps: list[dict]) -> bool: + """True when all four services are running (deriver has no healthcheck).""" + states = services_running(ps) + if any(name not in states for name in STACK_SERVICES): + return False + for state in states.values(): + if "exit" in state or state in {"dead", "paused"}: + return False + if "running" not in state and "healthy" not in state: + return False + return True + + +def _unavailable(proc: subprocess.CompletedProcess[str]) -> DockerError | None: + """Map a failed docker/compose process to a user-facing error, if obvious.""" + text = f"{proc.stderr or ''}{proc.stdout or ''}" + lower = text.lower() + if any(marker in lower for marker in _DAEMON_DOWN_MARKERS): + return DockerError( + "DOCKER_NOT_RUNNING", + "Docker is installed but the daemon is not running. Start it and retry.", + ) + if any(marker in lower for marker in _COMPOSE_MISSING_MARKERS): + return DockerError( + "DOCKER_COMPOSE_MISSING", + "Honcho start requires Docker Compose v2 (the `docker compose` plugin).", + ) + if any(marker in text for marker in _CRED_HELPER_MARKERS): + return DockerError( + "DOCKER_CREDENTIALS", + "Docker could not read registry credentials " + "(docker-credential-desktop is not on PATH). " + "Quit and reopen your terminal, or add Docker Desktop's bin " + "directory to PATH, then retry.", + {"exit_code": proc.returncode}, + ) + return None + + +def _run_compose( + profile: LocalProfile, + args: list[str], + *, + capture: bool = False, + check: bool = True, +) -> subprocess.CompletedProcess[str]: + cmd = compose_argv(profile) + args + cwd: Path = profile.dir() + try: + proc = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True) + except FileNotFoundError as e: + raise DockerError( + "DOCKER_NOT_INSTALLED", + "Docker is not installed. Install Docker Desktop (or another Compose-v2 runtime) and retry.", + ) from e + except OSError as e: + raise DockerError("COMPOSE_FAILED", str(e), {"command": cmd}) from e + if not capture: + if proc.stdout: + sys.stderr.write(proc.stdout) + if proc.stderr: + sys.stderr.write(proc.stderr) + if proc.returncode != 0: + classified = _unavailable(proc) + if classified is not None: + raise classified + if check and proc.returncode != 0: + raise DockerError( + "COMPOSE_FAILED", + "docker compose failed. See output above, or run `docker compose -p " + f"{profile.project_name} logs`.", + {"project": profile.project_name, "exit_code": proc.returncode}, + ) + return proc + + +def _run_docker( + args: list[str], + *, + check: bool = False, +) -> subprocess.CompletedProcess[str]: + try: + proc = subprocess.run( + ["docker", *args], + capture_output=True, + text=True, + ) + except FileNotFoundError as e: + raise DockerError( + "DOCKER_NOT_INSTALLED", + "Docker is not installed. Install Docker Desktop (or another Compose-v2 runtime) and retry.", + ) from e + except OSError as e: + raise DockerError("DOCKER_FAILED", str(e), {"command": args}) from e + if proc.returncode == 0: + return proc + classified = _unavailable(proc) + if classified is not None: + raise classified + if check: + raise DockerError( + "DOCKER_FAILED", + f"docker {' '.join(args)} failed.", + { + "exit_code": proc.returncode, + "stderr": (proc.stderr or "")[-500:], + }, + ) + return proc + + +def _pull(image: str) -> None: + proc = _run_docker(["pull", image], check=False) + if proc.stdout: + sys.stderr.write(proc.stdout) + if proc.stderr: + sys.stderr.write(proc.stderr) + if proc.returncode != 0: + raise DockerError( + "IMAGE_PULL_FAILED", + f"Failed to pull {image}.", + {"image": image, "exit_code": proc.returncode}, + ) + + +def _image_exists(image: str) -> bool: + return _run_docker(["image", "inspect", image], check=False).returncode == 0 + + +def _repo_digest(image: str) -> str | None: + proc = _run_docker( + ["image", "inspect", "--format", "{{json .RepoDigests}}", image], + check=False, + ) + if proc.returncode != 0: + return None + try: + digests = json.loads((proc.stdout or "").strip() or "[]") + except json.JSONDecodeError: + return None + if not isinstance(digests, list): + return None + repo = image_repository(image) + for item in digests: + if isinstance(item, str) and item.startswith(repo + "@"): + return item + for item in digests: + if isinstance(item, str) and "@sha256:" in item: + return item + return None + + +def _copy_from_image(image: str, paths: tuple[str, ...]) -> str | None: + """Create a stopped container and copy the first path that exists.""" + name = f"honcho-seed-{os.getpid()}-{time.time_ns()}" + created = _run_docker(["create", "--name", name, image], check=False) + if created.returncode != 0: + cid = (created.stdout or "").strip() or name + _run_docker(["rm", "-f", cid], check=False) + return None + cid = (created.stdout or "").strip() or name + try: + with tempfile.TemporaryDirectory(prefix="honcho-cfg-") as tmp: + dest = Path(tmp) / "config.toml" + for path in paths: + if dest.exists(): + dest.unlink() + copied = _run_docker(["cp", f"{cid}:{path}", str(dest)], check=False) + if copied.returncode == 0 and dest.exists(): + return dest.read_text(encoding="utf-8") + finally: + _run_docker(["rm", "-f", cid], check=False) + return None + + +def _parse_ps(stdout: str) -> list[dict]: + text = stdout.strip() + if not text: + return [] + if text.startswith("["): + try: + data = json.loads(text) + except json.JSONDecodeError: + return [] + return data if isinstance(data, list) else [] + rows: list[dict] = [] + for line in text.splitlines(): + line = line.strip() + if not line: + continue + try: + row = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(row, dict): + rows.append(row) + return rows diff --git a/honcho-cli/src/honcho_cli/local/env.py b/honcho-cli/src/honcho_cli/local/env.py new file mode 100644 index 00000000..ecf9c2c1 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/env.py @@ -0,0 +1,183 @@ +"""Render Compose + ``.env`` for a local stack profile.""" + +from __future__ import annotations + +import os +from contextlib import suppress +from importlib.resources import files +from pathlib import Path + +from honcho_cli.local.profile import LocalProfile + +# Keys honcho start owns. Unknown lines in an existing .env are preserved. +MANAGED_KEYS = ( + "AUTH_USE_AUTH", + "LOG_LEVEL", + "HONCHO_IMAGE", + "API_PORT", + "DB_PORT", + "REDIS_PORT", +) + +# Host env forwarded into the profile .env (overrides config.toml). +_SETTINGS_PREFIXES = ( + "LLM_", + "EMBEDDING_", + "DERIVER_", + "DIALECTIC_", + "DREAM_", + "SUMMARY_", +) +_LLM_KEYS = ( + "LLM_OPENAI_API_KEY", + "LLM_ANTHROPIC_API_KEY", + "LLM_GEMINI_API_KEY", +) + +_HEADER = ( + "# Generated by honcho start. Extra keys below the managed block are preserved." +) + +_PLACEHOLDERS = frozenset( + { + "", + "your-api-key-here", + "changeme", + "sk-...", + } +) + + +def is_placeholder_key(value: str | None) -> bool: + """True when ``value`` is missing or a known template placeholder.""" + if value is None: + return True + return value.strip() in _PLACEHOLDERS + + +def settings_from_environ() -> dict[str, str]: + """Host env vars that map to Honcho settings. Empty/placeholder values skipped.""" + return { + k: v + for k, v in os.environ.items() + if k.startswith(_SETTINGS_PREFIXES) and not is_placeholder_key(v) + } + + +def read_env_file(path: Path) -> dict[str, str]: + """Parse a dotenv file into a dict. Last assignment of a key wins.""" + if not path.exists(): + return {} + try: + lines = path.read_text(encoding="utf-8").splitlines() + except OSError: + return {} + out: dict[str, str] = {} + for line in lines: + stripped = line.strip() + if not stripped or stripped.startswith("#") or "=" not in stripped: + continue + k, _, v = stripped.partition("=") + out[k.strip()] = _unquote(v.strip()) + return out + + +def read_env_value(path: Path, key: str) -> str | None: + """Return the raw value for ``key`` in a dotenv file, or None.""" + return read_env_file(path).get(key) + + +def has_provider_key(profile: LocalProfile, extra: dict[str, str]) -> bool: + """True when host extra or profile ``.env`` has a real LLM API key.""" + stored = {**read_env_file(profile.env_file()), **extra} + return any(not is_placeholder_key(stored.get(k)) for k in _LLM_KEYS) + + +def managed_env(profile: LocalProfile) -> dict[str, str]: + """Values written into the managed block of ``.env``.""" + return { + "AUTH_USE_AUTH": "false", + "LOG_LEVEL": "INFO", + "HONCHO_IMAGE": profile.image, + "API_PORT": str(profile.api_port), + "DB_PORT": str(profile.db_port), + "REDIS_PORT": str(profile.redis_port), + } + + +def upsert_env( + path: Path, + updates: dict[str, str], + *, + drop: tuple[str, ...] = (), +) -> None: + """Write ``updates``, preserving unrelated user lines. + + Managed keys are written first (stable order), then any other keys in + ``updates``. Keys in ``drop`` are removed and not rewritten. Drops a + previous generated header so it is not duplicated. + """ + drop_set = frozenset(drop) + extras: list[str] = [] + if path.exists(): + for line in path.read_text(encoding="utf-8").splitlines(): + stripped = line.strip() + if stripped == _HEADER or stripped.startswith( + "# Generated by honcho start" + ): + continue + if not stripped or stripped.startswith("#"): + extras.append(line) + continue + if "=" in stripped: + k, _, _ = stripped.partition("=") + name = k.strip() + if name in updates or name in MANAGED_KEYS or name in drop_set: + continue + extras.append(line) + + managed = [f"{k}={updates[k]}" for k in MANAGED_KEYS if k in updates] + extra_updates = [ + f"{k}={v}" for k, v in updates.items() if k not in MANAGED_KEYS + ] + body = [_HEADER, *managed] + if extra_updates: + if not body[-1].startswith("#"): + body.append("") + body.extend(extra_updates) + if extras: + # Keep a blank line between generated and user keys when there are extras. + if extras[0].strip(): + body.append("") + body.extend(extras) + path.write_text("\n".join(body) + "\n") + with suppress(OSError): + os.chmod(path, 0o600) + + +def render_stack( + profile: LocalProfile, + extra: dict[str, str] | None = None, + drop: tuple[str, ...] = (), +) -> None: + """Write compose, init.sql, and .env into the profile directory.""" + directory = profile.dir() + directory.mkdir(parents=True, exist_ok=True) + with suppress(OSError): + os.chmod(directory, 0o700) + + templates = files("honcho_cli.local.templates") + compose = templates.joinpath("docker-compose.yml").read_text(encoding="utf-8") + init_sql = templates.joinpath("init.sql").read_text(encoding="utf-8") + profile.compose_file().write_text(compose) + (directory / "init.sql").write_text(init_sql) + updates = managed_env(profile) + if extra: + updates.update(extra) + upsert_env(profile.env_file(), updates, drop=drop) + + +def _unquote(value: str) -> str: + if len(value) >= 2 and value[0] == value[-1] and value[0] in {'"', "'"}: + return value[1:-1] + return value diff --git a/honcho-cli/src/honcho_cli/local/health.py b/honcho-cli/src/honcho_cli/local/health.py new file mode 100644 index 00000000..05593b99 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/health.py @@ -0,0 +1,53 @@ +"""Poll the local API health endpoint.""" + +from __future__ import annotations + +import time + +import httpx + +from honcho_cli.local.docker import compose_ps, services_running, stack_containers_up +from honcho_cli.local.profile import LocalProfile + + +def api_healthy(base_url: str, *, timeout: float = 2.0) -> bool: + """True when ``GET /health`` returns HTTP 200.""" + try: + with httpx.Client(timeout=timeout) as client: + response = client.get(base_url.rstrip("/") + "/health") + return response.status_code == 200 + except httpx.HTTPError: + return False + + +def stack_healthy(profile: LocalProfile) -> bool: + """True when Compose services are up and the API answers /health.""" + if not profile.compose_file().exists(): + return False + ps = compose_ps(profile) + if not stack_containers_up(ps): + return False + return api_healthy(profile.base_url) + + +def wait_for_health( + profile: LocalProfile, + *, + timeout: float, + interval: float = 1.0, +) -> bool: + """Poll until the API is healthy or ``timeout`` seconds elapse. + + Returns False on timeout. Fails fast if a required container has exited. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + ps = compose_ps(profile) + states = services_running(ps) + for _name, state in states.items(): + if "exit" in state or state in {"dead"}: + return False + if api_healthy(profile.base_url) and stack_containers_up(ps): + return True + time.sleep(interval) + return api_healthy(profile.base_url) diff --git a/honcho-cli/src/honcho_cli/local/profile.py b/honcho-cli/src/honcho_cli/local/profile.py new file mode 100644 index 00000000..69565071 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/profile.py @@ -0,0 +1,157 @@ +"""Named local-stack profiles under ``$HONCHO_CONFIG_DIR/profiles``. + +A profile is a Compose project directory, not an auth identity. +Resolution: ``--profile`` > ``HONCHO_PROFILE`` > ``local``. +""" + +from __future__ import annotations + +import json +import os +import re +from contextlib import suppress +from dataclasses import dataclass, replace + +from honcho_cli.local import ( + DEFAULT_API_PORT, + DEFAULT_DB_PORT, + DEFAULT_IMAGE, + DEFAULT_PROFILE, + DEFAULT_REDIS_PORT, +) +from honcho_cli.output import print_error + +_PROFILE_NAME = re.compile(r"^[a-z][a-z0-9_-]{0,62}$") + + +def profiles_dir(): + from honcho_cli import config as cfg + + return cfg.CONFIG_DIR / "profiles" + + +def validate_profile_name(name: str) -> str: + if name and _PROFILE_NAME.match(name): + return name + print_error( + "INVALID_PROFILE", + "Profile name must be lowercase alphanumeric, starting with a letter " + "(hyphens and underscores allowed).", + {"profile": name}, + ) + raise SystemExit(1) + + +def resolve_profile_name(flag: str | None) -> str: + raw = ( + (flag or "").strip() + or (os.environ.get("HONCHO_PROFILE") or "").strip() + or DEFAULT_PROFILE + ) + return validate_profile_name(raw) + + +def list_profile_names() -> list[str]: + """Profile directories that already have a Compose file.""" + root = profiles_dir() + if not root.is_dir(): + return [] + names: list[str] = [] + for path in sorted(root.iterdir()): + if ( + path.is_dir() + and _PROFILE_NAME.match(path.name) + and (path / "docker-compose.yml").exists() + ): + names.append(path.name) + return names + + +@dataclass +class LocalProfile: + """Ports and image for one local stack.""" + + name: str + api_port: int = DEFAULT_API_PORT + db_port: int = DEFAULT_DB_PORT + redis_port: int = DEFAULT_REDIS_PORT + image: str = DEFAULT_IMAGE + + @property + def project_name(self) -> str: + return f"honcho-{self.name}" + + @property + def base_url(self) -> str: + return f"http://127.0.0.1:{self.api_port}" + + def dir(self): + return profiles_dir() / self.name + + def compose_file(self): + return self.dir() / "docker-compose.yml" + + def env_file(self): + return self.dir() / ".env" + + def profile_file(self): + return self.dir() / "profile.json" + + def config_file(self): + return self.dir() / "config.toml" + + def endpoints(self) -> dict[str, str]: + return { + "api": self.base_url, + "docs": f"{self.base_url}/docs", + "postgres": f"postgresql://postgres:postgres@127.0.0.1:{self.db_port}/postgres", + "redis": f"redis://127.0.0.1:{self.redis_port}/0", + } + + def overlay(self, **fields) -> LocalProfile: + return replace(self, **{k: v for k, v in fields.items() if v is not None}) + + +def load_profile(name: str) -> LocalProfile: + profile = LocalProfile(name=validate_profile_name(name)) + path = profile.profile_file() + if not path.exists(): + return profile + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError): + return profile + if not isinstance(data, dict): + return profile + image = data.get("image") + return replace( + profile, + api_port=_port(data.get("apiPort"), profile.api_port), + db_port=_port(data.get("dbPort"), profile.db_port), + redis_port=_port(data.get("redisPort"), profile.redis_port), + image=image if isinstance(image, str) and image else profile.image, + ) + + +def save_profile(profile: LocalProfile) -> None: + directory = profile.dir() + directory.mkdir(parents=True, exist_ok=True) + with suppress(OSError): + os.chmod(directory, 0o700) + payload = { + "apiPort": profile.api_port, + "dbPort": profile.db_port, + "redisPort": profile.redis_port, + "image": profile.image, + } + profile.profile_file().write_text(json.dumps(payload, indent=2) + "\n") + + +def _port(value: object, default: int) -> int: + if isinstance(value, bool): + return default + try: + parsed = int(value) # type: ignore[arg-type] + except (TypeError, ValueError): + return default + return parsed if 1 <= parsed <= 65535 else default diff --git a/honcho-cli/src/honcho_cli/local/setup.py b/honcho-cli/src/honcho_cli/local/setup.py new file mode 100644 index 00000000..5b21728e --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/setup.py @@ -0,0 +1,469 @@ +"""Interactive ``honcho start --setup`` wizard. + +Writes curated LLM/feature overrides for the local stack. Secrets and knobs +go to the profile ``.env`` (env wins over ``config.toml``). Prompts are TTY +only — the start command rejects ``--setup`` in JSON / non-TTY mode. +""" + +from __future__ import annotations + +import tomllib +from dataclasses import dataclass +from pathlib import Path + +import typer +from rich.console import Console + +from honcho_cli.local.env import ( + is_placeholder_key, + read_env_file, + settings_from_environ, +) +from honcho_cli.output import print_error + +SETUP_MODES = ("basic", "advanced") +DIALECTIC_LEVELS = ("minimal", "low", "medium", "high", "max") +PROVIDERS = ("openai", "anthropic", "gemini", "openai-compatible") +EMBEDDING_TRANSPORTS = ("openai", "gemini") + +_CHAT_PREFIXES = ( + "DERIVER_MODEL_CONFIG", + "SUMMARY_MODEL_CONFIG", + "DREAM_DEDUCTION_MODEL_CONFIG", + "DREAM_INDUCTION_MODEL_CONFIG", + *(f"DIALECTIC_LEVELS__{level}__MODEL_CONFIG" for level in DIALECTIC_LEVELS), +) + +_PROVIDER_KEY_ENV = { + "openai": "LLM_OPENAI_API_KEY", + "openai-compatible": "LLM_OPENAI_API_KEY", + "anthropic": "LLM_ANTHROPIC_API_KEY", + "gemini": "LLM_GEMINI_API_KEY", +} + +_console = Console(stderr=True) + + +@dataclass(frozen=True) +class TomlSetupDefaults: + """Model/feature defaults copied from the image ``config.toml``. + + Honcho only ships OpenAI chat/embedding defaults. Other providers have + no suggested model in that file — the wizard does not invent one. + """ + + chat_transport: str | None = None + chat_model: str | None = None + embed_transport: str | None = None + embed_model: str | None = None + embed_dims: int | None = None + dreams_enabled: bool | None = None + flush_enabled: bool | None = None + + +def load_toml_setup_defaults(path: Path | None) -> TomlSetupDefaults: + """Read prompt defaults from the profile ``config.toml`` (image-aligned).""" + if path is None or not path.is_file(): + return TomlSetupDefaults() + try: + with path.open("rb") as fh: + data = tomllib.load(fh) + deriver = data.get("deriver") or {} + chat = deriver.get("model_config") or {} + embedding = data.get("embedding") or {} + embed = embedding.get("model_config") or {} + dream = data.get("dream") or {} + dims = embedding.get("VECTOR_DIMENSIONS") + return TomlSetupDefaults( + chat_transport=chat.get("transport"), + chat_model=chat.get("model"), + embed_transport=embed.get("transport"), + embed_model=embed.get("model"), + embed_dims=dims if isinstance(dims, int) and dims > 0 else None, + dreams_enabled=dream.get("ENABLED"), + flush_enabled=deriver.get("FLUSH_ENABLED"), + ) + except (OSError, tomllib.TOMLDecodeError, TypeError, AttributeError): + return TomlSetupDefaults() + + +def chat_model_default( + provider: str, + env: dict[str, str], + toml: TomlSetupDefaults, + *, + inferred: str | None = None, +) -> str: + """Prefer a previous wizard choice, else the image toml when transports match.""" + if inferred is None: + inferred = infer_provider(env) + if inferred == provider: + current = env.get("DERIVER_MODEL_CONFIG__MODEL") + if current: + return current + if toml.chat_model and _provider_matches_transport(provider, toml.chat_transport): + return toml.chat_model + return "" + + +def _provider_matches_transport(provider: str, transport: str | None) -> bool: + if not transport: + return False + return transport_of(provider) == transport + + +@dataclass(frozen=True) +class SetupAnswers: + """Curated knobs collected by the wizard (or tests).""" + + mode: str + provider: str + api_key: str + chat_model: str + base_url: str | None = None + embedding_api_key: str | None = None + embedding_key_transport: str | None = None + embedding_transport: str | None = None + embedding_model: str | None = None + embedding_dimensions: int | None = None + deriver_model: str | None = None + dialectic_model: str | None = None + dreams_enabled: bool | None = None + flush_enabled: bool | None = None + + +def transport_of(provider: str) -> str: + """Honcho ``MODEL_CONFIG.transport`` for a wizard provider id.""" + return "openai" if provider == "openai-compatible" else provider + + +def answers_to_env(answers: SetupAnswers) -> dict[str, str]: + """Map wizard answers to Honcho env overrides.""" + transport = transport_of(answers.provider) + env: dict[str, str] = {} + + env[_PROVIDER_KEY_ENV[answers.provider]] = answers.api_key + if answers.base_url: + env["LLM_OPENAI_BASE_URL"] = answers.base_url + + if answers.embedding_api_key and answers.embedding_key_transport: + embed_key = ( + "LLM_OPENAI_API_KEY" + if answers.embedding_key_transport == "openai" + else "LLM_GEMINI_API_KEY" + ) + env[embed_key] = answers.embedding_api_key + + for prefix in _CHAT_PREFIXES: + env[f"{prefix}__TRANSPORT"] = transport + env[f"{prefix}__MODEL"] = answers.chat_model + + if answers.deriver_model: + env["DERIVER_MODEL_CONFIG__TRANSPORT"] = transport + env["DERIVER_MODEL_CONFIG__MODEL"] = answers.deriver_model + + if answers.dialectic_model: + for level in DIALECTIC_LEVELS: + env[f"DIALECTIC_LEVELS__{level}__MODEL_CONFIG__TRANSPORT"] = transport + env[f"DIALECTIC_LEVELS__{level}__MODEL_CONFIG__MODEL"] = ( + answers.dialectic_model + ) + + if answers.embedding_transport: + env["EMBEDDING_MODEL_CONFIG__TRANSPORT"] = answers.embedding_transport + if answers.embedding_model: + env["EMBEDDING_MODEL_CONFIG__MODEL"] = answers.embedding_model + if answers.embedding_dimensions is not None: + env["EMBEDDING_VECTOR_DIMENSIONS"] = str(answers.embedding_dimensions) + elif answers.embedding_key_transport == "gemini": + # Basic + Anthropic chat: a Gemini key is unused unless embeddings switch. + env["EMBEDDING_MODEL_CONFIG__TRANSPORT"] = "gemini" + + if answers.dreams_enabled is not None: + env["DREAM_ENABLED"] = "true" if answers.dreams_enabled else "false" + if answers.flush_enabled is not None: + env["DERIVER_FLUSH_ENABLED"] = "true" if answers.flush_enabled else "false" + return env + + +def answers_drop_keys(answers: SetupAnswers) -> tuple[str, ...]: + """Keys to remove so a previous wizard run cannot leak into this one.""" + if answers.provider == "openai-compatible": + return () + return ("LLM_OPENAI_BASE_URL",) + + +def run_setup( + mode: str, + env_path: Path, + *, + config_path: Path | None = None, +) -> SetupAnswers: + """Prompt for ``basic`` or ``advanced`` knobs. Enter keeps the default.""" + env = read_env_file(env_path) + env.update(settings_from_environ()) + defaults = load_toml_setup_defaults(config_path) + _console.print() + _console.print( + " [dim]Configure the local stack. Press Enter to keep the default.[/dim]" + ) + _console.print( + " [dim]These values go in .env (they override config.toml).[/dim]" + ) + _console.print() + + inferred = infer_provider(env) + provider = _choose( + "LLM provider", + [ + ("openai", "OpenAI"), + ("anthropic", "Anthropic"), + ("gemini", "Gemini"), + ("openai-compatible", "OpenAI-compatible (OpenRouter, vLLM, Ollama, …)"), + ], + inferred if inferred in PROVIDERS else "openai", + ) + + base_url: str | None = None + if provider == "openai-compatible": + base_url = _prompt_text( + "OpenAI-compatible base URL", + env.get("LLM_OPENAI_BASE_URL") or "https://openrouter.ai/api/v1", + ) + + key_env = _PROVIDER_KEY_ENV[provider] + api_key = _prompt_secret("API key", env.get(key_env)) + + chat_default = chat_model_default( + provider, env, defaults, inferred=inferred + ) + chat_model = _prompt_text( + "Chat model (deriver, dialectic, summary, dream)", + chat_default, + required=True, + ) + + embedding_api_key: str | None = None + embedding_key_transport: str | None = None + embedding_transport: str | None = None + embedding_model: str | None = None + embedding_dimensions: int | None = None + deriver_model: str | None = None + dialectic_model: str | None = None + dreams_enabled: bool | None = None + flush_enabled: bool | None = None + + if mode == "advanced": + embedding_transport = _choose( + "Embedding provider", + [("openai", "OpenAI"), ("gemini", "Gemini")], + _default_embedding_transport(provider, env, defaults), + ) + same_embed = env.get("EMBEDDING_MODEL_CONFIG__TRANSPORT") == embedding_transport + current_embed = env.get("EMBEDDING_MODEL_CONFIG__MODEL") if same_embed else None + embed_from_toml = ( + defaults.embed_model + if defaults.embed_transport == embedding_transport + else None + ) + embedding_model = ( + _prompt_text("Embedding model", current_embed or embed_from_toml or "") + or None + ) + dim_default = ( + int(env["EMBEDDING_VECTOR_DIMENSIONS"]) + if env.get("EMBEDDING_VECTOR_DIMENSIONS", "").isdigit() + else (defaults.embed_dims or 1536) + ) + embedding_dimensions = _prompt_int("Embedding dimensions", dim_default) + embedding_key_transport, embedding_api_key = _embedding_key_if_needed( + provider, embedding_transport, env + ) + deriver_model = _prompt_text("Deriver model", chat_model) + dialectic_model = _prompt_text("Dialectic model (all reasoning levels)", chat_model) + dreams_enabled = _choose_bool( + "Dreams (periodic deeper reasoning)", + _env_bool( + env.get("DREAM_ENABLED"), + default=True if defaults.dreams_enabled is None else defaults.dreams_enabled, + ), + ) + flush_enabled = _choose_bool( + "Snappy local deriver (flush work immediately, skip batching)", + _env_bool( + env.get("DERIVER_FLUSH_ENABLED"), + default=False if defaults.flush_enabled is None else defaults.flush_enabled, + ), + ) + elif provider == "anthropic": + embedding_key_transport = _choose( + "Embeddings (Anthropic has none — pick a provider)", + [("openai", "OpenAI"), ("gemini", "Gemini")], + "openai", + ) + embed_key_env = _PROVIDER_KEY_ENV[ + "openai" if embedding_key_transport == "openai" else "gemini" + ] + embedding_api_key = _prompt_secret("Embedding API key", env.get(embed_key_env)) + + _console.print() + return SetupAnswers( + mode=mode, + provider=provider, + api_key=api_key, + chat_model=chat_model, + base_url=base_url, + embedding_api_key=embedding_api_key, + embedding_key_transport=embedding_key_transport, + embedding_transport=embedding_transport, + embedding_model=embedding_model, + embedding_dimensions=embedding_dimensions, + deriver_model=deriver_model, + dialectic_model=dialectic_model, + dreams_enabled=dreams_enabled, + flush_enabled=flush_enabled, + ) + + +def infer_provider(env: dict[str, str]) -> str: + """Best-effort provider from an existing profile ``.env``.""" + if env.get("LLM_OPENAI_BASE_URL"): + return "openai-compatible" + transport = env.get("DERIVER_MODEL_CONFIG__TRANSPORT") + if transport in ("anthropic", "gemini", "openai"): + return transport + if env.get("LLM_ANTHROPIC_API_KEY") and not env.get("LLM_OPENAI_API_KEY"): + return "anthropic" + if env.get("LLM_GEMINI_API_KEY") and not env.get("LLM_OPENAI_API_KEY"): + return "gemini" + return "openai" + + +def _default_embedding_transport( + provider: str, env: dict[str, str], defaults: TomlSetupDefaults +) -> str: + current = env.get("EMBEDDING_MODEL_CONFIG__TRANSPORT") + if current in EMBEDDING_TRANSPORTS: + return current + if defaults.embed_transport in EMBEDDING_TRANSPORTS: + return defaults.embed_transport + if provider == "gemini": + return "gemini" + return "openai" + + +def _embedding_key_if_needed( + chat_provider: str, + embed_transport: str, + env: dict[str, str], +) -> tuple[str | None, str | None]: + """Prompt for an embedding key when the chat provider cannot supply it.""" + chat_transport = transport_of(chat_provider) + if embed_transport == chat_transport or ( + chat_provider == "openai-compatible" and embed_transport == "openai" + ): + return None, None + key_env = _PROVIDER_KEY_ENV[embed_transport] + key = _prompt_secret(f"{embed_transport} embedding API key", env.get(key_env)) + return embed_transport, key + + +def _choose(label: str, options: list[tuple[str, str]], default: str) -> str: + ids = [item[0] for item in options] + default_idx = ids.index(default) + 1 if default in ids else 1 + _console.print(f" [dim]{label}[/dim]") + for i, (_oid, desc) in enumerate(options, 1): + _console.print(f" [dim]({i})[/dim] {desc}") + raw = typer.prompt( + " Choice", + default=str(default_idx), + show_default=True, + prompt_suffix=": ", + ).strip() + try: + idx = int(raw) + except ValueError: + if raw in ids: + return raw + return options[default_idx - 1][0] + if 1 <= idx <= len(options): + return options[idx - 1][0] + return options[default_idx - 1][0] + + +def _choose_bool(label: str, default: bool) -> bool: + return ( + _choose(label, [("true", "On"), ("false", "Off")], "true" if default else "false") + == "true" + ) + + +def _prompt_text(label: str, default: str, *, required: bool = False) -> str: + while True: + raw = typer.prompt( + f" {label}", + default=default, + show_default=bool(default), + prompt_suffix=": ", + ).strip() + value = raw or default + if value or not required: + return value + _console.print(" [red]A model name is required[/red]") + + +def _prompt_int(label: str, default: int) -> int: + while True: + raw = typer.prompt( + f" {label}", + default=str(default), + show_default=True, + prompt_suffix=": ", + ).strip() + try: + value = int(raw) + except ValueError: + _console.print(" [red]Enter an integer[/red]") + continue + if value > 0: + return value + _console.print(" [red]Must be a positive integer[/red]") + + +def _prompt_secret(label: str, current: str | None) -> str: + if current and not is_placeholder_key(current): + _console.print(f" [dim]Current {label}: {_redact(current)}[/dim]") + _console.print(" [dim](1)[/dim] Keep current key") + _console.print(" [dim](2)[/dim] Enter a new key") + choice = typer.prompt( + " Choice", default="1", show_default=True, prompt_suffix=": " + ).strip() + if choice != "2": + return current + _console.print(f" [dim]{label}[/dim]") + raw = typer.prompt( + f" {label}", + default="", + show_default=False, + hide_input=True, + prompt_suffix=": ", + ).strip() + if not raw or is_placeholder_key(raw): + print_error( + "MISSING_LLM_KEY", + f"{label} is required.", + ) + raise typer.Exit(1) + return raw + + +def _redact(key: str) -> str: + if len(key) <= 4: + return "***" + return "***" + key[-4:] + + +def _env_bool(value: str | None, *, default: bool) -> bool: + if value is None: + return default + return value.strip().lower() in ("1", "true", "yes", "on") diff --git a/honcho-cli/src/honcho_cli/local/templates/__init__.py b/honcho-cli/src/honcho_cli/local/templates/__init__.py new file mode 100644 index 00000000..e7802aa6 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/templates/__init__.py @@ -0,0 +1 @@ +"""Package data for the local stack (Compose template + Postgres init).""" diff --git a/honcho-cli/src/honcho_cli/local/templates/docker-compose.yml b/honcho-cli/src/honcho_cli/local/templates/docker-compose.yml new file mode 100644 index 00000000..4a19af67 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/templates/docker-compose.yml @@ -0,0 +1,100 @@ +# Managed by `honcho start`. Re-rendered on every start — edit .env and config.toml, not this file. +# +# Images: ghcr.io/plastic-labs/honcho (API + deriver), pgvector/pgvector:pg15, redis:8.2 +# Ports bind to 127.0.0.1. Auth is off (AUTH_USE_AUTH=false in .env). + +services: + api: + image: ${HONCHO_IMAGE:-ghcr.io/plastic-labs/honcho:latest} + entrypoint: ["sh", "docker/entrypoint.sh"] + depends_on: + database: + condition: service_healthy + redis: + condition: service_healthy + ports: + - "127.0.0.1:${API_PORT:-8000}:8000" + healthcheck: + test: + [ + "CMD", + "/app/.venv/bin/python", + "-c", + "import urllib.request; urllib.request.urlopen('http://localhost:8000/health', timeout=2).read()", + ] + interval: 5s + timeout: 5s + retries: 5 + start_period: 10s + volumes: + - lancedb-data:/app/lancedb_data + - ./config.toml:/app/config.toml:ro + environment: + - DB_CONNECTION_URI=postgresql+psycopg://postgres:postgres@database:5432/postgres + - CACHE_URL=redis://redis:6379/0?suppress=true + - CACHE_ENABLED=true + env_file: + - path: .env + required: false + restart: unless-stopped + + deriver: + image: ${HONCHO_IMAGE:-ghcr.io/plastic-labs/honcho:latest} + entrypoint: ["/app/.venv/bin/python", "-m", "src.deriver"] + depends_on: + api: + condition: service_healthy + database: + condition: service_healthy + redis: + condition: service_healthy + volumes: + - lancedb-data:/app/lancedb_data + - ./config.toml:/app/config.toml:ro + environment: + - DB_CONNECTION_URI=postgresql+psycopg://postgres:postgres@database:5432/postgres + - CACHE_URL=redis://redis:6379/0?suppress=true + - CACHE_ENABLED=true + env_file: + - path: .env + required: false + restart: unless-stopped + + database: + image: pgvector/pgvector:pg15 + restart: unless-stopped + ports: + - "127.0.0.1:${DB_PORT:-5432}:5432" + command: ["postgres", "-c", "max_connections=200"] + environment: + - POSTGRES_DB=postgres + - POSTGRES_USER=postgres + - POSTGRES_PASSWORD=postgres + - POSTGRES_HOST_AUTH_METHOD=trust + - PGDATA=/var/lib/postgresql/data/pgdata + volumes: + - ./init.sql:/docker-entrypoint-initdb.d/init.sql + - pgdata:/var/lib/postgresql/data/ + healthcheck: + test: ["CMD-SHELL", "pg_isready -U postgres -d postgres"] + interval: 5s + timeout: 5s + retries: 5 + + redis: + image: redis:8.2 + restart: unless-stopped + ports: + - "127.0.0.1:${REDIS_PORT:-6379}:6379" + volumes: + - redis-data:/data + healthcheck: + test: ["CMD-SHELL", "redis-cli ping"] + interval: 5s + timeout: 5s + retries: 5 + +volumes: + pgdata: + redis-data: + lancedb-data: diff --git a/honcho-cli/src/honcho_cli/local/templates/init.sql b/honcho-cli/src/honcho_cli/local/templates/init.sql new file mode 100644 index 00000000..0aa0fc22 --- /dev/null +++ b/honcho-cli/src/honcho_cli/local/templates/init.sql @@ -0,0 +1 @@ +CREATE EXTENSION IF NOT EXISTS vector; diff --git a/honcho-cli/src/honcho_cli/main.py b/honcho-cli/src/honcho_cli/main.py index 7ed8aa9b..8a9c16f6 100644 --- a/honcho-cli/src/honcho_cli/main.py +++ b/honcho-cli/src/honcho_cli/main.py @@ -64,9 +64,13 @@ def main( # Register top-level commands from honcho_cli.commands.setup import doctor, init +from honcho_cli.commands.stack import start, status, stop app.command()(init) app.command()(doctor) +app.command()(start) +app.command()(stop) +app.command()(status) @app.command("help", hidden=True) diff --git a/honcho-cli/src/honcho_cli/output.py b/honcho-cli/src/honcho_cli/output.py index 2f0ec5e9..e95f17e6 100644 --- a/honcho-cli/src/honcho_cli/output.py +++ b/honcho-cli/src/honcho_cli/output.py @@ -15,6 +15,8 @@ from rich.console import Console from rich.table import Table from rich.text import Text +from honcho_cli.branding import ICON_FAIL, ICON_OK, ICON_RUN + console = Console(stderr=True) stdout_console = Console() @@ -106,6 +108,24 @@ def status(msg: str) -> None: console.print(f"[dim]{msg}[/dim]") +def step(msg: str) -> None: + """Print a progress step. No-op in JSON mode.""" + if not use_json(): + console.print(f" {ICON_RUN} {msg}") + + +def ok(msg: str) -> None: + """Print a success line. No-op in JSON mode.""" + if not use_json(): + console.print(f" {ICON_OK} {msg}") + + +def fail(msg: str) -> None: + """Print a failure line. No-op in JSON mode.""" + if not use_json(): + console.print(f" {ICON_FAIL} {msg}") + + # Stable peer-color palette for transcript rendering. Brand blue first so the # primary peer lands on brand when there's only one speaker. _PEER_COLORS = ( diff --git a/honcho-cli/tests/test_local.py b/honcho-cli/tests/test_local.py new file mode 100644 index 00000000..02e5cf2f --- /dev/null +++ b/honcho-cli/tests/test_local.py @@ -0,0 +1,127 @@ +"""Local-stack contracts: profile files, env merge, image pin, port remap.""" + +from __future__ import annotations + +import json +import os +import subprocess + +import pytest +from honcho_cli.local.docker import ( + DockerError, + allocate_host_ports, + pin_image, + seed_config_toml, +) +from honcho_cli.local.env import managed_env, read_env_value, render_stack, upsert_env +from honcho_cli.local.profile import LocalProfile, load_profile, save_profile + + +@pytest.fixture +def cfg_dir(tmp_path, monkeypatch): + monkeypatch.setattr("honcho_cli.config.CONFIG_DIR", tmp_path) + monkeypatch.setattr("honcho_cli.config.CONFIG_FILE", tmp_path / "config.json") + for k in [k for k in os.environ if k.startswith("HONCHO_")]: + monkeypatch.delenv(k) + return tmp_path + + +def test_profile_roundtrip_has_no_secrets(cfg_dir): + profile = LocalProfile( + name="local", + api_port=8001, + image="ghcr.io/plastic-labs/honcho@sha256:abc", + ) + save_profile(profile) + loaded = load_profile("local") + assert loaded.api_port == 8001 + assert loaded.image.endswith("@sha256:abc") + on_disk = json.loads(profile.profile_file().read_text()) + assert "LLM" not in json.dumps(on_disk) + assert set(on_disk) == {"apiPort", "dbPort", "redisPort", "image"} + + +def test_upsert_preserves_extra_env_keys(tmp_path): + path = tmp_path / ".env" + path.write_text("CUSTOM_FLAG=keep-me\n# user comment\n") + upsert_env(path, managed_env(LocalProfile(name="local"))) + text = path.read_text() + assert "CUSTOM_FLAG=keep-me" in text + assert "user comment" in text + assert text.count("Generated by honcho start") == 1 + + +def test_upsert_writes_non_managed_and_preserves_later(tmp_path): + path = tmp_path / ".env" + first = managed_env(LocalProfile(name="local")) + first["DERIVER_MODEL_CONFIG__MODEL"] = "gpt-test" + upsert_env(path, first) + upsert_env(path, managed_env(LocalProfile(name="local"))) + later = path.read_text() + assert "DERIVER_MODEL_CONFIG__MODEL=gpt-test" in later + + +def test_render_stack_uses_published_image(cfg_dir): + profile = LocalProfile(name="local") + render_stack(profile) + compose = profile.compose_file().read_text() + assert "ghcr.io/plastic-labs/honcho" in compose + assert "build:" not in compose + assert compose.count("./config.toml:/app/config.toml:ro") == 2 + assert read_env_value(profile.env_file(), "AUTH_USE_AUTH") == "false" + assert oct(profile.env_file().stat().st_mode)[-3:] == "600" + + +def test_pin_latest_to_matching_digest(monkeypatch): + pulls: list[str] = [] + + def fake_run(args, *, check=False): + if args[:1] == ["pull"]: + pulls.append(args[1]) + return subprocess.CompletedProcess(args, 0, stdout="", stderr="") + if args[:2] == ["image", "inspect"]: + body = json.dumps( + [ + "ghcr.io/plastic-labs/honcho@sha256:deadbeef", + "ghcr.io/other/honcho@sha256:nope", + ] + ) + return subprocess.CompletedProcess(args, 0, stdout=body, stderr="") + raise AssertionError(args) + + monkeypatch.setattr("honcho_cli.local.docker._run_docker", fake_run) + assert pin_image("ghcr.io/plastic-labs/honcho:latest") == ( + "ghcr.io/plastic-labs/honcho@sha256:deadbeef" + ) + assert pulls == ["ghcr.io/plastic-labs/honcho:latest"] + + +def test_seed_config_toml_writes_once(cfg_dir, monkeypatch): + profile = LocalProfile( + name="local", image="ghcr.io/plastic-labs/honcho@sha256:abc" + ) + monkeypatch.setattr( + "honcho_cli.local.docker._copy_from_image", + lambda image, paths: "[deriver]\nWORKERS = 2\n", + ) + assert seed_config_toml(profile) is True + profile.config_file().write_text( + profile.config_file().read_text() + "# user edit\n" + ) + assert seed_config_toml(profile) is False + assert "# user edit" in profile.config_file().read_text() + + +def test_busy_port_remaps_unless_pinned(monkeypatch): + monkeypatch.setattr( + "honcho_cli.local.docker.port_available", + lambda port, host="127.0.0.1": port != 6379, + ) + profile, remapped = allocate_host_ports(LocalProfile(name="local")) + assert profile.redis_port == 6380 + assert remapped["redis"] == (6379, 6380) + + with pytest.raises(DockerError) as exc: + allocate_host_ports(LocalProfile(name="local"), pinned=frozenset({"redis"})) + assert exc.value.code == "PORT_IN_USE" + assert exc.value.details["flag"] == "--redis-port" diff --git a/honcho-cli/tests/test_setup.py b/honcho-cli/tests/test_setup.py new file mode 100644 index 00000000..5a411946 --- /dev/null +++ b/honcho-cli/tests/test_setup.py @@ -0,0 +1,66 @@ +"""Wizard mapping: ``answers_to_env`` and image-toml defaults.""" + +from __future__ import annotations + +from honcho_cli.local.setup import ( + DIALECTIC_LEVELS, + SetupAnswers, + answers_to_env, + chat_model_default, + load_toml_setup_defaults, +) + + +def test_basic_openai_applies_chat_model_everywhere(): + env = answers_to_env( + SetupAnswers( + mode="basic", + provider="openai", + api_key="sk-test", + chat_model="gpt-test", + ) + ) + assert env["LLM_OPENAI_API_KEY"] == "sk-test" + assert env["DERIVER_MODEL_CONFIG__MODEL"] == "gpt-test" + assert env["SUMMARY_MODEL_CONFIG__MODEL"] == "gpt-test" + for level in DIALECTIC_LEVELS: + assert env[f"DIALECTIC_LEVELS__{level}__MODEL_CONFIG__MODEL"] == "gpt-test" + assert "DREAM_ENABLED" not in env + assert "EMBEDDING_MODEL_CONFIG__MODEL" not in env + + +def test_basic_anthropic_keeps_openai_embeddings_default(): + env = answers_to_env( + SetupAnswers( + mode="basic", + provider="anthropic", + api_key="sk-ant", + chat_model="claude-haiku-4-5", + embedding_api_key="sk-embed", + embedding_key_transport="openai", + ) + ) + assert env["LLM_ANTHROPIC_API_KEY"] == "sk-ant" + assert env["LLM_OPENAI_API_KEY"] == "sk-embed" + assert env["DERIVER_MODEL_CONFIG__TRANSPORT"] == "anthropic" + assert "EMBEDDING_MODEL_CONFIG__TRANSPORT" not in env + + +def test_chat_default_comes_from_image_toml(tmp_path): + path = tmp_path / "config.toml" + path.write_text( + "[deriver.model_config]\n" + 'transport = "openai"\n' + 'model = "gpt-from-image"\n' + ) + defaults = load_toml_setup_defaults(path) + assert defaults.chat_model == "gpt-from-image" + assert chat_model_default("openai", {}, defaults) == "gpt-from-image" + assert chat_model_default("openai-compatible", {}, defaults) == "gpt-from-image" + assert chat_model_default("anthropic", {}, defaults) == "" + assert chat_model_default( + "openai", + {"DERIVER_MODEL_CONFIG__MODEL": "gpt-from-env"}, + defaults, + inferred="openai", + ) == "gpt-from-env" diff --git a/honcho-cli/tests/test_start.py b/honcho-cli/tests/test_start.py new file mode 100644 index 00000000..2b589813 --- /dev/null +++ b/honcho-cli/tests/test_start.py @@ -0,0 +1,159 @@ +"""CLI contracts for `honcho start` / `stop` / `status`.""" + +from __future__ import annotations + +import json +import os + +import pytest +from honcho_cli.local.docker import image_is_digest, image_repository +from honcho_cli.main import app +from typer.testing import CliRunner + + +@pytest.fixture +def cfg(tmp_path, monkeypatch): + f = tmp_path / "config.json" + monkeypatch.setattr("honcho_cli.config.CONFIG_DIR", tmp_path) + monkeypatch.setattr("honcho_cli.config.CONFIG_FILE", f) + monkeypatch.setattr("honcho_cli.commands.setup.CONFIG_FILE", f) + for k in [k for k in os.environ if k.startswith(("HONCHO_", "LLM_"))]: + monkeypatch.delenv(k) + return f + + +@pytest.fixture +def runner(): + return CliRunner() + + +@pytest.fixture(autouse=True) +def _host_ports_free(monkeypatch): + monkeypatch.setattr("honcho_cli.local.docker.port_available", lambda *a, **k: True) + + +@pytest.fixture(autouse=True) +def _stub_image_pin(monkeypatch): + def fake_pin(image: str) -> str: + if image_is_digest(image): + return image + return f"{image_repository(image)}@sha256:cafedeadbeef" + + monkeypatch.setattr("honcho_cli.commands.stack.pin_image", fake_pin) + monkeypatch.setattr("honcho_cli.commands.stack.seed_config_toml", lambda profile: False) + + +_PS = [ + {"Service": "api", "State": "running", "Health": "healthy"}, + {"Service": "deriver", "State": "running"}, + {"Service": "database", "State": "running", "Health": "healthy"}, + {"Service": "redis", "State": "running", "Health": "healthy"}, +] + + +def test_start_does_not_rewrite_environment_url(cfg, runner, monkeypatch): + cfg.write_text( + json.dumps({"apiKey": "k", "environmentUrl": "https://api.honcho.dev"}) + ) + monkeypatch.setattr("honcho_cli.commands.stack.stack_healthy", lambda profile: False) + monkeypatch.setattr("honcho_cli.commands.stack.compose_up", lambda profile, **k: None) + monkeypatch.setattr("honcho_cli.commands.stack.wait_for_health", lambda *a, **k: True) + monkeypatch.setattr("honcho_cli.commands.stack.compose_ps", lambda profile: _PS) + monkeypatch.setenv("LLM_OPENAI_API_KEY", "sk-test") + result = runner.invoke(app, ["start", "--json"]) + assert result.exit_code == 0, result.stderr + payload = json.loads(result.stdout) + assert payload["endpoints"]["api"] == "http://127.0.0.1:8000" + assert payload["image"].endswith("@sha256:cafedeadbeef") + on_disk = json.loads(cfg.read_text()) + assert on_disk["environmentUrl"] == "https://api.honcho.dev" + + +def test_start_requires_llm_key(cfg, runner, monkeypatch): + monkeypatch.setattr("honcho_cli.commands.stack.stack_healthy", lambda profile: False) + result = runner.invoke(app, ["start"]) + assert result.exit_code == 1 + assert json.loads(result.stderr)["error"]["code"] == "MISSING_LLM_KEY" + + +def test_stop_already_stopped_skips_down(cfg, runner, tmp_path, monkeypatch): + compose = tmp_path / "profiles" / "local" / "docker-compose.yml" + compose.parent.mkdir(parents=True) + compose.write_text("services: {}\n") + monkeypatch.setattr("honcho_cli.commands.stack.compose_ps", lambda profile: []) + down = [] + monkeypatch.setattr( + "honcho_cli.commands.stack.compose_down", + lambda profile, wipe=False: down.append(wipe), + ) + result = runner.invoke(app, ["stop"]) + assert result.exit_code == 0, result.stderr + assert down == [] + assert json.loads(result.stdout)["status"] == "stopped" + + +def test_status_lists_profiles_or_one(cfg, runner, tmp_path, monkeypatch): + for name, port in (("demo", 8001), ("local", 8000)): + d = tmp_path / "profiles" / name + d.mkdir(parents=True) + (d / "docker-compose.yml").write_text("services: {}\n") + (d / "profile.json").write_text(json.dumps({"apiPort": port}) + "\n") + + monkeypatch.setattr( + "honcho_cli.commands.stack.compose_ps", + lambda profile: _PS if profile.name == "local" else [], + ) + monkeypatch.setattr( + "honcho_cli.commands.stack.stack_healthy", + lambda profile: profile.name == "local", + ) + listed = runner.invoke(app, ["status"]) + assert listed.exit_code == 0, listed.stderr + rows = json.loads(listed.stdout)["profiles"] + by_name = {row["profile"]: row for row in rows} + assert by_name["local"]["status"] == "running" + assert by_name["demo"]["endpoints"]["api"] == "http://127.0.0.1:8001" + + one = runner.invoke(app, ["status", "--profile", "local"]) + assert one.exit_code == 0, one.stderr + payload = json.loads(one.stdout) + assert payload["profile"] == "local" + assert "profiles" not in payload + + +def test_start_setup_requires_tty(cfg, runner): + result = runner.invoke(app, ["start", "--setup", "basic", "--json"]) + assert result.exit_code == 1 + assert json.loads(result.stderr)["error"]["code"] == "SETUP_REQUIRES_TTY" + + +def test_start_setup_recreates_when_already_running(cfg, runner, monkeypatch): + from honcho_cli.local.setup import SetupAnswers + + ups: list[tuple[str, ...]] = [] + monkeypatch.setattr("honcho_cli.commands.stack.use_json", lambda: False) + monkeypatch.setattr("honcho_cli.commands.stack.stack_healthy", lambda profile: True) + monkeypatch.setattr( + "honcho_cli.commands.stack.compose_up", + lambda profile, **k: ups.append(k.get("recreate", ())), + ) + monkeypatch.setattr("honcho_cli.commands.stack.wait_for_health", lambda *a, **k: True) + monkeypatch.setattr("honcho_cli.commands.stack.compose_ps", lambda profile: _PS) + monkeypatch.setattr( + "honcho_cli.commands.stack.run_setup", + lambda mode, path, config_path=None: SetupAnswers( + mode="basic", + provider="openai", + api_key="sk-wiz", + chat_model="gpt-test", + ), + ) + pins: list[str] = [] + monkeypatch.setattr( + "honcho_cli.commands.stack.pin_image", + lambda image: pins.append(image) or image, + ) + result = runner.invoke(app, ["start", "--setup", "basic"]) + assert result.exit_code == 0, result.stderr + assert pins == [] + assert ups == [("api", "deriver")] diff --git a/skills/honcho-cli/SKILL.md b/skills/honcho-cli/SKILL.md index e773cfa0..4f75f259 100644 --- a/skills/honcho-cli/SKILL.md +++ b/skills/honcho-cli/SKILL.md @@ -18,6 +18,7 @@ allowed-tools: Bash(honcho:*), Bash(jq:*), Read, Grep ## Command groups - `honcho config` — CLI configuration +- `honcho start` / `stop` / `status` — local Docker stack (does not change `environmentUrl`). First start pins the Honcho image digest and writes `config.toml` into the profile. Pass `--setup basic` or `--setup advanced` for an interactive config wizard (TTY only; writes `.env` overrides). `honcho status` lists every profile; pass `--profile` for one. - `honcho workspace` — inspect, delete, search - `honcho peer` — inspect, card, chat, search - `honcho session` — inspect, view (transcript), context, summaries @@ -31,6 +32,7 @@ allowed-tools: Bash(honcho:*), Bash(jq:*), Read, Grep - Use `honcho session context` to see exactly what an agent receives. - Never run `honcho workspace delete` without `honcho workspace inspect` first. - Compare peer card with conclusions to understand memory state. +- `honcho start` does not rewrite `environmentUrl`. Use `HONCHO_BASE_URL=http://127.0.0.1:8000` to talk to local stack. ## Inspection tour From 99a06baf29997e2be1c4d9c182f6ed731b43fe01 Mon Sep 17 00:00:00 2001 From: Rajat Ahuja Date: Tue, 25 Aug 2026 12:34:42 -0400 Subject: [PATCH 5/7] perf(cache): hash-tag the cache namespace so an instance uses one shard (#1058) On Redis Cluster a key's slot comes from the substring inside the first {...}, when one is present. Untagged, one deployment's keys spread over every slot, so its client opens and holds a connection to every node in the cluster. Wrapping the namespace in braces puts them all on one slot, and therefore one node, cutting each deployment's connection count to the cluster by a factor of the shard count. Namespaces still hash independently of each other, so keys stay spread across the cluster and no shard becomes a hotspot. The tag needs two spellings, because the two ways a key gets built treat the string differently. cashews runs `prefix=` through format substitution, so braces have to be doubled there to survive as literals; keys built by concatenation need them single. A single brace passed to cashews is read as an empty substitution field and the namespace is dropped entirely, which would let two deployments collide on one key -- hence two clearly named helpers rather than one string, and a test that the two paths produce identical bytes. No key format change for a non-cluster backend, and no migration: the old keys simply age out by TTL. --- src/cache/client.py | 24 ++++++++++++ src/crud/collection.py | 9 +++-- src/crud/peer.py | 13 +++++-- src/crud/session.py | 9 +++-- src/crud/workspace.py | 9 +++-- tests/test_cache_key_namespace.py | 62 +++++++++++++++++++++++++++++++ 6 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 tests/test_cache_key_namespace.py diff --git a/src/cache/client.py b/src/cache/client.py index 1ad8e3f5..4b9fabb8 100644 --- a/src/cache/client.py +++ b/src/cache/client.py @@ -123,6 +123,28 @@ def get_cache_namespace() -> str: return cast(str, settings.CACHE.NAMESPACE) +# On Redis Cluster a key's slot is derived from the substring inside the first +# {...}, when one is present. Tagging the namespace puts every key an instance +# writes on a single slot, and therefore a single shard, so its client holds +# connections to one node rather than to all of them. Namespaces still hash +# independently of one another, so keys stay spread across the cluster. +# +# Two spellings, because the two ways a key gets built treat the string +# differently: cashews runs `prefix=` through format substitution, so braces +# have to be doubled to survive as literals, while direct construction does no +# substitution and needs them single. Both render to the same bytes, which +# tests/cache/test_cache_namespace_hash_tag.py asserts -- a mismatch would send +# writes and deletes to different keys with nothing raised. +def cache_key_namespace() -> str: + """Tagged namespace for keys built by string concatenation.""" + return "{" + get_cache_namespace() + "}" + + +def cache_prefix_namespace() -> str: + """Tagged namespace for cashews `prefix=`, which format-substitutes.""" + return "{{" + get_cache_namespace() + "}}" + + async def init_cache() -> None: """Initialize and verify cache connection if enabled.""" async with _cache_lock: @@ -256,6 +278,8 @@ __all__ = [ "init_cache", "close_cache", "cache", + "cache_key_namespace", + "cache_prefix_namespace", "safe_cache_delete", "safe_cache_set", ] diff --git a/src/crud/collection.py b/src/crud/collection.py index 63a775e3..a5101b6e 100644 --- a/src/crud/collection.py +++ b/src/crud/collection.py @@ -10,7 +10,8 @@ from sqlalchemy.orm import make_transient_to_detached from src import models from src.cache.client import ( cache, - get_cache_namespace, + cache_key_namespace, + cache_prefix_namespace, safe_cache_delete, safe_cache_set, ) @@ -22,13 +23,13 @@ logger = getLogger(__name__) COLLECTION_CACHE_KEY_TEMPLATE = ( "v2:workspace:{workspace_name}:collection:{observer}:{observed}" ) -COLLECTION_LOCK_PREFIX = f"{get_cache_namespace()}:lock:v2" +COLLECTION_LOCK_PREFIX = f"{cache_prefix_namespace()}:lock:v2" def collection_cache_key(workspace_name: str, observer: str, observed: str) -> str: """Generate cache key for collection.""" return ( - get_cache_namespace() + cache_key_namespace() + ":" + COLLECTION_CACHE_KEY_TEMPLATE.format( workspace_name=workspace_name, @@ -41,7 +42,7 @@ def collection_cache_key(workspace_name: str, observer: str, observed: str) -> s @cache( key=COLLECTION_CACHE_KEY_TEMPLATE, ttl=f"{settings.CACHE.DEFAULT_TTL_SECONDS}s", - prefix=get_cache_namespace(), + prefix=cache_prefix_namespace(), condition=NOT_NONE, ) @cache.locked( diff --git a/src/crud/peer.py b/src/crud/peer.py index 2f81f938..0bd1e120 100644 --- a/src/crud/peer.py +++ b/src/crud/peer.py @@ -12,7 +12,12 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import make_transient_to_detached from src import models, schemas -from src.cache.client import cache, get_cache_namespace, safe_cache_delete +from src.cache.client import ( + cache, + cache_key_namespace, + cache_prefix_namespace, + safe_cache_delete, +) from src.config import settings from src.crud.workspace import get_or_create_workspace from src.exceptions import ( @@ -32,13 +37,13 @@ logger = getLogger(__name__) 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" +PEER_LOCK_PREFIX = f"{cache_prefix_namespace()}:lock:v2" def peer_cache_key(workspace_name: str, peer_name: str) -> str: """Generate cache key for peer.""" return ( - get_cache_namespace() + cache_key_namespace() + ":" + PEER_CACHE_KEY_TEMPLATE.format( workspace_name=workspace_name, @@ -391,7 +396,7 @@ async def get_or_create_peers( @cache( key=PEER_CACHE_KEY_TEMPLATE, ttl=f"{settings.CACHE.DEFAULT_TTL_SECONDS}s", - prefix=get_cache_namespace(), + prefix=cache_prefix_namespace(), condition=NOT_NONE, ) @cache.locked( diff --git a/src/crud/session.py b/src/crud/session.py index 51ca1f13..216d4949 100644 --- a/src/crud/session.py +++ b/src/crud/session.py @@ -29,7 +29,8 @@ from sqlalchemy.types import BigInteger, Boolean from src import models, schemas from src.cache.client import ( cache, - get_cache_namespace, + cache_key_namespace, + cache_prefix_namespace, safe_cache_delete, safe_cache_set, ) @@ -67,13 +68,13 @@ class SessionDeletionResult: SESSION_CACHE_KEY_TEMPLATE = "v2:workspace:{workspace_name}:session:{session_name}" -SESSION_LOCK_PREFIX = f"{get_cache_namespace()}:lock:v2" +SESSION_LOCK_PREFIX = f"{cache_prefix_namespace()}:lock:v2" def session_cache_key(workspace_name: str, session_name: str) -> str: """Generate cache key for session.""" return ( - get_cache_namespace() + cache_key_namespace() + ":" + SESSION_CACHE_KEY_TEMPLATE.format( workspace_name=workspace_name, @@ -85,7 +86,7 @@ def session_cache_key(workspace_name: str, session_name: str) -> str: @cache( key=SESSION_CACHE_KEY_TEMPLATE, ttl=f"{settings.CACHE.DEFAULT_TTL_SECONDS}s", - prefix=get_cache_namespace(), + prefix=cache_prefix_namespace(), condition=NOT_NONE, ) @cache.locked( diff --git a/src/crud/workspace.py b/src/crud/workspace.py index a5042acd..05536bf2 100644 --- a/src/crud/workspace.py +++ b/src/crud/workspace.py @@ -15,7 +15,8 @@ from sqlalchemy.orm import make_transient_to_detached from src import models, schemas from src.cache.client import ( cache, - get_cache_namespace, + cache_key_namespace, + cache_prefix_namespace, safe_cache_delete, safe_cache_set, ) @@ -40,13 +41,13 @@ class WorkspaceDeletionResult: WORKSPACE_CACHE_KEY_TEMPLATE = "v2:workspace:{workspace_name}" -WORKSPACE_LOCK_PREFIX = f"{get_cache_namespace()}:lock:v2" +WORKSPACE_LOCK_PREFIX = f"{cache_prefix_namespace()}:lock:v2" def workspace_cache_key(workspace_name: str) -> str: """Generate cache key for workspace.""" return ( - get_cache_namespace() + cache_key_namespace() + ":" + WORKSPACE_CACHE_KEY_TEMPLATE.format(workspace_name=workspace_name) ) @@ -55,7 +56,7 @@ def workspace_cache_key(workspace_name: str) -> str: @cache( key=WORKSPACE_CACHE_KEY_TEMPLATE, ttl=f"{settings.CACHE.DEFAULT_TTL_SECONDS}s", - prefix=get_cache_namespace(), + prefix=cache_prefix_namespace(), condition=NOT_NONE, ) @cache.locked( diff --git a/tests/test_cache_key_namespace.py b/tests/test_cache_key_namespace.py new file mode 100644 index 00000000..14d2e7cc --- /dev/null +++ b/tests/test_cache_key_namespace.py @@ -0,0 +1,62 @@ +"""The namespace hash tag must survive both ways a cache key gets built. + +cashews format-substitutes `prefix=`, direct construction does not, so the two +need different spellings of the same tag. If they ever diverge, a write and its +invalidation land on different keys and nothing raises -- the cache just serves +stale rows. These tests are what fails instead. +""" + +import pytest +from redis.crc import key_slot + +from src.cache.client import ( + cache, + cache_key_namespace, + cache_prefix_namespace, + get_cache_namespace, +) +from src.crud.session import SESSION_CACHE_KEY_TEMPLATE, session_cache_key + + +def test_both_spellings_render_the_same_tag(): + ns = get_cache_namespace() + assert cache_key_namespace() == "{" + ns + "}" + # Doubled braces collapse to single ones when cashews formats the prefix. + assert cache_prefix_namespace().format() == cache_key_namespace() + + +@pytest.mark.asyncio +async def test_decorator_key_matches_helper_key(): + """The key the decorator writes is the key the helper computes.""" + + @cache( + key=SESSION_CACHE_KEY_TEMPLATE, + prefix=cache_prefix_namespace(), + ttl="60s", + ) + async def get_session(workspace_name: str, session_name: str) -> str: + # The names matter: cashews fills the key template from them. + return f"{workspace_name}/{session_name}" + + await get_session(workspace_name="w1", session_name="s1") + + written = [k async for k in cache.scan("*")] + assert session_cache_key("w1", "s1") in written + + +def test_one_namespace_hashes_to_one_slot(): + """Every key an instance writes shares a Redis Cluster slot.""" + keys = [ + session_cache_key("w1", "s1"), + session_cache_key("w2", "s2"), + f"{cache_key_namespace()}:lock:v2:anything", + ] + assert len({key_slot(k.encode()) for k in keys}) == 1 + + +def test_namespaces_hash_independently(): + """Tagging must not collapse the whole fleet onto one shard.""" + slots = { + key_slot(("{" + n + "}:v2:workspace:w").encode()) for n in ("a1", "b2", "c3") + } + assert len(slots) > 1 From 2dbc25093d1464b11ecc385f3eb32cc28c0f4726 Mon Sep 17 00:00:00 2001 From: ajspig <46900795+ajspig@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:53:12 -0400 Subject: [PATCH 6/7] chore: bump honcho-cli to 0.1.3 (#1067) --- honcho-cli/CHANGELOG.md | 7 +++++-- honcho-cli/pyproject.toml | 2 +- honcho-cli/src/honcho_cli/__init__.py | 2 +- uv.lock | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/honcho-cli/CHANGELOG.md b/honcho-cli/CHANGELOG.md index 7973341a..a8c1b12f 100644 --- a/honcho-cli/CHANGELOG.md +++ b/honcho-cli/CHANGELOG.md @@ -7,13 +7,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +## [0.1.3] - 2026-08-25 + ### Added -- `honcho session view` — session transcript table (`--last N`, `--page N --size M`, `--all`, `--reverse`, `--ids`, peer filter via `-p`). Content is shown verbatim, timestamps are normalized to UTC, and the command is read-only: unlike the other session commands it never get-or-creates the session +- `honcho start`, `honcho stop`, and `honcho status` — run a personal Honcho stack in Docker (API, deriver, Postgres, Redis). Profiles live under `~/.honcho/profiles/`. First start pins `ghcr.io/plastic-labs/honcho:latest` by digest and copies the image `config.toml`. Optional `--setup basic` / `--setup advanced` wizard writes LLM overrides to `.env` (#1029) +- `honcho session view` — session transcript table (`--last N`, `--page N --size M`, `--all`, `--reverse`, `--ids`, peer filter via `-p`). Content is shown verbatim, timestamps are normalized to UTC, and the command is read-only: unlike the other session commands it never get-or-creates the session (#1006) ### Fixed -- `honcho message list --last N` no longer stops at the first page of 50 — it walks pages to fill the requested window +- `honcho message list --last N` no longer stops at the first page of 50 — it walks pages to fill the requested window (#1006) ## [0.1.2] - 2026-07-20 diff --git a/honcho-cli/pyproject.toml b/honcho-cli/pyproject.toml index 2a5e59e2..f2fa4d0c 100644 --- a/honcho-cli/pyproject.toml +++ b/honcho-cli/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "honcho-cli" -version = "0.1.2" +version = "0.1.3" description = "A terminal for Honcho — memory that reasons." readme = "README.md" requires-python = ">=3.11" diff --git a/honcho-cli/src/honcho_cli/__init__.py b/honcho-cli/src/honcho_cli/__init__.py index 81efe6f5..5cbd28b3 100644 --- a/honcho-cli/src/honcho_cli/__init__.py +++ b/honcho-cli/src/honcho_cli/__init__.py @@ -1,3 +1,3 @@ """Honcho CLI — a terminal for Honcho.""" -__version__ = "0.1.2" +__version__ = "0.1.3" diff --git a/uv.lock b/uv.lock index 82f85e04..9217e608 100644 --- a/uv.lock +++ b/uv.lock @@ -1170,7 +1170,7 @@ dev = [{ name = "ruff", specifier = ">=0.11.13" }] [[package]] name = "honcho-cli" -version = "0.1.2" +version = "0.1.3" source = { editable = "honcho-cli" } dependencies = [ { name = "click" }, From 2f7658577e47ff62e40da697a9985e1e03d12bf1 Mon Sep 17 00:00:00 2001 From: Eugene Eisenstein Date: Tue, 25 Aug 2026 13:24:31 -0400 Subject: [PATCH 7/7] fix(scopes): scope observer sessions in SQL instead of a fetched name list (#1065) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_observation_context` resolved scope by fetching every session name the observer has a membership record in, then expanding that list into `session_name IN (...)` twice in one statement — once in the CTE and once in the outer select. That puts psycopg's 65535-bind-parameter ceiling at roughly 32,765 sessions, and the count only ever grows: the loose membership definition (`active_only=False`) counts sessions the peer has since left, so leaving a session does not shrink the scope. A workspace with tens of thousands of sessions for one peer produced a statement the driver could not serialize at all. Two new helpers in `crud.message` express the observer half as a correlated EXISTS over `session_peers`. Scope now costs two bind parameters regardless of membership size, and the membership query disappears (two round trips become one). The `session_peers` primary key is `(workspace_name, session_name, peer_name)`, so the correlated probe is an exact-match index hit. The caller-supplied allowlist stays an IN clause — it is route-capped at 1000 entries and carries none of the unbounded-growth risk. `resolve_session_scope` is left in place: three other callers still need the materialized list, including `_search_messages_external`, which sends session names to the vector store as a filter payload and cannot take SQL. Co-authored-by: Claude Opus 5 (1M context) --- src/crud/message.py | 86 ++++++++++++ src/utils/agent_tools.py | 23 ++- tests/conftest.py | 2 + tests/crud/test_session_scope_clauses.py | 169 +++++++++++++++++++++++ 4 files changed, 273 insertions(+), 7 deletions(-) create mode 100644 tests/crud/test_session_scope_clauses.py diff --git a/src/crud/message.py b/src/crud/message.py index fbfb1698..9759cb91 100644 --- a/src/crud/message.py +++ b/src/crud/message.py @@ -6,6 +6,7 @@ from typing import Any from nanoid import generate as generate_nanoid from sqlalchemy import ColumnElement, Select, and_, func, or_, select, text from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import InstrumentedAttribute from src import models, schemas from src.config import settings @@ -159,6 +160,91 @@ async def resolve_session_scope( return (allowed, False) if allowed else (None, True) +def observer_scope_clause( + workspace_name: str, + observer: str, + session_column: InstrumentedAttribute[str], +) -> ColumnElement[bool]: + """Correlated EXISTS restricting ``session_column`` to the observer's sessions. + + The in-database equivalent of filtering on :func:`get_peer_session_names`. + Prefer it whenever the scope feeds a single SQL statement: a peer's + membership count is unbounded, and materializing the names turns each one + into its own bind parameter. The PostgreSQL wire protocol caps parameters + at 65535 per statement, so a peer in enough sessions produces a query the + driver cannot serialize at all — and the resulting error carries every + parameter in its text. + + Matches the loose membership definition ``get_peer_session_names`` uses by + default: any membership record grants visibility, whether or not the peer + has since left the session. + """ + session_peers = models.session_peers_table + return ( + select(1) + .where(session_peers.c.workspace_name == workspace_name) + .where(session_peers.c.peer_name == observer) + .where(session_peers.c.session_name == session_column) + .exists() + ) + + +def resolve_session_scope_clauses( + workspace_name: str, + session_name: str | None, + session_allowlist: list[str] | None, + observer: str | None, + session_column: InstrumentedAttribute[str], +) -> tuple[list[ColumnElement[bool]], bool]: + """SQL-side counterpart to :func:`resolve_session_scope`. + + Returns ``(clauses, deny)``, where ``clauses`` are ANDed onto the caller's + statement and ``deny=True`` means return an empty result without querying. + Unlike :func:`resolve_session_scope` this touches no database and grows no + bind parameters with the observer's session count — the observer half + becomes a correlated EXISTS instead of an ``IN`` over fetched names. + + Scoping matches :func:`resolve_session_scope` case for case, with one + deliberate difference: where that function returns ``deny=True`` because an + observer's membership (or its intersection with the allowlist) is empty, + this returns clauses that simply match no rows. Callers reach the same empty + result, at the cost of running one indexed query that returns nothing. + + ``session_allowlist`` stays an ``IN`` clause: it is caller-supplied and + therefore bounded, so it carries none of the unbounded-growth risk. + + Args: + workspace_name: Name of the workspace + session_name: A single pinned session, if the caller named one. The + caller applies its own equality filter; this function only checks + the allowlist permits it. + session_allowlist: Optional session allowlist. ``None`` is + unrestricted; an empty list fails closed. + observer: When set, scope is limited to this peer's sessions + session_column: The session-name column to scope, e.g. + ``models.Message.session_name`` + """ + if session_name: + # Fail closed when the allowlist forbids the pinned session, matching + # `resolve_session_scope` — routes guard this too, but the dialectic + # tools reach CRUD directly, so enforce it at the boundary. + if session_allowlist is not None and session_name not in session_allowlist: + return [], True + return [], False + + clauses: list[ColumnElement[bool]] = [] + + if observer is not None: + clauses.append(observer_scope_clause(workspace_name, observer, session_column)) + + if session_allowlist is not None: + if not session_allowlist: + return [], True + clauses.append(session_column.in_(session_allowlist)) + + return clauses, False + + def _apply_token_limit( base_conditions: list[ColumnElement[Any]], token_limit: int ) -> Select[tuple[models.Message]]: diff --git a/src/utils/agent_tools.py b/src/utils/agent_tools.py index 5f87d455..e31b4d62 100644 --- a/src/utils/agent_tools.py +++ b/src/utils/agent_tools.py @@ -1273,10 +1273,19 @@ async def get_observation_context( if not message_ids: return [] - from src.crud.message import resolve_session_scope + from src.crud.message import resolve_session_scope_clauses - allowed_session_names, deny = await resolve_session_scope( - db, workspace_name, session_name, session_allowlist, observer + # Scope as SQL rather than as a fetched name list. The scope is applied to + # both the CTE and the outer select, so a materialized list would spend two + # bind parameters per session the observer belongs to — enough sessions and + # the statement exceeds the driver's 65535-parameter ceiling and cannot be + # sent at all. + scope_clauses, deny = resolve_session_scope_clauses( + workspace_name, + session_name, + session_allowlist, + observer, + models.Message.session_name, ) if deny: return [] @@ -1290,8 +1299,8 @@ async def get_observation_context( if session_name: stmt = stmt.where(models.Message.session_name == session_name) - elif allowed_session_names is not None: - stmt = stmt.where(models.Message.session_name.in_(allowed_session_names)) + for clause in scope_clauses: + stmt = stmt.where(clause) target_seqs_cte = stmt.cte("target_seqs") @@ -1314,8 +1323,8 @@ async def get_observation_context( if session_name: stmt = stmt.where(models.Message.session_name == session_name) - elif allowed_session_names is not None: - stmt = stmt.where(models.Message.session_name.in_(allowed_session_names)) + for clause in scope_clauses: + stmt = stmt.where(clause) result = await db.execute(stmt) messages = list(result.scalars().all()) diff --git a/tests/conftest.py b/tests/conftest.py index 7d9e81da..090d5395 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -86,6 +86,8 @@ _RUNTIME_MOCK_TEST_BLOCKLIST_PREFIXES = ( # LLM transport tests mock providers directly and don't need database/runtime setup. "tests/utils/test_length_finish_reason.py", "tests/utils/test_clients.py", + # Session-scope SQL shape — asserts on compiled statements, never executes one. + "tests/crud/test_session_scope_clauses.py", # Pure JWT scope tests — operate on src.security directly, no DB needed. "tests/test_security.py", "tests/test_generate_jwt_script.py", diff --git a/tests/crud/test_session_scope_clauses.py b/tests/crud/test_session_scope_clauses.py new file mode 100644 index 00000000..a4a77c60 --- /dev/null +++ b/tests/crud/test_session_scope_clauses.py @@ -0,0 +1,169 @@ +"""Observer session scope is enforced in SQL, not as a fetched name list. + +A peer's session-membership count is unbounded. Materializing it turns every +session name into its own bind parameter, and the PostgreSQL wire protocol caps +parameters at 65535 per statement, so a peer in enough sessions yields a +statement the driver refuses to serialize. `get_observation_context` applies the +scope twice in one statement, which halves that ceiling to ~32.7k sessions. + +These tests assert on compiled SQL and never execute a statement. +""" + +from typing import Any + +import pytest +from sqlalchemy import Select, select +from sqlalchemy.dialects import postgresql + +from src import models +from src.crud.message import resolve_session_scope_clauses +from src.utils.agent_tools import get_observation_context + + +def _compile(stmt: Select[Any]) -> tuple[str, dict[str, Any]]: + compiled = stmt.compile( + dialect=postgresql.dialect(), + compile_kwargs={"render_postcompile": True}, + ) + return str(compiled), dict(compiled.params) + + +class _FakeResult: + def scalars(self) -> "_FakeResult": + return self + + def all(self) -> list[Any]: + return [] + + +class _CapturingDB: + """Captures statements instead of executing them.""" + + def __init__(self) -> None: + self.statements: list[Any] = [] + + async def execute(self, stmt: Any) -> _FakeResult: + self.statements.append(stmt) + return _FakeResult() + + +@pytest.mark.parametrize( + ("session_allowlist", "observer", "expect_exists", "expected_params"), + [ + # Observer only: membership becomes a correlated EXISTS, so only the + # workspace and peer are bound — never the session names, which is what + # keeps the parameter count from growing with membership. + (None, "observer-peer", True, ["observer-peer", "workspace"]), + # Allowlist only: caller-supplied and therefore bounded, so IN is fine. + (["s1", "s2"], None, False, ["s1", "s2"]), + # Both: the EXISTS is intersected with the bounded IN. + (["s1"], "observer-peer", True, ["observer-peer", "s1", "workspace"]), + # Neither: unrestricted, nothing filtered and nothing bound. + (None, None, False, []), + ], + ids=["observer-only", "allowlist-only", "observer-and-allowlist", "unrestricted"], +) +def test_scope_clause_shape( + session_allowlist: list[str] | None, + observer: str | None, + expect_exists: bool, + expected_params: list[str], +) -> None: + clauses, deny = resolve_session_scope_clauses( + "workspace", + None, + session_allowlist, + observer, + models.Message.session_name, + ) + + assert not deny + + sql, params = _compile(select(models.Message.public_id).where(*clauses)) + + assert ("EXISTS" in sql.upper()) is expect_exists + assert ("session_peers" in sql) is expect_exists + assert sorted(params.values()) == expected_params + + +@pytest.mark.parametrize( + ("session_name", "session_allowlist", "observer"), + [ + # An empty allowlist fails closed rather than matching everything. + (None, [], "observer-peer"), + (None, [], None), + # A pinned session the allowlist forbids fails closed. + ("s9", ["s1", "s2"], "observer-peer"), + ], + ids=[ + "empty-allowlist-with-observer", + "empty-allowlist-without-observer", + "pinned-session-not-in-allowlist", + ], +) +def test_scope_fails_closed( + session_name: str | None, + session_allowlist: list[str] | None, + observer: str | None, +) -> None: + clauses, deny = resolve_session_scope_clauses( + "workspace", + session_name, + session_allowlist, + observer, + models.Message.session_name, + ) + + assert deny + assert clauses == [] + + +@pytest.mark.asyncio +async def test_get_observation_context_scope_costs_no_per_session_parameters() -> None: + """The statement's parameter count depends on message_ids, not membership.""" + db = _CapturingDB() + message_ids = [f"msg-{i}" for i in range(5)] + + await get_observation_context( + db, # pyright: ignore[reportArgumentType] + "workspace", + None, + message_ids, + observer="observer-peer", + ) + + assert len(db.statements) == 1 + sql, params = _compile(db.statements[0]) + + # The scope must be applied to *both* the CTE and the outer select — a + # materialized list would have cost two parameters per session there. + # Count the subquery's FROM rather than EXISTS: the adjacency check is also + # an EXISTS, so counting those would pass with the CTE's scope missing. + assert sql.count("FROM public.session_peers") == 2 + # Both must correlate to the enclosing `messages` row. An uncorrelated + # subquery compiles just as happily and would silently drop scoping + # instead of enforcing it. + assert sql.count("session_peers.session_name = public.messages.session_name") == 2 + + # Everything bound is either a message id, the workspace, the peer, or the + # ±1 adjacency window — nothing that scales with the peer's session count. + expected = set(message_ids) | {"workspace", "observer-peer", -1, 1} + assert set(params.values()) <= expected + + +@pytest.mark.asyncio +async def test_get_observation_context_denies_without_querying() -> None: + """Fail-closed scopes must not reach the database at all.""" + db = _CapturingDB() + + result = await get_observation_context( + db, # pyright: ignore[reportArgumentType] + "workspace", + None, + ["msg-1"], + observer="observer-peer", + session_allowlist=[], + ) + + assert result == [] + assert db.statements == []