feat(support): add support report command - #290
Conversation
Files a ticket via POST {app_url}/v1/support/issues: compose with
-m/--subject or $EDITOR, attach the active workspace and diagnostic
context, optionally attach logs, and retry once on a connection error
or 5xx using a stable idempotency key.
The support intake is a normal /v1 API-gateway route, not a webapp/ OAuth one: build the URL from api_url via sdk_base_path the same way probe_runtime_status does, and drop the app_url/oauth_base path entirely. Workspace now travels only as X-Workspace-Id (never in the JSON body) so it matches every other authenticated call.
Match the repo's prevailing convention for "error: ..." lines (see client::ingest's IngestError::message) rather than the literal punctuation in the draft spec text.
Validate the subject after compose (both -m/--subject and $EDITOR paths): more than 200 chars errors out before any HTTP call, matching the server's own subject_too_long limit. Map subject_too_long, subject_required, and body_required in error_message instead of letting them fall through to the generic status/code message.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Review
Blocking Issues
src/commands/support.rs:136— a failed POST destroys the report composed in$EDITOR. The temp file fromutil::open_editoris deleted whenopen_editorreturns, andhandle_errorexits without printing the composed text. A connection error or arate_limitedresponse makes the user retype the whole report. This failure mode is the common one, because users runsupport reportwhen the service is already failing.
Action Required
Print the composed subject and body to stderr before exiting on any post_support_issue error. An alternative fix is persisting the compose file and printing the path.
Non-blocking
Four nits are inline: log redaction misses the curl paste case, mask_credential panics on a non-ASCII header value, the None error arm discards the server message, and default_workspace_id reaches the network inside build_request.
CI checks were still queued or in progress when this review started, so test results are not reflected here.
Byte-slicing a string at fixed offsets panics the moment a boundary lands mid multi-byte character. Slice by char instead; ASCII behavior (head+tail, head-only, or "***") is unchanged.
redact_log_line only masked an Authorization: header at the start of a line, missing a pasted `curl -H "Authorization: Bearer ..."` or a timestamp-prefixed access-log line. Scan for "bearer " case- insensitively at any position and mask the token that follows; fall back to the plain Authorization: header case (no Bearer scheme) only when no Bearer token was found, so nothing is masked twice.
error_message's generic fallback for a status with no stable error
code (an upstream 5xx, a framework-level rejection) printed only the
bare status, dropping whatever the body said (e.g. a FastAPI
{"detail": ...}). Fold in util::api_error's rendering, truncated so an
unbounded non-JSON body can't flood the terminal.
resolve_optional_workspace called client::credentials::default_workspace_id, which issues a live GET /workspaces for an env/flag-sourced api key. A support report must not block on the API being slow or down -- that's exactly when this command runs. Read only the saved default workspace from the profile instead; an exact workspace is optional for filing.
open_editor's temp file is gone by the time a POST fails, so a lost send used to lose the text the user just wrote with no way to recover it. compose() now reports whether the text came from $EDITOR; send_and_report persists it to support-draft-<unix-seconds>.md (mode 0600) under the config dir on any failed send when it did, and prints a re-file hint (or the whole report, if even saving fails). The -m/--subject path needs nothing extra -- that text is still in shell history.
post_support_issue_with_delay is now pub(crate) so persist_on_editor_failure_writes_a_draft_when_send_failed_and_editor_composed can exercise the real retry-once-on-5xx path with Duration::ZERO instead of eating the real 2s RETRY_DELAY on every test run.
mask_authorization_header_value masked everything from the value to the end of the line, so a log line like "... missing authorization: token expired for user 42" lost its trailing message along with the credential. Bound the masked run to the same rule mask_bearer_tokens uses (up to whitespace, a quote, or end of line) and leave the rest of the line untouched.
persist_composed_report wrapped the raw subject in single quotes; an apostrophe in it would leave the shell sitting at a continuation prompt. The hint now reads both --subject and -m back from the saved draft file at re-file time (head -n 1 / tail -n +3, both paths double-quoted) instead of interpolating user-controlled text into the printed command line.
hd_live_x123456789 reads like a plausible live-credential shape; swap it for an obviously-fake fixture value.
There was a problem hiding this comment.
All prior blocking and nit threads are addressed in the new commits.
Checked the cycle-2 fixes:
mask_authorization_header_valuebounds the value to the token and keeps the rest of the line.refile_hintreads subject and body from the draft file, so user text never lands on the printed command line.- Byte-offset slicing in both mask helpers stays on char boundaries, because
to_ascii_lowercasepreserves length. - The renamed fixture token clears the scanner finding.
No new blocking issues.
CI note: CI / test, CI / fmt, and the integration checks were not reported as passing when this review started. This approval covers the code, not a test result.
Validate --logs and --context before opening $EDITOR, and save a draft when the composed subject is the thing that's too long. Both paths previously discarded text the user had just written, since compose()'s temp file is deleted the moment it returns.
The table confirmation never mentioned --logs, so a dropped or mis-read log file was invisible until a support reply asked for it.
Two runs failing inside the same second wrote the same support-draft-<secs>.md, so the second silently replaced the first.
The send can sit for two client timeouts plus the 2s retry backoff with nothing on screen, on exactly the failing-API case this command is for.
There was a problem hiding this comment.
The full diff was omitted from the review prompt. Reviewed the since-diff plus src/commands/support.rs, src/client/support.rs, src/util.rs and src/main.rs read from the checkout.
All prior threads are addressed in code. No blocking issues in this cycle. One nit left inline. CI checks other than changelog and scenario-parity had not reported when this review started.
A report typed on one line was rejected for the missing body and discarded with it, since the editor's temp file is already gone. An untouched template still aborts silently.
There was a problem hiding this comment.
Cycle 5 review: blocking issues only.
The full diff was omitted from the review prompt. gh pr diff output exceeded the read limit, so the review used the since-diff plus a read of src/commands/support.rs, src/client/support.rs, src/util.rs, src/main.rs, src/cli.rs, src/commands.rs, and src/client.rs from the checkout.
All prior blocking and nit threads are resolved in code. abort_or_rescue saves a draft only when split_composed finds a content line, so an untouched template still cancels silently. No new blocking issue found.
CI status note: only CI / changelog reported success at review time. The other checks were queued or in progress, so their results are unknown.
Adds
hotdata support report, which files a support ticket viaPOST /v1/support/issueson the API host with the workspace inX-Workspace-Id, diagnostic context, optional logs, and an idempotency key with one retry. Depends on the server endpoint in hotdata-dev/monopoly (feat/support-intake-api), which is flag-gated until credentials are configured.