From 58004dd7f036525ab9a15163fb3d9d8e8fd48edb Mon Sep 17 00:00:00 2001 From: ajspig Date: Mon, 13 Apr 2026 16:50:47 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20polish=20command=20surfaces=20=E2=80=94?= =?UTF-8?q?=20scoping,=20validation,=20perf,=20consistency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/honcho_cli/commands/conclusion.py | 25 +++++++-- honcho-cli/src/honcho_cli/commands/message.py | 19 +++---- honcho-cli/src/honcho_cli/commands/peer.py | 34 +++++++---- honcho-cli/src/honcho_cli/commands/session.py | 56 ++++++++++++------- .../src/honcho_cli/commands/workspace.py | 18 +++--- honcho-cli/src/honcho_cli/common.py | 3 + honcho-cli/src/honcho_cli/config.py | 5 ++ honcho-cli/src/honcho_cli/main.py | 40 ++++++++++--- 8 files changed, 138 insertions(+), 62 deletions(-) diff --git a/honcho-cli/src/honcho_cli/commands/conclusion.py b/honcho-cli/src/honcho_cli/commands/conclusion.py index 522b5660..969e5e87 100644 --- a/honcho-cli/src/honcho_cli/commands/conclusion.py +++ b/honcho-cli/src/honcho_cli/commands/conclusion.py @@ -181,13 +181,14 @@ def delete( yes: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), peer: Optional[str] = typer.Option(None, "--peer", "-p", help="Override peer ID"), + quiet: bool = typer.Option(False, "--quiet", "-q", help="Suppress status messages"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Delete a conclusion.""" from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, peer=peer) + handle_cmd_flags(json_output=json_output, quiet=quiet, workspace=workspace, peer=peer) validate_resource_id(conclusion_id, "conclusion") client, config = get_client() @@ -197,11 +198,27 @@ def delete( print_error("NO_PEER", "Observer peer ID required. Use --observer or set default peer.") raise typer.Exit(1) - if not yes: - typer.confirm(f"Delete conclusion '{conclusion_id}'?", abort=True) - p = client.peer(observer) + if not yes: + # Show a short preview so the user knows which conclusion is targeted. + preview_content: str | None = None + try: + scope_preview = p.conclusions_of(observed) if observed else p.conclusions + for c in scope_preview.list(size=100).items: + if c.id == conclusion_id: + preview_content = c.content + break + except Exception: + pass + typer.echo( + f" id: {conclusion_id}\n" + f" observer: {observer}\n" + f" observed: {observed or '(self)'}\n" + f" content: {(preview_content[:200] + '...') if preview_content and len(preview_content) > 200 else (preview_content or '(not found in first 100)')}" + ) + typer.confirm(f"Delete conclusion '{conclusion_id}'?", abort=True) + try: if observed: scope = p.conclusions_of(observed) diff --git a/honcho-cli/src/honcho_cli/commands/message.py b/honcho-cli/src/honcho_cli/commands/message.py index 13240503..c9246e27 100644 --- a/honcho-cli/src/honcho_cli/commands/message.py +++ b/honcho-cli/src/honcho_cli/commands/message.py @@ -25,6 +25,7 @@ def list_messages( brief: bool = typer.Option(False, "--brief", help="Show only IDs, peer, token count, and created_at (no content)"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), session: Optional[str] = typer.Option(None, "--session", "-s", help="Override session ID"), + quiet: bool = typer.Option(False, "--quiet", "-q", help="Suppress status messages"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """List messages in a session.""" @@ -32,7 +33,7 @@ def list_messages( from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session) + handle_cmd_flags(json_output=json_output, quiet=quiet, workspace=workspace, session=session) sid = _get_session_id(session_id) client, config = get_client() sess = client.session(sid) @@ -102,16 +103,14 @@ def get_message( client, config = get_client() try: - # Use raw HTTP to get a single message + # Hit the direct message endpoint instead of paging the session. + from honcho.http import routes + from honcho.api_types import MessageResponse + from honcho.message import Message + sess = client.session(sid) - msgs = list(sess.messages()) - msg = next((m for m in msgs if m.id == message_id), None) - - if msg is None: - from honcho_cli.output import print_error - - print_error("MESSAGE_NOT_FOUND", f"Message '{message_id}' not found in session '{sid}'", {"message_id": message_id, "session_id": sid}) - raise typer.Exit(1) + data = client._http.get(routes.message(sess.workspace_id, sess.id, message_id)) + msg = Message.from_api_response(MessageResponse.model_validate(data)) print_result({ "id": msg.id, diff --git a/honcho-cli/src/honcho_cli/commands/peer.py b/honcho-cli/src/honcho_cli/commands/peer.py index 3bb487ef..46fa89f4 100644 --- a/honcho-cli/src/honcho_cli/commands/peer.py +++ b/honcho-cli/src/honcho_cli/commands/peer.py @@ -77,19 +77,24 @@ def inspect( try: card = p.get_card() - sessions = list(p.sessions()) - conclusions = list(p.conclusions.list()) + # First page only; SyncPage.total (when the server supplies it) is + # authoritative for counts without walking every page. + session_page = p.sessions() + conclusion_page = p.conclusions.list(size=10) + + session_items = session_page.items + conclusion_items = conclusion_page.items result = { "id": pid, "card": card, - "session_count": len(sessions), - "conclusion_count": len(conclusions), + "session_count": session_page.total if session_page.total is not None else len(session_items), + "conclusion_count": conclusion_page.total if conclusion_page.total is not None else len(conclusion_items), "recent_conclusions": [ {"id": c.id, "content": c.content[:200], "created_at": str(c.created_at)} - for c in conclusions[:10] + for c in conclusion_items ], - "sessions": [{"id": s.id} for s in sessions[:10]], + "sessions": [{"id": s.id} for s in session_items[:10]], } print_result(result) except Exception as e: @@ -124,7 +129,6 @@ def card( def chat( query: str = typer.Argument(help="Question to ask about the peer"), target: Optional[str] = typer.Option(None, help="Target peer for perspective"), - session: Optional[str] = typer.Option(None, help="Session context"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), peer: Optional[str] = typer.Option(None, "--peer", "-p", help="Peer ID (uses default if omitted)"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), @@ -139,7 +143,9 @@ def chat( p = client.peer(pid) try: - response = p.chat(query, target=target, session=session) + # Session scope comes from the global -s pipeline (config.session_id), + # keeping chat consistent with every other peer/session command. + response = p.chat(query, target=target, session=config.session_id or None) print_result({"peer_id": pid, "query": query, "response": response}) except Exception as e: _handle_error(e, "peer", pid) @@ -208,10 +214,15 @@ def create_peer( try: p = client.peer(pid, configuration=peer_config, metadata=parsed_metadata) + # get-or-create semantics: the server may have an existing config that + # differs from what we passed. Read back what's actually stored so the + # output reflects server state, not the (possibly-None) input. + server_config = _config_to_dict(p.get_configuration()) + server_metadata = p.get_metadata() result = { "peer_id": p.id, - "metadata": parsed_metadata, - "configuration": {"observe_me": observe_me} if observe_me is not None else None, + "metadata": server_metadata, + "configuration": server_config, } print_result(result) except Exception as e: @@ -276,7 +287,6 @@ def set_metadata( def representation( peer_id: Optional[str] = typer.Argument(None, help="Peer ID (uses default if omitted)"), target: Optional[str] = typer.Option(None, help="Target peer to get representation about"), - session: Optional[str] = typer.Option(None, help="Scope representation to a session"), search_query: Optional[str] = typer.Option(None, help="Semantic search query to filter conclusions"), max_conclusions: Optional[int] = typer.Option(None, help="Maximum number of conclusions to include"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), @@ -295,7 +305,7 @@ def representation( try: result = p.representation( target=target, - session=session, + session=config.session_id or None, search_query=search_query, max_conclusions=max_conclusions, ) diff --git a/honcho-cli/src/honcho_cli/commands/session.py b/honcho-cli/src/honcho_cli/commands/session.py index 9df08db3..6c9e9bb9 100644 --- a/honcho-cli/src/honcho_cli/commands/session.py +++ b/honcho-cli/src/honcho_cli/commands/session.py @@ -34,7 +34,6 @@ def _get_session_id(session_id: str | None) -> str: def list_sessions( peer_id: Optional[str] = typer.Option(None, "--peer", "-p", help="Filter by peer"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), - session: Optional[str] = typer.Option(None, "--session", "-s", help="Override session ID"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """List sessions in the workspace.""" @@ -42,7 +41,7 @@ def list_sessions( from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session) + handle_cmd_flags(json_output=json_output, workspace=workspace) client, config = get_client() try: @@ -84,17 +83,22 @@ def inspect( try: peers = sess.peers() - messages = list(sess.messages()) + msg_page = sess.messages() summaries = sess.summaries() sess_config = sess.get_configuration() from honcho_cli.commands.workspace import _compact_config + # Use SyncPage.total when the server provides it; fall back to a + # first-page count to avoid paginating the full session just for a + # count. + message_count = msg_page.total if msg_page.total is not None else len(msg_page.items) + raw_config = _config_to_dict(sess_config) if sess_config else None result = { "id": sid, "peers": [{"id": p.id} for p in peers], - "message_count": len(messages), + "message_count": message_count, "summaries": { "short": summaries.short_summary if hasattr(summaries, "short_summary") else None, "long": summaries.long_summary if hasattr(summaries, "long_summary") else None, @@ -208,21 +212,33 @@ def delete( yes: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), session: Optional[str] = typer.Option(None, "--session", "-s", help="Override session ID"), + quiet: bool = typer.Option(False, "--quiet", "-q", help="Suppress status messages"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Delete a session and all its data. Destructive — requires --yes or interactive confirm.""" from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session) + handle_cmd_flags(json_output=json_output, quiet=quiet, workspace=workspace, session=session) sid = _get_session_id(session_id) client, config = get_client() + sess = client.session(sid) if not yes: + # Show a short preview so the user knows what's about to disappear. + try: + peers = sess.peers() + message_count = len(sess.messages().items) + peer_ids = [p.id for p in peers] + typer.echo( + f" session: {sid}\n" + f" peers: {', '.join(peer_ids) if peer_ids else '(none)'}\n" + f" messages: {message_count}+ (first page)" + ) + except Exception: + pass typer.confirm(f"Delete session '{sid}' and all its messages, conclusions, and queue items?", abort=True) - sess = client.session(sid) - try: sess.delete() status(f"Session '{sid}' deleted") @@ -257,8 +273,8 @@ def session_peers( @app.command("add-peers") def add_peers( + session_id: str = typer.Argument(help="Session ID"), peer_ids: List[str] = typer.Argument(help="Peer IDs to add to the session"), - session_id: Optional[str] = typer.Option(None, "--session", "-s", help="Session ID (uses default if omitted)"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: @@ -266,7 +282,7 @@ def add_peers( from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session_id) + handle_cmd_flags(json_output=json_output, workspace=workspace) sid = _get_session_id(session_id) client, config = get_client() sess = client.session(sid) @@ -280,8 +296,8 @@ def add_peers( @app.command("remove-peers") def remove_peers( + session_id: str = typer.Argument(help="Session ID"), peer_ids: List[str] = typer.Argument(help="Peer IDs to remove from the session"), - session_id: Optional[str] = typer.Option(None, "--session", "-s", help="Session ID (uses default if omitted)"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: @@ -289,7 +305,7 @@ def remove_peers( from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session_id) + handle_cmd_flags(json_output=json_output, workspace=workspace) sid = _get_session_id(session_id) client, config = get_client() sess = client.session(sid) @@ -304,17 +320,17 @@ def remove_peers( @app.command() def search( query: str = typer.Argument(help="Search query"), + session_id: Optional[str] = typer.Argument(None, help="Session ID (uses default if omitted)"), limit: int = typer.Option(10, help="Max results"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), - session: Optional[str] = typer.Option(None, "--session", "-s", help="Session ID (uses default if omitted)"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Search messages in a session.""" from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session) - sid = _get_session_id(None) + handle_cmd_flags(json_output=json_output, workspace=workspace) + sid = _get_session_id(session_id) client, config = get_client() sess = client.session(sid) @@ -337,19 +353,19 @@ def search( @app.command() def representation( peer_id: str = typer.Argument(help="Peer ID to get representation for"), + session_id: Optional[str] = typer.Argument(None, help="Session ID (uses default if omitted)"), target: Optional[str] = typer.Option(None, help="Target peer (what peer_id knows about target)"), search_query: Optional[str] = typer.Option(None, help="Semantic search query to filter conclusions"), max_conclusions: Optional[int] = typer.Option(None, help="Maximum number of conclusions to include"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), - session: Optional[str] = typer.Option(None, "--session", "-s", help="Session ID (uses default if omitted)"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Get the representation of a peer within a session.""" from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session) - sid = _get_session_id(None) + handle_cmd_flags(json_output=json_output, workspace=workspace) + sid = _get_session_id(session_id) client, config = get_client() sess = client.session(sid) @@ -391,16 +407,16 @@ def get_metadata( @app.command("set-metadata") def set_metadata( metadata: str = typer.Argument(help="JSON metadata to set (e.g. '{\"key\": \"value\"}')"), + session_id: Optional[str] = typer.Argument(None, help="Session ID (uses default if omitted)"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), - session: Optional[str] = typer.Option(None, "--session", "-s", help="Session ID (uses default if omitted)"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Set metadata for a session.""" from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output, workspace=workspace, session=session) - sid = _get_session_id(None) + handle_cmd_flags(json_output=json_output, workspace=workspace) + sid = _get_session_id(session_id) client, config = get_client() try: diff --git a/honcho-cli/src/honcho_cli/commands/workspace.py b/honcho-cli/src/honcho_cli/commands/workspace.py index a5ad7a29..652c8132 100644 --- a/honcho-cli/src/honcho_cli/commands/workspace.py +++ b/honcho-cli/src/honcho_cli/commands/workspace.py @@ -76,7 +76,7 @@ def inspect( handle_cmd_flags(json_output=json_output, workspace=workspace) wid = _get_workspace_id(workspace_id) - client, config = get_client() + client, config = get_client(require_workspace=False) # Override workspace if positional arg given if workspace_id: @@ -116,6 +116,7 @@ def delete( yes: bool = typer.Option(False, "--yes", "-y", help="Skip confirmation prompt (for scripted/agent use)"), cascade: bool = typer.Option(False, "--cascade", help="Delete all sessions before deleting the workspace"), dry_run: bool = typer.Option(False, "--dry-run", help="Show what would be deleted without deleting"), + quiet: bool = typer.Option(False, "--quiet", "-q", help="Suppress status messages"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Delete a workspace. Use --dry-run first to see what will be deleted. @@ -126,10 +127,12 @@ def delete( from honcho_cli.common import handle_cmd_flags from honcho_cli.main import get_client - handle_cmd_flags(json_output=json_output) + handle_cmd_flags(json_output=json_output, quiet=quiet) validate_resource_id(workspace_id, "workspace") - client, config = get_client() + # workspace_id is a required positional and we rebuild the client with it + # immediately, so the default-workspace guard isn't needed here. + client, config = get_client(require_workspace=False) ws_client = _with_workspace(client, workspace_id) # Always fetch sessions for dry-run or cascade @@ -175,7 +178,6 @@ def delete( @app.command() def search( query: str = typer.Argument(help="Search query"), - workspace_id: Optional[str] = typer.Option(None, help="Workspace ID"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), limit: int = typer.Option(10, help="Max results"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), @@ -186,7 +188,7 @@ def search( handle_cmd_flags(json_output=json_output, workspace=workspace) - wid = _get_workspace_id(workspace_id) + wid = _get_workspace_id(None) client, config = get_client() try: @@ -208,11 +210,9 @@ def search( @app.command("queue-status") def queue_status( - workspace_id: Optional[str] = typer.Option(None, help="Workspace ID"), workspace: Optional[str] = typer.Option(None, "--workspace", "-w", help="Override workspace ID"), observer: Optional[str] = typer.Option(None, help="Filter by observer peer"), sender: Optional[str] = typer.Option(None, help="Filter by sender peer"), - session: Optional[str] = typer.Option(None, help="Filter by session"), json_output: bool = typer.Option(False, "--json", help="Force JSON output"), ) -> None: """Get queue processing status.""" @@ -221,11 +221,11 @@ def queue_status( handle_cmd_flags(json_output=json_output, workspace=workspace) - _get_workspace_id(workspace_id) + _get_workspace_id(None) client, config = get_client() try: - result = client.queue_status(observer=observer, sender=sender, session=session) + result = client.queue_status(observer=observer, sender=sender, session=config.session_id or None) print_result(result.__dict__ if hasattr(result, "__dict__") else result) except Exception as e: _handle_error(e, "queue", "status") diff --git a/honcho-cli/src/honcho_cli/common.py b/honcho-cli/src/honcho_cli/common.py index e0e67f87..65e9de42 100644 --- a/honcho-cli/src/honcho_cli/common.py +++ b/honcho-cli/src/honcho_cli/common.py @@ -18,6 +18,7 @@ from honcho_cli.output import set_json_mode, set_quiet_mode def handle_cmd_flags( json_output: bool = False, + quiet: bool = False, workspace: str | None = None, peer: str | None = None, session: str | None = None, @@ -25,6 +26,8 @@ def handle_cmd_flags( """Apply command-level flags. Idempotent if already set by group callback.""" if json_output: set_json_mode(True) + if quiet: + set_quiet_mode(True) from honcho_cli.main import _global_overrides diff --git a/honcho-cli/src/honcho_cli/config.py b/honcho-cli/src/honcho_cli/config.py index 85358d33..e638f654 100644 --- a/honcho-cli/src/honcho_cli/config.py +++ b/honcho-cli/src/honcho_cli/config.py @@ -82,6 +82,11 @@ class CLIConfig: val = os.environ.get(env_var) if val: setattr(config, fld_name, val) + elif val == "": + # SDK reads these env vars directly and crashes on empty + # strings with a Pydantic ValidationError. Drop them so the + # SDK falls back to kwargs / defaults. + os.environ.pop(env_var, None) return config diff --git a/honcho-cli/src/honcho_cli/main.py b/honcho-cli/src/honcho_cli/main.py index bf70f560..91bc4b6d 100644 --- a/honcho-cli/src/honcho_cli/main.py +++ b/honcho-cli/src/honcho_cli/main.py @@ -29,9 +29,26 @@ app = typer.Typer( ) +def _json_requested_early() -> bool: + """Best-effort JSON detection before Typer parses flags. + + version_callback is eager and fires before set_json_mode() runs, so we + can't call use_json() here. Mirror its logic against argv/env/TTY. + """ + import os + import sys + + return ( + "--json" in sys.argv + or os.environ.get("HONCHO_JSON", "").lower() in ("1", "true") + or not sys.stdout.isatty() + ) + + def version_callback(value: bool) -> None: if value: - print(BANNER) + if not _json_requested_early(): + print(BANNER) print(f" honcho-cli {__version__}") raise typer.Exit() @@ -58,9 +75,12 @@ def main( if ctx.invoked_subcommand is None: from rich.console import Console + from honcho_cli.output import use_json + console = Console() - console.print(f"[bold #B6DAFD]{BANNER}[/bold #B6DAFD]") - console.print(f" [dim]v{__version__}[/dim]\n") + if not use_json(): + console.print(f"[bold #B6DAFD]{BANNER}[/bold #B6DAFD]") + console.print(f" [dim]v{__version__}[/dim]\n") console.print(ctx.get_help()) raise typer.Exit() @@ -74,17 +94,23 @@ _global_overrides: dict[str, str | None] = { def get_resolved_config(): - """Get config with global flag overrides applied.""" + """Get config with global flag overrides applied. + + Overrides flow through ``validate_resource_id`` so that a malformed + ``-w``/``-p``/``-s`` value fails fast with a structured error rather than + reaching the API and surfacing as an opaque ``UNKNOWN_ERROR``. + """ from honcho_cli.config import CLIConfig + from honcho_cli.validation import validate_resource_id config = CLIConfig.load() if _global_overrides["workspace"]: - config.workspace_id = _global_overrides["workspace"] + config.workspace_id = validate_resource_id(_global_overrides["workspace"], "workspace") if _global_overrides["peer"]: - config.peer_id = _global_overrides["peer"] + config.peer_id = validate_resource_id(_global_overrides["peer"], "peer") if _global_overrides["session"]: - config.session_id = _global_overrides["session"] + config.session_id = validate_resource_id(_global_overrides["session"], "session") return config