CLI: polish three rough edges from the 1.3.7 prod shake-down

roboflow/adapters/rfapi.py

@@ -12,7 +12,18 @@
 
 
 class RoboflowError(Exception):
-    pass
+    """Generic API error.
+
+    Optional `status_code` is the HTTP status from the upstream response
+    when available — set by helpers like `_raise_for_trash_response` so
+    callers can branch on auth (401) vs not-found (404) without string
+    matching the message. Existing call sites that pass only a message
+    still work; the attribute defaults to `None`.
+    """
+
+    def __init__(self, message, status_code=None):
+        super().__init__(message)
+        self.status_code = status_code
 
 
 class ImageUploadError(RoboflowError):
@@ -911,8 +922,11 @@ def _raise_for_trash_response(response):
     error messages are agent-friendly. Falls back to the raw text if the
     body isn't JSON or doesn't contain an `error` field.
 
-    The single `raise` at the end means we can't accidentally swallow the
-    intended error if a future refactor widens the except clause.
+    Carries the HTTP status code on the raised exception so callers (e.g.
+    the CLI) can map auth failures (401) to the right exit code without
+    string-matching. The single `raise` at the end means we can't
+    accidentally swallow the intended error if a future refactor widens
+    the except clause.
     """
     msg = None
     try:
@@ -922,7 +936,7 @@ def _raise_for_trash_response(response):
     except ValueError:
         # Body wasn't JSON — fall through to response.text.
         pass
-    raise RoboflowError(msg or response.text)
+    raise RoboflowError(msg or response.text, status_code=response.status_code)
 
 
 def delete_project(api_key, workspace_url, project_url):

roboflow/cli/_output.py

@@ -176,6 +176,93 @@ def output_error(
     sys.exit(exit_code)
 
 
+def confirm_destructive(args: Any, prompt: str) -> bool:
+    """Gate a destructive action on either ``--yes`` or an interactive TTY confirmation.
+
+    Returns ``True`` if the action is approved (caller should proceed) or
+    ``False`` if the user declined at the prompt (caller should bail
+    cleanly via ``output(args, {"cancelled": True}, ...)``).
+
+    Calls ``output_error`` and exits with code 1 when *no* TTY is available
+    and ``--yes`` wasn't passed, rather than prompting on a closed stdin
+    (which would either hang or — worse — silently default to a permissive
+    behavior).
+
+    The previous logic gated on ``--json`` ("if --json is set, skip the
+    prompt") which conflated *output formatting* with *destructive intent*.
+    Anyone piping ``roboflow project delete X --json`` into ``jq`` for
+    parsing got their project nuked without any confirmation. Now ``--json``
+    is purely a formatting flag; the kill-switch is ``--yes``/``-y``.
+    """
+    if getattr(args, "yes", False):
+        return True
+
+    # Either explicit `--yes` is required or we need an interactive TTY to
+    # ask. typer.confirm() reads from stdin; if stdin is closed (CI, piped
+    # input, agent) we'd hang — bail with a useful hint instead.
+    if not sys.stdin.isatty():
+        output_error(
+            args,
+            "This is a destructive action and requires confirmation.",
+            hint=(
+                "Re-run with --yes / -y to confirm, or run interactively. "
+                "(--json is a formatting flag and does not bypass this.)"
+            ),
+            exit_code=1,
+        )
+        return False  # unreachable: output_error sys.exits
+
+    import typer
+
+    confirmed = typer.confirm(prompt, default=False)
+    if not confirmed:
+        # Caller renders the cancelled state.
+        output(args, {"cancelled": True}, text="Cancelled.")
+    return confirmed
+
+
+def output_api_error(
+    args: Any,
+    exc: Exception,
+    *,
+    hint: Optional[str] = None,
+    auth_hint: Optional[str] = None,
+    not_found_hint: Optional[str] = None,
+) -> None:
+    """Render a server-side error and exit with the correct code.
+
+    Maps an exception's HTTP status (carried as ``exc.status_code`` when the
+    raiser sets it — see ``_raise_for_trash_response`` in ``adapters.rfapi``)
+    to the per-CONTRIBUTING.md exit-code contract:
+
+    * **401** → exit code 2 ("auth error"). ``auth_hint`` overrides ``hint``
+      so we can surface a key-specific suggestion ("check ROBOFLOW_API_KEY"
+      etc.) regardless of what the caller passes.
+    * **404** → exit code 3 ("not found"). ``not_found_hint`` overrides
+      ``hint`` similarly. Useful for resources that may legitimately be
+      absent (deleted, mis-typed slug, etc.).
+    * **anything else / no status_code attached** → exit code 1 ("error"),
+      using ``hint`` verbatim.
+
+    Without this helper every handler had to either string-match the message
+    (brittle) or fall back to a single ``exit_code=3`` for both 401 and 404,
+    which broke the ``$? == 2`` contract that scripts rely on to decide
+    whether to retry vs. re-auth.
+    """
+    status = getattr(exc, "status_code", None)
+    if status == 401:
+        output_error(
+            args,
+            str(exc),
+            hint=auth_hint or hint or "Check that ROBOFLOW_API_KEY is set and not revoked.",
+            exit_code=2,
+        )
+    elif status == 404:
+        output_error(args, str(exc), hint=not_found_hint or hint, exit_code=3)
+    else:
+        output_error(args, str(exc), hint=hint, exit_code=1)
+
+
 def stub(args: Any) -> None:
     """Placeholder handler for not-yet-implemented commands."""
     output_error(args, "This command is not yet implemented.", hint="Coming soon.", exit_code=1)

roboflow/cli/handlers/project.py

@@ -223,7 +223,7 @@ def _create_project(args):  # noqa: ANN001
 
 def _delete_project(args):  # noqa: ANN001
     from roboflow.adapters import rfapi
-    from roboflow.cli._output import output, output_error
+    from roboflow.cli._output import confirm_destructive, output, output_api_error, output_error
     from roboflow.cli._resolver import resolve_resource
     from roboflow.config import load_roboflow_api_key
 
@@ -247,26 +247,52 @@ def _delete_project(args):  # noqa: ANN001
         )
         return
 
-    if not getattr(args, "yes", False) and not getattr(args, "json", False):
-        import typer
-
-        confirmed = typer.confirm(
-            f"Move '{workspace_url}/{project_slug}' to Trash? "
-            "(Retained for 30 days. Any in-flight trainings will be cancelled.)",
-            default=False,
-        )
-        if not confirmed:
-            output(args, {"cancelled": True}, text="Cancelled.")
-            return
+    if not confirm_destructive(
+        args,
+        f"Move '{workspace_url}/{project_slug}' to Trash? "
+        "(Retained for 30 days. Any in-flight trainings will be cancelled.)",
+    ):
+        return
 
     try:
         data = rfapi.delete_project(api_key, workspace_url, project_slug)
     except rfapi.RoboflowError as exc:
-        output_error(
+        # Idempotent re-delete: when the project is already in Trash the
+        # public API's URL filter excludes it, so the DELETE returns 404
+        # with a generic "endpoint does not exist" message. That looks like
+        # a permissions error to the user. Probe Trash explicitly — if the
+        # slug is there, treat the call as a no-op success so scripts can
+        # safely retry without special-casing the second attempt.
+        if getattr(exc, "status_code", None) == 404:
+            try:
+                trash = rfapi.list_trash(api_key, workspace_url)
+            except rfapi.RoboflowError:
+                trash = None
+            if trash is not None:
+                already = next(
+                    (p for p in trash.get("sections", {}).get("projects", []) if p.get("url") == project_slug),
+                    None,
+                )
+                if already is not None:
+                    data = {
+                        "deleted": True,
+                        "type": "project",
+                        "workspace": workspace_url,
+                        "project": project_slug,
+                        "projectId": already.get("id"),
+                        "trash": True,
+                        "alreadyInTrash": True,
+                    }
+                    output(
+                        args,
+                        data,
+                        text=f"{workspace_url}/{project_slug} is already in Trash (no-op).",
+                    )
+                    return
+        output_api_error(
             args,
-            str(exc),
+            exc,
             hint="Check your API key has 'project:update' scope on this workspace.",
-            exit_code=3,
         )
         return
 
@@ -279,7 +305,7 @@ def _delete_project(args):  # noqa: ANN001
 
 def _restore_project(args):  # noqa: ANN001
     from roboflow.adapters import rfapi
-    from roboflow.cli._output import output, output_error
+    from roboflow.cli._output import output, output_api_error, output_error
     from roboflow.cli._resolver import resolve_resource
     from roboflow.config import load_roboflow_api_key
 
@@ -306,11 +332,11 @@ def _restore_project(args):  # noqa: ANN001
     try:
         trash = rfapi.list_trash(api_key, workspace_url)
     except rfapi.RoboflowError as exc:
-        output_error(
+        output_api_error(
             args,
-            str(exc),
+            exc,
+            auth_hint="Check that ROBOFLOW_API_KEY is valid for this workspace.",
             hint="Check your API key has 'project:read' scope on this workspace.",
-            exit_code=3,
         )
         return
 
@@ -328,11 +354,10 @@ def _restore_project(args):  # noqa: ANN001
     try:
         data = rfapi.restore_trash_item(api_key, workspace_url, "project", match["id"])
     except rfapi.RoboflowError as exc:
-        output_error(
+        output_api_error(
             args,
-            str(exc),
+            exc,
             hint="Check your API key has 'project:update' scope on this workspace.",
-            exit_code=3,
         )
         return
 

roboflow/cli/handlers/trash.py

@@ -52,7 +52,7 @@ def _resolve_workspace(args):
 
 def _list_trash(args):  # noqa: ANN001
     from roboflow.adapters import rfapi
-    from roboflow.cli._output import output, output_error
+    from roboflow.cli._output import output, output_api_error
     from roboflow.cli._table import format_table
 
     workspace_url, api_key = _resolve_workspace(args)
@@ -62,11 +62,10 @@ def _list_trash(args):  # noqa: ANN001
     try:
         trash = rfapi.list_trash(api_key, workspace_url)
     except rfapi.RoboflowError as exc:
-        output_error(
+        output_api_error(
             args,
-            str(exc),
+            exc,
             hint="Check your API key has 'project:read' scope on this workspace.",
-            exit_code=3,
         )
         return
 

roboflow/cli/handlers/version.py

@@ -356,7 +356,7 @@ def _create(args):  # noqa: ANN001
 
 def _delete_version(args):  # noqa: ANN001
     from roboflow.adapters import rfapi
-    from roboflow.cli._output import output, output_error
+    from roboflow.cli._output import confirm_destructive, output, output_api_error, output_error
     from roboflow.cli._resolver import resolve_resource
     from roboflow.config import load_roboflow_api_key
 
@@ -388,26 +388,55 @@ def _delete_version(args):  # noqa: ANN001
         )
         return
 
-    if not getattr(args, "yes", False) and not getattr(args, "json", False):
-        import typer
-
-        confirmed = typer.confirm(
-            f"Move version '{workspace_url}/{project_slug}/{version_num}' to Trash? "
-            "(Retained for 30 days. Any in-flight training will be cancelled.)",
-            default=False,
-        )
-        if not confirmed:
-            output(args, {"cancelled": True}, text="Cancelled.")
-            return
+    if not confirm_destructive(
+        args,
+        f"Move version '{workspace_url}/{project_slug}/{version_num}' to Trash? "
+        "(Retained for 30 days. Any in-flight training will be cancelled.)",
+    ):
+        return
 
     try:
         data = rfapi.delete_version(api_key, workspace_url, project_slug, version_num)
     except rfapi.RoboflowError as exc:
-        output_error(
+        # Idempotent re-delete: if the version is already in Trash, the
+        # public API URL is filtered and DELETE returns 404 — surface it
+        # as an explicit no-op so retries don't surface a misleading
+        # "missing scope" message. Same shape as project delete above.
+        if getattr(exc, "status_code", None) == 404:
+            try:
+                trash = rfapi.list_trash(api_key, workspace_url)
+            except rfapi.RoboflowError:
+                trash = None
+            if trash is not None:
+                target = str(version_num)
+                already = next(
+                    (
+                        v
+                        for v in trash.get("sections", {}).get("versions", [])
+                        if str(v.get("id")) == target and v.get("parentUrl") == project_slug
+                    ),
+                    None,
+                )
+                if already is not None:
+                    data = {
+                        "deleted": True,
+                        "type": "version",
+                        "workspace": workspace_url,
+                        "project": project_slug,
+                        "version": str(version_num),
+                        "trash": True,
+                        "alreadyInTrash": True,
+                    }
+                    output(
+                        args,
+                        data,
+                        text=f"{workspace_url}/{project_slug}/{version_num} is already in Trash (no-op).",
+                    )
+                    return
+        output_api_error(
             args,
-            str(exc),
+            exc,
             hint="Check your API key has 'version:update' scope and the version exists.",
-            exit_code=3,
         )
         return
 
@@ -420,7 +449,7 @@ def _delete_version(args):  # noqa: ANN001
 
 def _restore_version(args):  # noqa: ANN001
     from roboflow.adapters import rfapi
-    from roboflow.cli._output import output, output_error
+    from roboflow.cli._output import output, output_api_error, output_error
     from roboflow.cli._resolver import resolve_resource
     from roboflow.config import load_roboflow_api_key
 
@@ -455,11 +484,10 @@ def _restore_version(args):  # noqa: ANN001
     try:
         trash = rfapi.list_trash(api_key, workspace_url)
     except rfapi.RoboflowError as exc:
-        output_error(
+        output_api_error(
             args,
-            str(exc),
+            exc,
             hint="Check your API key has 'project:read' scope on this workspace.",
-            exit_code=3,
         )
         return
 
@@ -488,11 +516,10 @@ def _restore_version(args):  # noqa: ANN001
             parent_id=match.get("parentId"),
         )
     except rfapi.RoboflowError as exc:
-        output_error(
+        output_api_error(
             args,
-            str(exc),
+            exc,
             hint="Check your API key has 'version:update' scope on this workspace.",
-            exit_code=3,
         )
         return