Skip to content

feat(support): add support report command - #290

Merged
zfarrell merged 19 commits into
mainfrom
feat/support-report
Sep 6, 2026
Merged

feat(support): add support report command#290
zfarrell merged 19 commits into
mainfrom
feat/support-report

Conversation

@zfarrell

@zfarrell zfarrell commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Adds hotdata support report, which files a support ticket via POST /v1/support/issues on the API host with the workspace in X-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.

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.
@zfarrell
zfarrell requested a review from a team as a code owner September 5, 2026 17:36
@zfarrell
zfarrell requested review from anoop-narang and removed request for a team September 5, 2026 17:36
Comment thread src/commands/support.rs
Comment thread src/commands/support.rs Outdated
Comment thread src/commands/support.rs Outdated
Comment thread src/commands/support.rs Outdated
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.74691% with 120 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/commands/support.rs 93.72% 75 Missing ⚠️
src/util.rs 32.43% 25 Missing ⚠️
src/main.rs 0.00% 20 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread src/commands/support.rs Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  • src/commands/support.rs:136 — a failed POST destroys the report composed in $EDITOR. The temp file from util::open_editor is deleted when open_editor returns, and handle_error exits without printing the composed text. A connection error or a rate_limited response makes the user retype the whole report. This failure mode is the common one, because users run support report when 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.
Comment thread src/commands/support.rs Outdated
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.
Comment thread src/commands/support.rs Outdated
Comment thread src/commands/support.rs Outdated
Comment thread src/commands/support.rs Outdated
claude[bot]
claude Bot previously approved these changes Sep 5, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All five prior threads are resolved in code. Remaining findings are non-blocking.

Note: CI checks for this push were still queued or in progress when this review ran. This review did not run tests.

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.
Comment thread src/commands/support.rs
claude[bot]
claude Bot previously approved these changes Sep 5, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All prior blocking and nit threads are addressed in the new commits.

Checked the cycle-2 fixes:

  • mask_authorization_header_value bounds the value to the token and keeps the rest of the line.
  • refile_hint reads 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_lowercase preserves 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.
Comment thread src/commands/support.rs Outdated
claude[bot]
claude Bot previously approved these changes Sep 6, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@zfarrell
zfarrell merged commit f21f1db into main Sep 6, 2026
13 checks passed
@zfarrell
zfarrell deleted the feat/support-report branch September 6, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant