fix: CLI output shape, destructive-confirm previews, skip needless round-trips
This commit is contained in:
parent
5cff3bc458
commit
726699785e
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue