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

[yeldarby]

Why

End-to-end testing of the v1.3.7 CLI against prod (roboflow#11131 / roboflow-python#463 shipped this morning) surfaced three rough edges. None are correctness bugs — every command does the right server-side thing — but they all lead users (and especially agents) into wrong conclusions and were called out in the prod shake-down report. Worth fixing before the next release.

The three findings

1. Auth errors return exit 3 (should be 2)

Per CONTRIBUTING.md: 0=success, 1=error, 2=auth, 3=not-found. Every rfapi.RoboflowError from the trash endpoints was caught with exit_code=3, so scripts can't differentiate re-auth from wrong slug:

$ roboflow -k bogus-key trash list --json ; echo $?
{"error": ...}
3                ← should be 2

2. --json silently bypasses destructive-action confirmation

$ roboflow project delete moondream-example --json < /dev/null
{"deleted": true, "trash": true, ...}    ← deleted, no prompt!

Old logic was if not yes and not json: prompt. That conflated output formatting with destructive intent — anyone piping … --json | jq got their project nuked because --json was treated as "I'm in CI, skip the prompt."

3. Idempotent re-delete bubbles up a misleading "missing scope" 404

When a project/version/workflow is already in Trash, the public API's URL filter excludes it from the active set, so the next DELETE returns 404 ("Unsupported request. DELETE /ws/proj does not exist..."). The CLI surfaced that as exit 3 with Check your API key has 'project:update' scope — factually wrong (the user has the scope; the item is just where they wanted it).

What

All three fixes live centrally so future trash / destructive endpoints inherit the right behavior for free.

Auth exit code (#1)

• RoboflowError(message, status_code=None) now stores status_code. Defaulted to None for back-compat with the 30+ existing call sites that pass message-only.
• _raise_for_trash_response stamps response.status_code on the raised exception.
• New output_api_error(args, exc, hint=..., auth_hint=..., not_found_hint=...) helper in cli/_output.py does the central status→exit-code mapping (401→2, 404→3, else→1). All four trash handlers (project / version / workflow / trash) replace hardcoded output_error(..., exit_code=3) with it.

Destructive guard (#2)

• New confirm_destructive(args, prompt) in cli/_output.py:
  • --yes → proceed.
  • TTY without --yes → prompt (regardless of --json).
  • No TTY without --yes → bail with exit 1 + hint about --yes.
• project delete, version delete, workflow delete all switched to it.

Idempotent re-delete (#3)

• Each delete handler intercepts a 404 from rfapi.delete_X, probes list_trash, and if the slug/url is found in the matching sections array, emits a synthetic success payload mirroring a real delete plus alreadyInTrash: true. If the slug is genuinely missing from trash too, the 404 propagates as exit 3 (typos still surface as errors).

Sample payload from a re-delete:

{
  "deleted": true,
  "type": "project",
  "workspace": "q3-board-meeting",
  "project": "moondream-example",
  "projectId": "Jjk3Tjq2yjxenyA9g5er",
  "trash": true,
  "alreadyInTrash": true
}

Tests

tests/cli/test_trash_polish.py adds 14 tests covering all three:

• output_api_error mapping for 401 / 404 / 500 / no-status-code (defaults to 1, NOT 3 or 2 — important so legacy RoboflowError(text) callers don't accidentally claim "not found" / "auth").
• _raise_for_trash_response actually attaches status_code.
• confirm_destructive honors --yes, refuses no-TTY without --yes, ignores --json when deciding (regression guard for finding final changes to pip package #2), and prompts via typer on a TTY (both accept and decline paths).
• Idempotent re-delete success path on project / version / workflow, plus the negative case where a 404 with empty trash propagates as exit 3 (so we don't mask real "not found" errors).

$ python -m unittest discover -s tests
Ran 526 tests in 7.222s
OK (skipped=1)

$ ruff check roboflow tests
All checks passed!

$ ruff format --check roboflow tests
113 files already formatted

$ mypy roboflow
Success: no issues found in 62 source files

Backwards compatibility

• RoboflowError(message) (no status_code) still works — every call site outside the trash endpoints does this and is unaffected.
• The new helpers (output_api_error, confirm_destructive) are additive in _output.py. Nothing else imports them yet.
• Wire format of successful deletes is unchanged; the new alreadyInTrash field is additive for the previously-error-out 404 case, so any caller currently parsing a successful delete payload keeps working.

Test plan

• pytest / python -m unittest — all 526 tests pass.
• ruff check roboflow tests — clean.
• ruff format --check roboflow tests — clean.
• mypy roboflow — clean.
• One reviewer sanity-check of the synthetic alreadyInTrash payload shape (mirrors successful delete fields verbatim — type, workspace, project/workflow, projectId/workflowId, trash).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68ac209753

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve subclass HTTP status when initializing RoboflowError

RoboflowError.__init__ now unconditionally assigns self.status_code = status_code, which defaults to None; this overwrites status codes set by subclasses that call super().__init__(message) without forwarding the second argument (notably ImageUploadError and AnnotationSaveError in rfapi.py). As a result, paths that raise ImageUploadError(..., status_code=...) or AnnotationSaveError(..., status_code=...) silently lose the HTTP code, regressing existing behavior and removing useful error context for retry/auth handling.

Useful? React with 👍 / 👎.

[digaobarbosa]

Quick suggestions to address the codex review detail.

[digaobarbosa]

Suggested change

	self.message = message
	self.retries = 0
	super().__init__(self.message, status_code=status_code)

[digaobarbosa]

Suggested change

	self.message = message
	self.retries = 0
	super().__init__(self.message, status_code=status_code)