diff --git a/honcho-cli/src/honcho_cli/commands/conclusion.py b/honcho-cli/src/honcho_cli/commands/conclusion.py index 969e5e87..d3c4c3f8 100644 --- a/honcho-cli/src/honcho_cli/commands/conclusion.py +++ b/honcho-cli/src/honcho_cli/commands/conclusion.py @@ -201,22 +201,17 @@ def delete( 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)')}" - ) + # SDK doesn't expose a get-by-id on ConclusionScope, so we can't + # preview content cheaply — don't paginate the list just to + # decorate the prompt. Show identifying fields only. + from honcho_cli.output import use_json + + if not use_json(): + typer.echo( + f" id: {conclusion_id}\n" + f" observer: {observer}\n" + f" observed: {observed or '(self)'}" + ) typer.confirm(f"Delete conclusion '{conclusion_id}'?", abort=True) try: diff --git a/honcho-cli/src/honcho_cli/commands/message.py b/honcho-cli/src/honcho_cli/commands/message.py index c9246e27..675dd8da 100644 --- a/honcho-cli/src/honcho_cli/commands/message.py +++ b/honcho-cli/src/honcho_cli/commands/message.py @@ -39,9 +39,10 @@ def list_messages( sess = client.session(sid) try: - # SDK returns most-recent-first. Default case (newest N) only needs - # the first page. --reverse (oldest N) still walks all pages until - # the SDK accepts order=asc on session.messages(). + # Server supports ?reverse=true on messages/list, but the Python + # SDK doesn't forward it from Session.messages() yet. Until then, + # --reverse walks every page via the SDK iterator and slices + # — O(pages) in the session size. Safe for small sessions if not reverse: msgs = sess.messages().items[:last] else: diff --git a/honcho-cli/src/honcho_cli/commands/peer.py b/honcho-cli/src/honcho_cli/commands/peer.py index 46fa89f4..5a7f9123 100644 --- a/honcho-cli/src/honcho_cli/commands/peer.py +++ b/honcho-cli/src/honcho_cli/commands/peer.py @@ -214,16 +214,14 @@ 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": server_metadata, - "configuration": server_config, - } + # Only round-trip to the server when the caller passed config or + # metadata — in that case get-or-create may have returned a + # pre-existing peer and the echoed output would lie. When no input + # was passed, skip the two extra API calls entirely. + result: dict[str, object] = {"peer_id": p.id} + if peer_config is not None or parsed_metadata is not None: + result["metadata"] = p.get_metadata() + result["configuration"] = _config_to_dict(p.get_configuration()) print_result(result) except Exception as e: _handle_error(e, "peer", pid) diff --git a/honcho-cli/src/honcho_cli/commands/session.py b/honcho-cli/src/honcho_cli/commands/session.py index 754452ab..abe15374 100644 --- a/honcho-cli/src/honcho_cli/commands/session.py +++ b/honcho-cli/src/honcho_cli/commands/session.py @@ -89,21 +89,27 @@ def inspect( 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) + # Use SyncPage.total when the server provides it; otherwise fall back + # to the first-page count and flag that we did so, so scripted + # callers don't treat a lower bound as a total. + if msg_page.total is not None: + message_count = msg_page.total + message_count_is_total = True + else: + message_count = len(msg_page.items) + message_count_is_total = False 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": message_count, + "message_count_is_total": message_count_is_total, "summaries": { "short": summaries.short_summary if hasattr(summaries, "short_summary") else None, "long": summaries.long_summary if hasattr(summaries, "long_summary") else None, }, - "configuration": _compact_config(raw_config) if isinstance(raw_config, dict) else raw_config, + "configuration": _compact_config(raw_config) if raw_config else None, } print_result(result) except Exception as e: @@ -184,17 +190,30 @@ def delete( 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 + # Only in interactive/TTY mode — scripted (--json) callers already + # know what they're deleting, and they still need to pass --yes. + # Narrow the except to HonchoError so auth/network failures surface + # before the user types 'y' on a destructive op. + from honcho import HonchoError + + from honcho_cli.output import use_json + + if not use_json(): + try: + peers = sess.peers() + msg_page = sess.messages() + if msg_page.total is not None: + msg_count_str = str(msg_page.total) + else: + msg_count_str = f"{len(msg_page.items)} (first page; more may exist)" + 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: {msg_count_str}" + ) + except HonchoError as preview_err: + status(f"preview unavailable: {preview_err}") typer.confirm(f"Delete session '{sid}' and all its messages, conclusions, and queue items?", abort=True) try: diff --git a/honcho-cli/src/honcho_cli/commands/workspace.py b/honcho-cli/src/honcho_cli/commands/workspace.py index 652c8132..af876e32 100644 --- a/honcho-cli/src/honcho_cli/commands/workspace.py +++ b/honcho-cli/src/honcho_cli/commands/workspace.py @@ -37,10 +37,7 @@ def _raw_list(page) -> list: return items -def _compact_config(config_dict: dict) -> dict | str: - """Return '(defaults)' if all config values are None, else the dict.""" - if all(v is None for v in config_dict.values()): - return "(defaults)" +def _compact_config(config_dict: dict) -> dict: return config_dict @@ -255,21 +252,42 @@ def _config_to_dict(config) -> dict: def _handle_error(e: Exception, resource: str, resource_id: str) -> None: - """Handle SDK exceptions with structured error output.""" - error_str = str(e) - if "404" in error_str or "not found" in error_str.lower(): + """Handle SDK exceptions with structured error output. + + Dispatches on the SDK's typed exception hierarchy + (``honcho.http.exceptions``) and falls back to its ``status`` field for + any APIError subclass we don't enumerate. Substring matching on the + message is used only as a last-ditch fallback for non-SDK exceptions. + """ + from honcho import ( + APIError, + AuthenticationError, + NotFoundError, + PermissionDeniedError, + ServerError, + ) + + if isinstance(e, NotFoundError): print_error( f"{resource.upper()}_NOT_FOUND", f"{resource.title()} '{resource_id}' not found", {resource: resource_id}, ) raise typer.Exit(1) - elif "401" in error_str or "403" in error_str or "auth" in error_str.lower(): - print_error("AUTH_ERROR", f"Authentication failed: {error_str}", {}) + if isinstance(e, (AuthenticationError, PermissionDeniedError)): + print_error("AUTH_ERROR", f"Authentication failed: {e}", {}) raise typer.Exit(3) - elif "500" in error_str or "server" in error_str.lower(): - print_error("SERVER_ERROR", f"Server error: {error_str}", {resource: resource_id}) + if isinstance(e, ServerError): + print_error("SERVER_ERROR", f"Server error: {e}", {resource: resource_id}) raise typer.Exit(2) - else: - print_error("UNKNOWN_ERROR", str(e), {resource: resource_id}) + if isinstance(e, APIError): + # Catch-all for typed API errors we haven't special-cased + # (BadRequest, Conflict, UnprocessableEntity, RateLimit, ...). + print_error( + "API_ERROR", + f"API error ({e.status}): {e}", + {resource: resource_id, "status": e.status}, + ) raise typer.Exit(1) + print_error("UNKNOWN_ERROR", str(e), {resource: resource_id}) + raise typer.Exit(1)