From 3a164cc95406799236b0cd1934a7c50e92d3dd49 Mon Sep 17 00:00:00 2001 From: Arnold Cubici-Jones <108676317+AJCJ1@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:34:42 +0100 Subject: [PATCH] feat(cli): report a bad render, and open support from the terminal Adds `urlbox report ` to file a render report with the Urlbox team (category + comment), a post-render hint that prints the ready-to-run report command, and `urlbox support` to open the contact page. - Every render response prints a muted hint line naming the render id and the report command, text-mode + TTY only (json/quiet/piped unchanged). - `urlbox report` is session-authed (needs `urlbox login`): --category and --comment for scripts and agents, an interactive picker + comment prompt otherwise. Errors map to the closed exit-code set (auth 3, not-found 5, conflict 7). - The api client captures the render id from the x-urlbox-request-id response header and error-body requestId, in-memory only so envelopes are byte-identical. - `urlbox support` mirrors `urlbox dashboard`: opens the contact page, prints the URL when headless, never launches a browser in json/quiet. - Also scrubs review-round shorthand and private-repo references out of existing comments. --- CHANGELOG.md | 12 + README.md | 14 + SURFACE.txt | 14 + ...ANDOFF-2026-09-11-render-report-command.md | 49 + .../plans/2026-09-11-render-report-command.md | 1578 +++++++++++++++++ ...2026-09-11-render-report-command-design.md | 204 +++ internal/api/download_client.go | 2 +- internal/api/download_client_test.go | 2 +- internal/api/http_client.go | 50 +- internal/api/http_client_test.go | 9 +- internal/api/render_id_test.go | 94 + internal/api/smoke_test.go | 10 +- internal/api/types.go | 5 + internal/api/types_test.go | 2 +- internal/browser/opener.go | 2 +- internal/browser/opener_test.go | 4 +- internal/cmd/auth_preflight.go | 2 - internal/cmd/commands.go | 4 +- internal/cmd/config.go | 32 +- internal/cmd/config_test.go | 36 +- internal/cmd/dashboard.go | 4 +- internal/cmd/dashboard_test.go | 8 +- internal/cmd/doctor.go | 10 +- internal/cmd/doctor_internal_test.go | 4 +- internal/cmd/doctor_test.go | 10 +- internal/cmd/e2e_test.go | 2 +- internal/cmd/error_hints_test.go | 1 - internal/cmd/help_agent_test.go | 2 +- internal/cmd/link.go | 2 +- internal/cmd/link_test.go | 2 +- internal/cmd/overlay.go | 6 +- internal/cmd/overlay_test.go | 2 +- internal/cmd/render.go | 37 +- internal/cmd/render_output.go | 7 +- internal/cmd/render_output_test.go | 2 +- internal/cmd/render_test.go | 48 +- internal/cmd/report.go | 216 +++ internal/cmd/report_hint.go | 63 + internal/cmd/report_hint_test.go | 157 ++ internal/cmd/report_test.go | 255 +++ internal/cmd/root.go | 18 +- internal/cmd/root_test.go | 12 +- internal/cmd/secret_input.go | 2 +- internal/cmd/secret_input_test.go | 4 +- internal/cmd/skill.go | 2 +- internal/cmd/skill_test.go | 6 +- internal/cmd/status.go | 10 +- internal/cmd/status_test.go | 4 +- internal/cmd/support.go | 88 + internal/cmd/support_test.go | 89 + internal/cmd/surface.go | 4 +- internal/cmd/upgrade_test.go | 2 +- internal/cmd/version.go | 2 +- internal/cmd/version_test.go | 13 +- internal/config/apihost_validate.go | 4 +- internal/config/apihost_validate_test.go | 4 +- internal/config/config.go | 4 +- internal/config/config_test.go | 4 +- internal/config/repo.go | 2 +- internal/config/resolve.go | 8 +- internal/config/resolve_test.go | 8 +- internal/config/safe_write.go | 2 - internal/config/safe_write_test.go | 2 - internal/config/secret_validate.go | 15 +- internal/config/secret_validate_test.go | 12 +- internal/config/update.go | 10 +- internal/config/update_test.go | 8 +- internal/output/envelope.go | 2 +- internal/output/envelope_test.go | 2 +- internal/output/jq.go | 4 +- internal/prompt/prompt.go | 19 + internal/prompt/textinput_test.go | 13 + internal/surface/snapshot.go | 2 +- internal/surface/snapshot_test.go | 2 +- internal/validation/validate.go | 2 +- internal/validation/validate_test.go | 5 +- schema/schema.go | 4 +- skills/SKILL.md | 33 + 78 files changed, 3153 insertions(+), 237 deletions(-) create mode 100644 docs/superpowers/HANDOFF-2026-09-11-render-report-command.md create mode 100644 docs/superpowers/plans/2026-09-11-render-report-command.md create mode 100644 docs/superpowers/specs/2026-09-11-render-report-command-design.md create mode 100644 internal/api/render_id_test.go create mode 100644 internal/cmd/report.go create mode 100644 internal/cmd/report_hint.go create mode 100644 internal/cmd/report_hint_test.go create mode 100644 internal/cmd/report_test.go create mode 100644 internal/cmd/support.go create mode 100644 internal/cmd/support_test.go create mode 100644 internal/prompt/textinput_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index bd2729e..4fe9a0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to the `urlbox` CLI are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [SemVer](https://semver.org/spec/v2.0.0.html). +## Unreleased + +**Report a bad render from the CLI.** Every render response now prints a +muted hint line with the render's id and the ready-to-run report command +(interactive terminals only — json/quiet/piped output is byte-identical). +`urlbox report ` files the report: `--category` + `--comment` +for agents and scripts, an interactive category picker + comment prompt +for humans. Reports need a signed-in session (`urlbox login`); render +secrets cannot file them. `urlbox support` opens the support contact +page, with the dashboard command's headless and machine-readable +behaviour. + ## v1.2.0 — 2026-08-19 **`urlbox login` is the only interactive sign-in; `urlbox auth` is gone.** diff --git a/README.md b/README.md index da371f1..39d577d 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,18 @@ urlbox status ps_abc123 --wait Pass `--async` to queue a render and get a `renderId` back immediately. `status` checks it, and `status --wait` polls (every 2s by default) until it reaches a terminal state — `succeeded` or `failed`. Webhooks and long-running renders are covered at [urlbox.com/docs/cli/async-and-webhooks](https://urlbox.com/docs/cli/async-and-webhooks). +### Reporting a bad render + +```sh +urlbox report 01a0906a-…_ps # interactive: category picker + comment +urlbox report 01a0906a-…_ps --category bot-detection --comment "page shows a captcha" +``` + +Every render prints its id in a hint line under the result. `report` files +the problem straight to the Urlbox team (requires `urlbox login`); the team +follows up on your report from the dashboard. `urlbox support` opens the +contact page for anything else. + ### Account and context ```sh @@ -128,6 +140,8 @@ urlbox usage # render usage for the current period | Command | Does | |---------|------| | `login` / `logout` | Sign in through the browser; sign out and revoke this device's session | +| `report ` | File a render problem report with the Urlbox team (needs `login`) | +| `support` | Open the support contact page in your browser | | `whoami` (alias `me`) | Show the signed-in user and active org/project | | `orgs list` / `orgs select` | List or switch your active organisation (`--project` finishes the switch in one step) | | `projects list` / `select` / `show` | Browse and switch the active project | diff --git a/SURFACE.txt b/SURFACE.txt index 8f719c3..23ca4fd 100644 --- a/SURFACE.txt +++ b/SURFACE.txt @@ -480,6 +480,15 @@ urlbox render [url] --user-agent urlbox render [url] --wait-until urlbox render [url] --webhook-url urlbox render [url] --width +urlbox report +urlbox report --agent +urlbox report --category +urlbox report --comment +urlbox report --jq +urlbox report --max-retries +urlbox report --no-retry +urlbox report --output-format +urlbox report --profile urlbox schema urlbox schema --agent urlbox schema --jq @@ -627,6 +636,11 @@ urlbox storage update --provider urlbox storage update --region urlbox storage update --sas-token urlbox storage update --secret +urlbox support +urlbox support --agent +urlbox support --jq +urlbox support --output-format +urlbox support --profile urlbox upgrade urlbox upgrade --agent urlbox upgrade --jq diff --git a/docs/superpowers/HANDOFF-2026-09-11-render-report-command.md b/docs/superpowers/HANDOFF-2026-09-11-render-report-command.md new file mode 100644 index 0000000..49b5ceb --- /dev/null +++ b/docs/superpowers/HANDOFF-2026-09-11-render-report-command.md @@ -0,0 +1,49 @@ +# HANDOFF: CLI render reporting (fresh session, new feature) + +**Repo:** urlbox-cli (this repo). New branch off latest `origin/main` — main has moved past PR #1; fetch first. +**Process:** brainstorm (Arnold, one question at a time) → spec → plan → subagent execution with per-task reviews → Arnold's manual gate. Nothing implements before the spec is approved. + +## The feature + +Reporting a bad render, from the CLI, via the mono's public render-report endpoints. Three pieces: + +1. **Post-render affordance (interactive TTY only).** After a render result, a non-interruptive stderr affordance offers to report it — the interaction pattern is Claude Code's post-output feedback strip (ignorable, doesn't break the flow of work; a keypress starts the reporting flow). It must be: ignorable, never blocking, and completely absent in json/quiet/non-TTY. NOTE: the CLI currently stores nothing about past renders — remembering "the render that just happened" is a design question to solve fresh in the brainstorm. +2. **`urlbox report ` command.** Agent path: flags, JSON output, no prompts, house error codes. An agent that notices a bad render reports it cold. +3. **`urlbox support` command.** Opens the browser on the Urlbox support/contact page (house `browser` opener; print URL on headless). Small supplement, same feature branch. + +**Arnold's rulings already made:** +- LIMITED reporting for now (his stated preference): category + comment. Area/region-specific reporting (the API's `rects`) is deliberately out — it's the dashboard's click-to-highlight concept and too tricky for a first CLI pass. +- Do NOT mine the archived `urlbox/cli` repo's old report-command branch. Ruled out — causes confusion. Design fresh. +- CLI-self-problem reporting (bugs in the CLI itself) is a SEPARATE future feature; not this branch beyond `urlbox support`. +- The 1–4 rating in Claude Code was an interaction-pattern EXAMPLE, not a requested rating feature. Do not build a rating. + +## Pinned API contracts (from mono origin/main, 2026-09-11 — RE-VERIFY at build time; the mono is actively changing, Arnold has an in-flight render-reports worktree) + +Base: `/v2`, org-scoped. Routes tagged public — callers: dashboard session, bearer session, or org API key (per the v2 controller header). + +- `POST /v2/organisation/{org}/render-reports` — body: `renderId` (string 1–255; missing/expired renderIds are accepted by design), `category` (enum below), `comment` (string 1–2000, required), `rects` (optional, CLI sends none). Returns the report DTO. +- `GET /v2/organisation/{org}/render-reports` — filters: `renderId?`, `state?` (`open|in_review|resolved`), `category?`, `limit` (1–100, default 25). +- `POST /v2/organisation/{org}/render-reports/{report}/respond` — `report` id + `message` (1–2000). Probably out of scope for the CLI's first pass; note it exists. + +Categories: `bot-detection`, `login-required`, `missing-content`, `cookie-banner-or-popup`, `render-failed`, `other`. + +**MUST-VERIFY before design freezes:** which CLI credentials the routes actually accept — the login session token clearly works (bearer session); confirm whether a PROJECT render secret (`URLBOX_API_SECRET`, all an env-only agent has) is honoured as "org API key", or whether workflow 2 requires a session. This decides the command's auth guard and the docs story. Verify live against production, not from code reading alone. + +## Open design questions for the brainstorm (Arnold rules on each) + +1. The affordance: exact keypress, copy, and when it appears (every render? backoff? dismiss-forever?). +2. What the limited report captures interactively: category picker (house `prompt.SelectOne`) + one comment line? Or category only? +3. Last-render memory: where it lives (config? separate state file?), what it stores, retention. Keep minimal. +4. `urlbox report` flags: `--category`/`--comment` naming, `--last` sugar?, JSON output shape. +5. `urlbox support` target URL (urlbox.com/contact exists; confirm the canonical support page). + +## Conventions (all binding, learned/ruled in the previous sessions) + +- Zero code comments (linter-forced exported doc comments only). TDD failing-first. `make ci` + `make surface-snapshot` per task; SURFACE.txt committed with the code. +- Production-shape fixtures ONLY — pin every wire shape from the mono contracts / live API; invented fixtures repeatedly caused real bugs. +- Visual rules: lists = shared table renderer, details = KV box, view replaces summary on ok; named mutation summaries. Closed error-code set, non-empty hints, unified login hint. stdout = data, stderr = human. +- Agent rules: every interactive affordance has a flag equivalent; non-TTY never hangs; `--output-format json` everywhere; `--yes` bypasses prompts. +- No profiles in docs/help. Positioning: "website screenshot API", never "web automation"; customer word is "proxies". No em-dashes in customer-facing prose. +- Commits: per-task during execution, squash to one before PR; author `Arnold Cubici-Jones <108676317+AJCJ1@users.noreply.github.com>`; subjects <72 chars; NEVER push without Arnold's word. +- Propose-before-implement with Arnold ALWAYS: questions get answers, not edits; copy is his; decisions surface for veto, including knock-on wording. Validate findings against the real npm binary / live API before claiming anything (renders are free on his staff account). +- Docs for this feature (fumadocs CLI section + command reference) ride the same effort; page-by-page with Arnold. diff --git a/docs/superpowers/plans/2026-09-11-render-report-command.md b/docs/superpowers/plans/2026-09-11-render-report-command.md new file mode 100644 index 0000000..b50b330 --- /dev/null +++ b/docs/superpowers/plans/2026-09-11-render-report-command.md @@ -0,0 +1,1578 @@ +# CLI Render Reporting Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Post-render report hint line, `urlbox report ` (session-authed create against `/v2/organisation/{org}/render-reports`), and `urlbox support` (browser opener for https://urlbox.com/contact). + +**Architecture:** The api package captures the render id that sync responses only carry in the `x-urlbox-request-id` header (and error bodies carry as `requestId`), exposing it in-memory only — envelopes never change shape. The render command prints a gated muted stderr hint line naming `urlbox report `. `report` is a session command modelled on `proxies create`; `support` is a clone of `dashboard`. + +**Tech Stack:** Go, cobra, charmbracelet/huh (prompts), lipgloss (styles), httptest via `internal/api/apitest`. + +**Spec:** `docs/superpowers/specs/2026-09-11-render-report-command-design.md` — binding. Re-verify mono contracts at build time (Arnold has an in-flight render-reports worktree). + +## Global Constraints + +- Zero code comments except linter-forced exported doc comments. +- TDD failing-first; `make ci` (fmt-check, lint, test, unit, build, surface-check) green per task; `make surface-snapshot` + commit SURFACE.txt with any surface change. +- stdout = data, stderr = human. Every interactive affordance has a flag equivalent; non-TTY never hangs. +- Production-shape fixtures ONLY — wire shapes below were captured live 2026-09-11; do not invent fields. +- All user-facing copy in this plan (hint line, prompt titles, category labels, summaries, hints, help text) is **proposed** — Arnold approves or rewrites at task review before merge. No em-dashes in customer-facing prose; positioning is "website screenshot API". +- Commits per task (squash to one before PR), author `Arnold Cubici-Jones <108676317+AJCJ1@users.noreply.github.com>`, subjects <72 chars, imperative. NEVER push without Arnold's word. NEVER open a PR without Arnold's explicit ask. +- The engine test-watcher note does not apply here; `make test` runs plain `go test`. + +## Pinned wire shapes (captured live 2026-09-11 — fixture sources) + +```text +Sync success 200: body {"renderUrl":"https://renders.urlbox.com/…png","size":17752,…} (NO renderId) + header x-urlbox-request-id: 01a0906a-fff5-7565-8dc5-02868f11a4fa_ps +Sync failure 400: body {"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"} + (no x-urlbox-request-id header) +Async 201: body {"status":"created","renderId":"01a09072-9ec6-76ca-bcc7-bbf5cc35a9e4_pa","statusUrl":"https://api.urlbox.com/v1/render/…"} +Report create DTO (from mono origin/main zod contract packages/domain/src/render-report/render-report.schemas.ts): + {"id":"rpt_x1","renderId":"01a0…_ps","category":"bot-detection","comment":"…","rects":[], + "state":"open","outputUrl":null,"outputFormat":null,"resolutionMessage":null, + "resolutionExampleOptions":null,"resolutionExampleUrl":null,"hasUnseenResolution":false, + "createdAt":"2026-09-11T12:00:00.000Z"} +Report 404: oRPC error {"defined":false,"code":"NOT_FOUND","status":404,"message":"Render not found"} +Report 409: {"defined":false,"code":"CONFLICT","status":409,"message":"This report is being looked at by the team and can no longer be edited."} +Categories: bot-detection, login-required, missing-content, cookie-banner-or-popup, render-failed, other +``` + +Note the oRPC error shape has `message` at top level — `extractAPIError` (`internal/api/http_client.go:299`) already handles the flat `{"message": …}` shape. + +--- + +### Task 0: Branch setup + +**Files:** none (git only) + +Preconditions Arnold resolves before execution starts (surface, don't decide): +the main checkout `~/Code/work/urlbox-cli` sits on `feat/account-management` +with a modified `README.md` and stray untracked files (`.idea/`, `check.png`, +`hello.png`). Switching to a branch off `origin/main` will fail on the README. + +- [ ] **Step 1: Fetch and branch** + +```bash +cd ~/Code/work/urlbox-cli +git fetch origin +git switch -c feat/render-report origin/main +``` + +Expected: new branch at `c9d96d6` (or newer origin/main tip). + +- [ ] **Step 2: Commit the docs** + +```bash +git add docs/superpowers/HANDOFF-2026-09-11-render-report-command.md \ + docs/superpowers/specs/2026-09-11-render-report-command-design.md \ + docs/superpowers/plans/2026-09-11-render-report-command.md +git commit -m "docs: render-report spec and plan" +``` + +- [ ] **Step 3: Baseline green** + +Run: `make ci` +Expected: PASS on the untouched tree. + +--- + +### Task 1: Capture the render id in the api package + +**Files:** +- Modify: `internal/api/types.go` (Response struct, line 34) +- Modify: `internal/api/http_client.go` (`do` at line 81, add `extractRequestID` near `extractAPIError` at line 299) +- Create: `internal/api/render_id_test.go` + +**Interfaces:** +- Produces: `api.Response.RenderID string` — set from the `x-urlbox-request-id` response header on any 2xx render-family response; zero value when the header is absent. +- Produces: `api.RenderIDError` — `type RenderIDError struct { Err *output.CLIError; RenderID string }` with `Error() string` and `Unwrap() error`; returned by `HTTPClient.do` instead of the bare `*output.CLIError` when a ≥400 response body carries a non-empty `requestId`. `errors.As(err, &cliErr)` still resolves the inner `*output.CLIError` via Unwrap, so `Execute` (root.go:53) and the render command's hint refinement (render.go:405) keep working unchanged. + +- [ ] **Step 1: Write the failing tests** + +Create `internal/api/render_id_test.go` (package `api_test`, mirroring `http_client_test.go`): + +```go +package api_test + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/urlbox/urlbox-cli/internal/api" + "github.com/urlbox/urlbox-cli/internal/api/apitest" + "github.com/urlbox/urlbox-cli/internal/output" +) + +func TestRender_SyncSuccess_CapturesRenderIDHeader(t *testing.T) { + srv := apitest.New(apitest.ScriptedResponse{ + Status: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Urlbox-Request-Id": []string{"01a0906a-fff5-7565-8dc5-02868f11a4fa_ps"}, + }, + Body: `{"renderUrl":"https://renders.urlbox.com/x.png","size":17752}`, + }) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + resp, err := c.Render(context.Background(), map[string]any{"url": "https://example.com"}) + if err != nil { + t.Fatalf("Render: %v", err) + } + if resp.RenderID != "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps" { + t.Fatalf("RenderID=%q", resp.RenderID) + } + if _, ok := resp.Data["renderId"]; ok { + t.Fatalf("renderId must NOT be injected into Data: %v", resp.Data) + } +} + +func TestRender_SyncSuccess_NoHeader_RenderIDEmpty(t *testing.T) { + srv := apitest.New(apitest.SuccessJSON(`{"renderUrl":"https://renders.urlbox.com/x.png","size":1}`)) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + resp, err := c.Render(context.Background(), map[string]any{"url": "https://example.com"}) + if err != nil { + t.Fatalf("Render: %v", err) + } + if resp.RenderID != "" { + t.Fatalf("RenderID=%q, want empty", resp.RenderID) + } +} + +func TestRender_Failure_WrapsRenderIDError(t *testing.T) { + srv := apitest.New(apitest.ScriptedResponse{ + Status: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"}`, + }) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + c.Retry = api.NoRetryConfig() + _, err := c.Render(context.Background(), map[string]any{"url": "https://x.invalid"}) + var ridErr *api.RenderIDError + if !errors.As(err, &ridErr) { + t.Fatalf("want RenderIDError, got %T: %v", err, err) + } + if ridErr.RenderID != "01a09082-25cf-74bd-9188-022c4f5bf25f_ps" { + t.Fatalf("RenderID=%q", ridErr.RenderID) + } + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("errors.As must still reach the CLIError through Unwrap") + } + if cliErr.Code != output.ErrValidation { + t.Fatalf("code=%q, want validation (InvalidURLError)", cliErr.Code) + } +} + +func TestRender_Failure_NoRequestID_PlainCLIError(t *testing.T) { + srv := apitest.New(apitest.ScriptedResponse{ + Status: http.StatusUnauthorized, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid token","code":"ApiKeyInvalid"}}`, + }) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + c.Retry = api.NoRetryConfig() + _, err := c.Render(context.Background(), map[string]any{"url": "https://example.com"}) + var ridErr *api.RenderIDError + if errors.As(err, &ridErr) { + t.Fatalf("no requestId in body must yield a plain CLIError, got RenderIDError") + } + var cliErr *output.CLIError + if !errors.As(err, &cliErr) || cliErr.Code != output.ErrAuth { + t.Fatalf("want auth CLIError, got %v", err) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/api/ -run 'RenderID|CapturesRenderIDHeader' -v` +Expected: FAIL — `resp.RenderID undefined` / `api.RenderIDError` undefined (compile errors count as the failing state). + +- [ ] **Step 3: Implement** + +`internal/api/types.go` — add the field to `Response` (after `Hint`, line 42): + +```go + // RenderID is the render id the API surfaced OUTSIDE the body — the + // x-urlbox-request-id response header. Sync render bodies carry no id, + // so this is the only place a sync render's id exists. In-memory only: + // json:"-" keeps envelopes unchanged. + RenderID string `json:"-"` +``` + +`internal/api/http_client.go` — in `do`, replace the ≥400 branch (line 133-135): + +```go + if resp.StatusCode >= 400 { + cliErr := mapStatusToCLIError(resp, respBody) + if rid := extractRequestID(respBody); rid != "" { + return nil, &RenderIDError{Err: cliErr, RenderID: rid} + } + return nil, cliErr + } +``` + +and set the field when building the success Response (line 171): + +```go + return &Response{OK: true, Data: data, RenderID: resp.Header.Get("x-urlbox-request-id")}, nil +``` + +Add next to `extractAPIError` (line 299): + +```go +// extractRequestID reads the requestId field Urlbox error bodies carry +// alongside the error object. Returns "" for non-JSON or absent field. +func extractRequestID(body []byte) string { + var parsed map[string]any + if err := json.Unmarshal(body, &parsed); err != nil { + return "" + } + rid, _ := parsed["requestId"].(string) + return rid +} + +// RenderIDError decorates a CLIError from a failed render call with the +// render id the error body carried, so the render command can offer +// `urlbox report ` for the failed render. Unwrap keeps errors.As +// resolution to *output.CLIError intact everywhere else. +type RenderIDError struct { + Err *output.CLIError + RenderID string +} + +func (e *RenderIDError) Error() string { return e.Err.Error() } + +func (e *RenderIDError) Unwrap() error { return e.Err } +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/api/ -v` +Expected: PASS (all api tests, not just the new ones — the changed error return must not break existing render/status error tests). + +- [ ] **Step 5: `make ci`, commit** + +```bash +make ci +git add internal/api/types.go internal/api/http_client.go internal/api/render_id_test.go +git commit -m "feat(api): capture render id from header and error bodies" +``` + +--- + +### Task 2: Post-render hint line + +**Files:** +- Create: `internal/cmd/report_hint.go` +- Create: `internal/cmd/report_hint_test.go` +- Modify: `internal/cmd/render.go` (error branch line 402-412, envelope write line 445-453) + +**Interfaces:** +- Consumes: `api.Response.RenderID`, `api.RenderIDError` (Task 1); `isStderrTTY` (root.go:26); `output.ResolveFormat`, `output.NewStylesForWriter`. +- Produces: `reportHintLine(renderID string) string`; `printReportHint(cmd *cobra.Command, renderID string)` (gated, no-op when gates fail); `renderIDForHint(resp *api.Response) string`; `appendReportHint(cmd *cobra.Command, err error) error` (mutates the wrapped CLIError's Hint when gates pass, always returns err unchanged). + +- [ ] **Step 1: Write the failing tests** + +Create `internal/cmd/report_hint_test.go`. The harness idiom is `storage_test.go`: temp XDG config, `apitest` server, full `Execute`. Render needs only the api_secret, so `writeCompatConfig(t, dir, false)` suffices; text mode is forced with `--output-format text` and stderr TTY with `SetStderrTTYForTest(true)`. + +```go +package cmd + +import ( + "bytes" + "net/http" + "strings" + "testing" + + "github.com/urlbox/urlbox-cli/internal/api/apitest" +) + +const syncSuccessBody = `{"renderUrl":"https://renders.urlbox.com/x.png","size":17752}` + +func syncSuccessWithHeader() apitest.ScriptedResponse { + return apitest.ScriptedResponse{ + Status: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Urlbox-Request-Id": []string{"01a0906a-fff5-7565-8dc5-02868f11a4fa_ps"}, + }, + Body: syncSuccessBody, + } +} + +func runRenderForHint(t *testing.T, script apitest.ScriptedResponse, args ...string) (stdout, stderr string, code int) { + t.Helper() + dir := t.TempDir() + writeCompatConfig(t, dir, false) + t.Setenv("XDG_CONFIG_HOME", dir) + srv := apitest.New(script) + t.Cleanup(srv.Close) + t.Setenv("URLBOX_API_HOST", srv.URL()) + var out, errBuf bytes.Buffer + code = Execute(args, &out, &errBuf) + return out.String(), errBuf.String(), code +} + +func TestRenderHint_SyncSuccess_TextTTY_PrintsHintToStderr(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + stdout, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout, stderr) + } + if !strings.Contains(stderr, "urlbox report 01a0906a-fff5-7565-8dc5-02868f11a4fa_ps") { + t.Fatalf("stderr missing hint: %q", stderr) + } + if strings.Contains(stdout, "urlbox report") { + t.Fatalf("hint leaked to stdout: %q", stdout) + } +} + +func TestRenderHint_SyncSuccess_NonTTYStderr_NoHint(t *testing.T) { + SetStderrTTYForTest(false) + t.Cleanup(ResetStderrTTYForTest) + _, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stderr, "urlbox report") { + t.Fatalf("hint must be absent off-TTY: %q", stderr) + } +} + +func TestRenderHint_JSONMode_NoHint(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + stdout, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "json") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stderr, "urlbox report") || strings.Contains(stdout, "urlbox report") { + t.Fatalf("hint must be absent in json mode\nstdout=%q\nstderr=%q", stdout, stderr) + } + if strings.Contains(stdout, "renderId") { + t.Fatalf("sync json envelope must not gain a renderId field: %q", stdout) + } +} + +func TestRenderHint_QuietMode_NoHint(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + stdout, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "quiet") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stdout+stderr, "urlbox report") { + t.Fatalf("hint must be absent in quiet mode") + } +} + +func TestRenderHint_SyncSuccess_NoHeader_NoHint(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + _, stderr, code := runRenderForHint(t, apitest.SuccessJSON(syncSuccessBody), + "render", "https://example.com", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stderr, "urlbox report") { + t.Fatalf("no id captured, no hint: %q", stderr) + } +} + +func TestRenderHint_Async_UsesBodyRenderID(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + _, stderr, code := runRenderForHint(t, + apitest.SuccessJSON(`{"status":"created","renderId":"01a09072-9ec6-76ca-bcc7-bbf5cc35a9e4_pa","statusUrl":"https://api.urlbox.com/v1/render/x"}`), + "render", "https://example.com", "--async", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if !strings.Contains(stderr, "urlbox report 01a09072-9ec6-76ca-bcc7-bbf5cc35a9e4_pa") { + t.Fatalf("async hint missing: %q", stderr) + } +} + +func TestRenderHint_Failure_TextTTY_HintUnderError(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + fail := apitest.ScriptedResponse{ + Status: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"}`, + } + stdout, stderr, code := runRenderForHint(t, fail, + "render", "https://x.invalid", "--output-format", "text", "--no-retry") + if code != 2 { + t.Fatalf("exit %d, want 2 (validation)\n%s\n%s", code, stdout, stderr) + } + if !strings.Contains(stderr, "urlbox report 01a09082-25cf-74bd-9188-022c4f5bf25f_ps") { + t.Fatalf("failure hint missing from stderr: %q", stderr) + } +} + +func TestRenderHint_Failure_JSONMode_NoHintInEnvelope(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + fail := apitest.ScriptedResponse{ + Status: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"}`, + } + stdout, _, code := runRenderForHint(t, fail, + "render", "https://x.invalid", "--output-format", "json", "--no-retry") + if code != 2 { + t.Fatalf("exit %d, want 2", code) + } + if strings.Contains(stdout, "urlbox report") { + t.Fatalf("json error envelope must not carry the report hint: %q", stdout) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/cmd/ -run RenderHint -v` +Expected: FAIL — hint assertions unmet (success cases print no hint; failure case hint absent). + +- [ ] **Step 3: Implement** + +Create `internal/cmd/report_hint.go`: + +```go +package cmd + +import ( + "errors" + "fmt" + + "github.com/spf13/cobra" + + "github.com/urlbox/urlbox-cli/internal/api" + "github.com/urlbox/urlbox-cli/internal/output" +) + +// reportHintLine is the post-render affordance: one ready-to-run command +// carrying the render id. Copy is Arnold's; change only with his sign-off. +func reportHintLine(renderID string) string { + return "Something wrong with this render? Report it: urlbox report " + renderID +} + +// renderIDForHint picks the id the hint should name: async bodies carry +// renderId; sync bodies carry nothing, so the header-captured Response +// field is the fallback. +func renderIDForHint(resp *api.Response) string { + if id, ok := resp.Data["renderId"].(string); ok && id != "" { + return id + } + return resp.RenderID +} + +// hintGatesPass centralises the affordance rulings: text output only, +// stderr TTY only. json/quiet stay byte-identical for agents. +func hintGatesPass(cmd *cobra.Command) bool { + formatFlag, _ := cmd.Root().PersistentFlags().GetString("output-format") + if output.ResolveFormat(formatFlag, cmd.OutOrStdout()) != output.FormatText { + return false + } + return isStderrTTY(cmd.ErrOrStderr()) +} + +// printReportHint writes the muted hint line to stderr after a render +// result. No-op when any gate fails or no id was captured. +func printReportHint(cmd *cobra.Command, renderID string) { + if renderID == "" || !hintGatesPass(cmd) { + return + } + styles := output.NewStylesForWriter(cmd.ErrOrStderr()) + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), styles.Muted.Render(" "+reportHintLine(renderID))) +} + +// appendReportHint rides the failed render's existing Hint (printed under +// the Error: line in text mode) so the report affordance lands below the +// error rather than above it. The mutation only happens when the text+TTY +// gates pass, so json/quiet error envelopes never carry it. +func appendReportHint(cmd *cobra.Command, err error) error { + var ridErr *api.RenderIDError + if !errors.As(err, &ridErr) || ridErr.RenderID == "" || !hintGatesPass(cmd) { + return err + } + if ridErr.Err.Hint != "" { + ridErr.Err.Hint += "\n" + } + ridErr.Err.Hint += reportHintLine(ridErr.RenderID) + return err +} +``` + +Modify `internal/cmd/render.go`. The error branch (line 402-412) gets one added line before `return err`: + +```go + if err != nil { + var cli *output.CLIError + if errors.As(err, &cli) { + if cli.Code == output.ErrTimeout || cli.Code == output.ErrNetwork { + cli.Hint = networkHint(errors.New(cli.Message), true, f.timeout) + } + } + return appendReportHint(cmd, err) + } +``` + +The success tail (line 445-453) becomes: + +```go + env := output.NewEnvelope( + "render", + resp.Data, + summariseRenderResp(resp), + breadcrumbsForResp(resp, f), + ) + env.Warnings = warnings + werr := writeRenderEnvelope(cmd, env) + printReportHint(cmd, renderIDForHint(resp)) + return werr +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/cmd/ -run RenderHint -v` then `go test ./internal/cmd/` +Expected: PASS, including every pre-existing render/status test (the envelope and json/quiet bytes are unchanged). + +- [ ] **Step 5: `make ci`, commit** + +```bash +make ci +git add internal/cmd/report_hint.go internal/cmd/report_hint_test.go internal/cmd/render.go +git commit -m "feat(render): report hint line on every render response" +``` + +--- + +### Task 3: prompt.TextInput + +**Files:** +- Modify: `internal/prompt/prompt.go` +- Create: `internal/prompt/textinput_test.go` + +**Interfaces:** +- Produces: `prompt.TextInput(title string, validate func(string) error) (string, error)` — huh single-line input on stderr, trimmed result; `validate` (nil-able) runs inside huh so the user retries in place; returns `prompt.ErrNotInteractive` off-TTY. + +- [ ] **Step 1: Write the failing test** + +Create `internal/prompt/textinput_test.go` (test binaries have no TTY stdin, so the reachable path is the guard): + +```go +package prompt + +import ( + "errors" + "testing" +) + +func TestTextInput_NonInteractive_ReturnsErrNotInteractive(t *testing.T) { + _, err := TextInput("Add a short description:", nil) + if !errors.Is(err, ErrNotInteractive) { + t.Fatalf("want ErrNotInteractive, got %v", err) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/prompt/ -run TextInput -v` +Expected: FAIL — `TextInput` undefined. + +- [ ] **Step 3: Implement** + +Append to `internal/prompt/prompt.go` (same shape as `TypeToConfirm`, line 76): + +```go +// TextInput draws a single-line text input to stderr and returns the +// trimmed value. validate (optional) runs inside the form so the user can +// correct in place. It returns ErrNotInteractive when stdin is not a +// terminal. +func TextInput(title string, validate func(string) error) (string, error) { + if !term.IsTerminal(int(os.Stdin.Fd())) { //nolint:gosec // file descriptors fit in int on every platform Go supports + return "", ErrNotInteractive + } + var typed string + input := huh.NewInput().Title(title).Value(&typed) + if validate != nil { + input = input.Validate(validate) + } + if err := input.WithTheme(theme()).Run(); err != nil { + return "", err + } + return strings.TrimSpace(typed), nil +} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/prompt/ -v` +Expected: PASS. + +- [ ] **Step 5: `make ci`, commit** + +```bash +make ci +git add internal/prompt/prompt.go internal/prompt/textinput_test.go +git commit -m "feat(prompt): TextInput single-line prompt" +``` + +--- + +### Task 4: `urlbox report` — flag path, errors, output + +**Files:** +- Create: `internal/cmd/report.go` +- Create: `internal/cmd/report_test.go` +- Modify: `internal/cmd/root.go` (registration, after `newRenderCmd()` line 239) +- Modify: `SURFACE.txt` (via `make surface-snapshot`) + +**Interfaces:** +- Consumes: `loadSession`, `requireActiveOrg`, `attachSessionRetryFlags` (session_helpers.go); `asCLIError` (login_resolve.go:155); `writeEnvelopeWithQuietData` (config.go:638); `valueOrEmpty`; `interactiveText` (projects.go:538). +- Produces: `newReportCmd() *cobra.Command`; `reportCategories` (ordered slice of `{value, label}` pairs — Task 5's picker reuses it); `renderReportsPath(org string) string` returning `"/v2/organisation/" + org + "/render-reports"`; `runReport` split so Task 5 only fills the interactive branch stub `resolveReportInputs`. + +- [ ] **Step 1: Write the failing tests** + +Create `internal/cmd/report_test.go`: + +```go +package cmd + +import ( + "bytes" + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/urlbox/urlbox-cli/internal/api/apitest" +) + +const reportDTOJSON = `{"id":"rpt_x1","renderId":"01a0906a-fff5-7565-8dc5-02868f11a4fa_ps","category":"bot-detection","comment":"page shows a captcha","rects":[],"state":"open","outputUrl":null,"outputFormat":null,"resolutionMessage":null,"resolutionExampleOptions":null,"resolutionExampleUrl":null,"hasUnseenResolution":false,"createdAt":"2026-09-11T12:00:00.000Z"}` + +func runReport(t *testing.T, scripts []apitest.ScriptedResponse, args ...string) (stdout, stderr string, code int, srv *apitest.Server) { + t.Helper() + dir := t.TempDir() + writeCompatConfig(t, dir, true) + t.Setenv("XDG_CONFIG_HOME", dir) + srv = apitest.New(scripts...) + t.Cleanup(srv.Close) + t.Setenv("URLBOX_API_HOST", srv.URL()) + var out, errBuf bytes.Buffer + code = Execute(args, &out, &errBuf) + return out.String(), errBuf.String(), code, srv +} + +func TestReport_FlagsPath_PostsAndSummarises(t *testing.T) { + stdout, stderr, code, srv := runReport(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--category", "bot-detection", "--comment", "page shows a captcha", + "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout, stderr) + } + reqs := srv.Requests() + if reqs[0].Method != "POST" || reqs[0].Path != "/v2/organisation/org_compat/render-reports" { + t.Fatalf("request: %+v", reqs[0]) + } + var body map[string]any + if err := json.Unmarshal(reqs[0].Body, &body); err != nil { + t.Fatalf("body: %v", err) + } + if body["renderId"] != "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps" || + body["category"] != "bot-detection" || + body["comment"] != "page shows a captcha" { + t.Fatalf("body: %v", body) + } + if _, present := body["rects"]; present { + t.Fatalf("rects must not be sent: %v", body) + } + if !strings.Contains(stdout, "Report filed for render 01a0906a-fff5-7565-8dc5-02868f11a4fa_ps") { + t.Fatalf("summary missing: %q", stdout) + } +} + +func TestReport_JSONMode_EmitsDTOEnvelope(t *testing.T) { + stdout, _, code, _ := runReport(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--category", "other", "--comment", "x", + "--output-format", "json") + if code != 0 { + t.Fatalf("exit %d\n%s", code, stdout) + } + var env map[string]any + if err := json.Unmarshal([]byte(stdout), &env); err != nil { + t.Fatalf("envelope: %v", err) + } + if env["ok"] != true || env["command"] != "report" { + t.Fatalf("envelope: %v", env) + } + data, _ := env["data"].(map[string]any) + if data["id"] != "rpt_x1" || data["state"] != "open" { + t.Fatalf("data: %v", data) + } +} + +func TestReport_QuietMode_EmitsReportID(t *testing.T) { + stdout, _, code, _ := runReport(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--category", "other", "--comment", "x", + "--output-format", "quiet") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.TrimSpace(stdout) != `"rpt_x1"` { + t.Fatalf("quiet stdout=%q, want the report id", stdout) + } +} + +func TestReport_InvalidCategory_UsageErrorListsValues(t *testing.T) { + _, stderr, code, _ := runReport(t, nil, + "report", "01a0_ps", "--category", "captcha", "--comment", "x", + "--output-format", "text") + if code != 1 { + t.Fatalf("exit %d, want 1 (usage)", code) + } + for _, want := range []string{"bot-detection", "login-required", "missing-content", "cookie-banner-or-popup", "render-failed", "other"} { + if !strings.Contains(stderr, want) { + t.Fatalf("error must list %q: %q", want, stderr) + } + } +} + +func TestReport_EmptyComment_UsageError(t *testing.T) { + _, _, code, _ := runReport(t, nil, + "report", "01a0_ps", "--category", "other", "--comment", " ", + "--output-format", "text") + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } +} + +func TestReport_MissingFlags_NonInteractive_UsageError(t *testing.T) { + stdout, _, code, _ := runReport(t, nil, + "report", "01a0_ps", "--output-format", "json") + if code != 1 { + t.Fatalf("exit %d, want 1 (never hangs)", code) + } + if !strings.Contains(stdout, `"usage"`) || !strings.Contains(stdout, "--category") { + t.Fatalf("json error envelope must name the missing flags: %q", stdout) + } +} + +func TestReport_NotLoggedIn_AuthError(t *testing.T) { + dir := t.TempDir() + writeCompatConfig(t, dir, false) + t.Setenv("XDG_CONFIG_HOME", dir) + var out, errBuf bytes.Buffer + code := Execute([]string{"report", "01a0_ps", "--category", "other", "--comment", "x", "--output-format", "text"}, &out, &errBuf) + if code != 3 { + t.Fatalf("exit %d, want 3 (auth)", code) + } + if !strings.Contains(errBuf.String(), "urlbox login") { + t.Fatalf("auth error must carry the login hint: %q", errBuf.String()) + } +} + +func TestReport_RenderNotFound_Exit5WithExpiryHint(t *testing.T) { + notFound := apitest.ScriptedResponse{ + Status: http.StatusNotFound, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"defined":false,"code":"NOT_FOUND","status":404,"message":"Render not found"}`, + } + _, stderr, code, _ := runReport(t, []apitest.ScriptedResponse{notFound}, + "report", "01a0906a-gone_ps", "--category", "other", "--comment", "x", + "--output-format", "text") + if code != 5 { + t.Fatalf("exit %d, want 5", code) + } + if !strings.Contains(stderr, "Render not found") { + t.Fatalf("API message must surface: %q", stderr) + } + if !strings.Contains(stderr, "60 days") { + t.Fatalf("hint must mention render expiry: %q", stderr) + } +} + +func TestReport_LockedReport_Exit7SurfacesAPIMessage(t *testing.T) { + locked := apitest.ScriptedResponse{ + Status: http.StatusConflict, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"defined":false,"code":"CONFLICT","status":409,"message":"This report is being looked at by the team and can no longer be edited."}`, + } + _, stderr, code, _ := runReport(t, []apitest.ScriptedResponse{locked}, + "report", "01a0_ps", "--category", "other", "--comment", "x", + "--output-format", "text") + if code != 7 { + t.Fatalf("exit %d, want 7", code) + } + if !strings.Contains(stderr, "being looked at by the team") { + t.Fatalf("API conflict message must surface: %q", stderr) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/cmd/ -run TestReport_ -v` +Expected: FAIL — `unknown command "report"`. + +- [ ] **Step 3: Implement** + +Create `internal/cmd/report.go`: + +```go +package cmd + +import ( + "context" + "fmt" + "strings" + + "github.com/spf13/cobra" + + "github.com/urlbox/urlbox-cli/internal/output" +) + +// reportCategory pairs the API enum value with the human label the +// interactive picker shows. Order is the picker order. Labels are +// customer-facing copy — Arnold's. +type reportCategory struct { + value string + label string +} + +// reportCategories is the closed category set from the mono render-report +// contract (packages/domain/src/render-report/render-report.schemas.ts). +var reportCategories = []reportCategory{ + {value: "bot-detection", label: "Bot detection / blocked"}, + {value: "login-required", label: "Login required"}, + {value: "missing-content", label: "Missing content"}, + {value: "cookie-banner-or-popup", label: "Cookie banner or popup"}, + {value: "render-failed", label: "Render failed"}, + {value: "other", label: "Other"}, +} + +func categoryValues() []string { + vals := make([]string, len(reportCategories)) + for i, c := range reportCategories { + vals[i] = c.value + } + return vals +} + +func isValidCategory(v string) bool { + for _, c := range reportCategories { + if c.value == v { + return true + } + } + return false +} + +const maxReportCommentLen = 2000 + +// renderReportsPath builds the org-scoped create/list path, matching the +// credkind orgListPath convention. +func renderReportsPath(org string) string { + return "/v2/organisation/" + org + "/render-reports" +} + +func newReportCmd() *cobra.Command { + var category, comment string + c := &cobra.Command{ + Use: "report ", + Short: "Report a problem with a render", + Long: `File a render report with the Urlbox team. + +The render id is printed after every render, and async responses carry it +as renderId. Requires a signed-in session (urlbox login) with an active +organisation. Reports are limited to a category plus a short comment; the +team follows up via support. + +Without --category/--comment in an interactive terminal, the command +prompts for the missing values. In non-interactive use both flags are +required. + +Exit codes: + 0 report filed + 1 usage (missing/invalid category or comment in non-interactive use) + 3 not logged in + 5 render not found (expired or not from your organisation) + 7 the report is already with the team and locked`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runReport(cmd, args[0], category, comment) + }, + } + c.Flags().StringVar(&category, "category", "", "Problem category: "+strings.Join(categoryValues(), ", ")) + c.Flags().StringVar(&comment, "comment", "", "Short description of what went wrong (required with --category)") + attachSessionRetryFlags(c) + return c +} + +func runReport(cmd *cobra.Command, renderID, category, comment string) error { + sess, cliErr := loadSession(cmd) + if cliErr != nil { + return cliErr + } + org, orgErr := requireActiveOrg(sess) + if orgErr != nil { + return orgErr + } + category, comment, inErr := resolveReportInputs(cmd, category, comment) + if inErr != nil { + return inErr + } + body := map[string]any{ + "renderId": renderID, + "category": category, + "comment": comment, + } + ctx := context.Background() + var created map[string]any + if err := sess.Client.PostJSON(ctx, renderReportsPath(org), body, &created); err != nil { + return refineReportError(err) + } + env := output.NewEnvelope("report", created, + fmt.Sprintf("Report filed for render %s", renderID), nil) + return writeEnvelopeWithQuietData(cmd, env, valueOrEmpty(created["id"])) +} + +// resolveReportInputs validates flag-provided values and fills missing ones +// interactively (text mode + TTY). Task 5 wires the prompts; until then the +// interactive branch behaves like non-interactive. +func resolveReportInputs(cmd *cobra.Command, category, comment string) (string, string, *output.CLIError) { + if category != "" && !isValidCategory(category) { + return "", "", output.NewCLIError(output.ErrUsage, + fmt.Sprintf("unknown --category %q", category), + "Use one of: "+strings.Join(categoryValues(), ", ")+".") + } + comment = strings.TrimSpace(comment) + if len(comment) > maxReportCommentLen { + return "", "", output.NewCLIError(output.ErrUsage, + fmt.Sprintf("--comment is too long (%d chars, max %d)", len(comment), maxReportCommentLen), + "Shorten the comment; detail can follow via urlbox support.") + } + if category != "" && comment != "" { + return category, comment, nil + } + if interactiveText(cmd) { + return promptReportInputs(cmd, category, comment) + } + return "", "", missingReportFlagsError(category, comment) +} + +func missingReportFlagsError(category, comment string) *output.CLIError { + missing := []string{} + if category == "" { + missing = append(missing, "--category") + } + if comment == "" { + missing = append(missing, "--comment") + } + return output.NewCLIError(output.ErrUsage, + "missing "+strings.Join(missing, " and "), + "Pass --category ("+strings.Join(categoryValues(), ", ")+") and --comment, or run in an interactive terminal.") +} + +// promptReportInputs is completed in the interactive task; the flag path +// never reaches it with both values set. +func promptReportInputs(cmd *cobra.Command, category, comment string) (string, string, *output.CLIError) { + return "", "", missingReportFlagsError(category, comment) +} + +// refineReportError swaps the generic 404/409 hints (written for render +// lookups) for report-specific recovery text. Everything else passes +// through asCLIError untouched. +func refineReportError(err error) error { + cli := asCLIError(err) + switch cli.Code { + case output.ErrNotFound: + cli.Hint = "Renders are kept for around 60 days and must belong to your active organisation. Check the id against a recent render." + case output.ErrConflict: + cli.Hint = "The team already has this report in review. Use `urlbox support` if you need to add something." + } + return cli +} +``` + +Register in `internal/cmd/root.go` after `newRenderCmd()` (line 239): + +```go + cmd.AddCommand(newRenderCmd()) + cmd.AddCommand(newReportCmd()) +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/cmd/ -run TestReport_ -v` then `go test ./internal/cmd/` +Expected: PASS. (`TestReport_MissingFlags_NonInteractive_UsageError` passes already because the interactive stub falls through to the usage error — Task 5 upgrades it.) + +- [ ] **Step 5: Surface snapshot, `make ci`, commit** + +```bash +make surface-snapshot +make ci +git add internal/cmd/report.go internal/cmd/report_test.go internal/cmd/root.go SURFACE.txt +git commit -m "feat(report): urlbox report command, flag path" +``` + +Expected new SURFACE.txt entries: `urlbox report ` plus `--agent --category --comment --jq --max-retries --no-retry --output-format --profile` lines. + +--- + +### Task 5: `urlbox report` — interactive path + +**Files:** +- Modify: `internal/cmd/report.go` (replace the `promptReportInputs` stub) +- Modify: `internal/cmd/report_test.go` (add interactive tests) + +**Interfaces:** +- Consumes: `promptPick` (session_helpers.go:153), `pickFunc` (login_resolve.go:13), `errNotInteractivePick` (login_resolve.go:15), `prompt.TextInput` (Task 3), `reportCategories` (Task 4). +- Produces: package vars `reportCategoryPick pickFunc` and `reportCommentInput func(string, func(string) error) (string, error)` with `SetReportPromptsForTest(pick pickFunc, input func(string, func(string) error) (string, error))` / `ResetReportPromptsForTest()`. + +- [ ] **Step 1: Write the failing tests** + +Append to `internal/cmd/report_test.go`: + +```go +func TestReport_Interactive_PromptsForCategoryAndComment(t *testing.T) { + SetReportPromptsForTest( + func(label string, options []string, active int) (int, error) { + if len(options) != 6 { + t.Fatalf("picker options: %v", options) + } + return 0, nil + }, + func(title string, validate func(string) error) (string, error) { + if validate == nil { + t.Fatal("comment input must validate non-empty") + } + if err := validate(""); err == nil { + t.Fatal("empty comment must fail validation") + } + return "page shows a captcha", nil + }, + ) + t.Cleanup(ResetReportPromptsForTest) + stdout, stderr, code, srv := runReport(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout, stderr) + } + var body map[string]any + if err := json.Unmarshal(srv.Requests()[0].Body, &body); err != nil { + t.Fatalf("body: %v", err) + } + if body["category"] != "bot-detection" || body["comment"] != "page shows a captcha" { + t.Fatalf("prompted values not sent: %v", body) + } +} + +func TestReport_Interactive_FlagCategoryOnlyPromptsComment(t *testing.T) { + picked := false + SetReportPromptsForTest( + func(label string, options []string, active int) (int, error) { + picked = true + return 0, nil + }, + func(title string, validate func(string) error) (string, error) { + return "left half is blank", nil + }, + ) + t.Cleanup(ResetReportPromptsForTest) + _, _, code, srv := runReport(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0_ps", "--category", "missing-content", + "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if picked { + t.Fatal("category picker must not run when --category was given") + } + var body map[string]any + if err := json.Unmarshal(srv.Requests()[0].Body, &body); err != nil { + t.Fatalf("body: %v", err) + } + if body["category"] != "missing-content" || body["comment"] != "left half is blank" { + t.Fatalf("body: %v", body) + } +} + +func TestReport_Interactive_NonTTYStdin_FallsBackToUsage(t *testing.T) { + ResetReportPromptsForTest() + _, stderr, code, _ := runReport(t, nil, + "report", "01a0_ps", "--output-format", "text") + if code != 1 { + t.Fatalf("exit %d, want 1 (prompt.ErrNotInteractive path)", code) + } + if !strings.Contains(stderr, "--category") { + t.Fatalf("usage error must name the flags: %q", stderr) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/cmd/ -run TestReport_Interactive -v` +Expected: FAIL — `SetReportPromptsForTest` undefined. + +- [ ] **Step 3: Implement** + +In `internal/cmd/report.go`, replace the `promptReportInputs` stub and add the injection vars: + +```go +var ( + reportCategoryPick pickFunc = promptPick + reportCommentInput func(string, func(string) error) (string, error) = prompt.TextInput +) + +// SetReportPromptsForTest swaps both report prompts. Pair with +// t.Cleanup(ResetReportPromptsForTest). +func SetReportPromptsForTest(pick pickFunc, input func(string, func(string) error) (string, error)) { + reportCategoryPick = pick + reportCommentInput = input +} + +// ResetReportPromptsForTest restores the production prompts. +func ResetReportPromptsForTest() { + reportCategoryPick = promptPick + reportCommentInput = prompt.TextInput +} + +func promptReportInputs(cmd *cobra.Command, category, comment string) (string, string, *output.CLIError) { + if category == "" { + labels := make([]string, len(reportCategories)) + for i, c := range reportCategories { + labels[i] = c.label + } + idx, err := reportCategoryPick("What went wrong with this render?", labels, -1) + if err != nil { + return "", "", missingReportFlagsError(category, comment) + } + category = reportCategories[idx].value + } + if comment == "" { + typed, err := reportCommentInput("Add a short description:", func(s string) error { + if strings.TrimSpace(s) == "" { + return errors.New("a short description is required") + } + if len(s) > maxReportCommentLen { + return fmt.Errorf("keep it under %d characters", maxReportCommentLen) + } + return nil + }) + if err != nil { + return "", "", missingReportFlagsError(category, comment) + } + comment = typed + } + return category, comment, nil +} +``` + +Add `"errors"` and `"github.com/urlbox/urlbox-cli/internal/prompt"` to report.go's imports. + +Note: `promptPick` maps `prompt.ErrNotInteractive` to `errNotInteractivePick`; both prompt error paths intentionally collapse to the same usage error the non-interactive branch produces, so a piped stdin can never hang or half-file. + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/cmd/ -run TestReport_ -v` +Expected: PASS, including the Task 4 tests unchanged. + +- [ ] **Step 5: `make ci`, commit** + +```bash +make ci +git add internal/cmd/report.go internal/cmd/report_test.go +git commit -m "feat(report): interactive category and comment prompts" +``` + +--- + +### Task 6: `urlbox support` + +**Files:** +- Create: `internal/cmd/support.go` +- Create: `internal/cmd/support_test.go` +- Modify: `internal/cmd/root.go` (registration, after `newStorageCmd()` line 244) +- Modify: `SURFACE.txt` (via `make surface-snapshot`) + +**Interfaces:** +- Consumes: `browser.Opener` / `browser.NewOSOpener` (internal/browser/opener.go), `detectHeadless` (dashboard.go:54), `writeEnvelope` (config.go:623). +- Produces: `newSupportCmd() *cobra.Command`; test hooks `SetSupportOpenerForTest` / `ResetSupportOpenerForTest` (own opener var, per the dashboard no-cross-contamination note at dashboard.go:20-24). + +- [ ] **Step 1: Write the failing tests** + +Create `internal/cmd/support_test.go`: + +```go +package cmd + +import ( + "bytes" + "encoding/json" + "strings" + "testing" +) + +type fakeSupportOpener struct{ opened []string } + +func (f *fakeSupportOpener) Open(url string) error { + f.opened = append(f.opened, url) + return nil +} + +func TestSupport_TextMode_OpensContactPage(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + SetHeadlessDetectorForTest(func() bool { return false }) + t.Cleanup(ResetHeadlessDetectorForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "text"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout.String(), stderr.String()) + } + if len(fake.opened) != 1 || fake.opened[0] != "https://urlbox.com/contact" { + t.Fatalf("opened: %v", fake.opened) + } +} + +func TestSupport_JSONMode_NoBrowserEmitsEnvelope(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "json"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d", code) + } + if len(fake.opened) != 0 { + t.Fatalf("json mode must not launch a browser: %v", fake.opened) + } + var env map[string]any + if err := json.Unmarshal(stdout.Bytes(), &env); err != nil { + t.Fatalf("envelope: %v", err) + } + data, _ := env["data"].(map[string]any) + if env["command"] != "support" || data["url"] != "https://urlbox.com/contact" { + t.Fatalf("envelope: %v", env) + } +} + +func TestSupport_Headless_PrintsURLToStderr(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + SetHeadlessDetectorForTest(func() bool { return true }) + t.Cleanup(ResetHeadlessDetectorForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "text"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d", code) + } + if len(fake.opened) != 0 { + t.Fatalf("headless must not launch: %v", fake.opened) + } + if !strings.Contains(stderr.String(), "https://urlbox.com/contact") { + t.Fatalf("stderr must carry the URL: %q", stderr.String()) + } +} + +func TestSupport_QuietMode_EmitsBareURL(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "quiet"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.TrimSpace(stdout.String()) != "https://urlbox.com/contact" { + t.Fatalf("quiet stdout=%q", stdout.String()) + } + if len(fake.opened) != 0 { + t.Fatalf("quiet mode must not launch: %v", fake.opened) + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `go test ./internal/cmd/ -run TestSupport_ -v` +Expected: FAIL — `unknown command "support"`. + +- [ ] **Step 3: Implement** + +Create `internal/cmd/support.go` (mirror of dashboard.go with its own opener): + +```go +package cmd + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/urlbox/urlbox-cli/internal/browser" + "github.com/urlbox/urlbox-cli/internal/output" +) + +// supportURL is the Urlbox contact page — the canonical support entry +// point (ruled 2026-09-11; /support has no dedicated page). +const supportURL = "https://urlbox.com/contact" + +// supportOpener is the browser opener used by the support command. Own var +// so test injection doesn't cross-contaminate dashboard's. +var supportOpener browser.Opener = browser.NewOSOpener() + +// SetSupportOpenerForTest swaps in a fake browser.Opener. Pair with +// t.Cleanup(ResetSupportOpenerForTest). +func SetSupportOpenerForTest(o browser.Opener) { supportOpener = o } + +// ResetSupportOpenerForTest restores the production OSOpener. +func ResetSupportOpenerForTest() { supportOpener = browser.NewOSOpener() } + +func newSupportCmd() *cobra.Command { + return &cobra.Command{ + Use: "support", + Short: "Open the Urlbox support contact page in your browser", + Long: `Opens https://urlbox.com/contact in your default browser. + +On headless environments the URL is printed to stderr instead. The +standard envelope is still emitted on stdout so agents and pipelines can +rely on the same shape regardless of host. + +Exit codes: + 0 browser launched, or URL printed in headless mode + 10 the OS browser handler returned an error (URL is in the hint)`, + Args: cobra.NoArgs, + RunE: func(c *cobra.Command, _ []string) error { + return runSupport(c) + }, + } +} + +func runSupport(c *cobra.Command) error { + data := map[string]any{"url": supportURL} + formatFlag, _ := c.Root().PersistentFlags().GetString("output-format") + resolvedFormat := output.ResolveFormat(formatFlag, c.OutOrStdout()) + if resolvedFormat == output.FormatJSON || resolvedFormat == output.FormatQuiet { + return writeSupportEnvelope(c, data, + "Support URL emitted (no browser launched in machine-readable mode)", + []output.Breadcrumb{{Action: "copy", Cmd: supportURL}}) + } + if isHeadless() { + _, _ = fmt.Fprintln(c.ErrOrStderr(), + "Support URL: "+supportURL+" (open in any browser)") + return writeSupportEnvelope(c, data, + "Support URL printed (no graphical session detected)", + []output.Breadcrumb{{Action: "copy", Cmd: supportURL}}) + } + if err := supportOpener.Open(supportURL); err != nil { + return output.NewCLIError( + output.ErrServer, + "Failed to open browser: "+err.Error(), + "Open this URL manually: "+supportURL, + ) + } + return writeSupportEnvelope(c, data, + "Opened "+supportURL, + []output.Breadcrumb{{Action: "browse", Cmd: supportURL}}) +} + +// writeSupportEnvelope mirrors writeDashboardEnvelope: quiet emits the +// bare URL for pipelines, everything else the standard envelope. +func writeSupportEnvelope(c *cobra.Command, data map[string]any, summary string, breadcrumbs []output.Breadcrumb) error { + formatFlag, _ := c.Root().PersistentFlags().GetString("output-format") + jqExpr, _ := c.Root().PersistentFlags().GetString("jq") + stdout := c.OutOrStdout() + format := output.ResolveFormat(formatFlag, stdout) + if format == output.FormatQuiet && jqExpr == "" { + _, err := fmt.Fprintln(stdout, supportURL) + return err + } + env := output.NewEnvelope("support", data, summary, breadcrumbs) + return writeEnvelope(c, env) +} +``` + +Register in `internal/cmd/root.go` after `newStorageCmd()` (line 244): + +```go + cmd.AddCommand(newStorageCmd()) + cmd.AddCommand(newSupportCmd()) +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `go test ./internal/cmd/ -run TestSupport_ -v` then `go test ./internal/cmd/` +Expected: PASS. + +- [ ] **Step 5: Surface snapshot, `make ci`, commit** + +```bash +make surface-snapshot +make ci +git add internal/cmd/support.go internal/cmd/support_test.go internal/cmd/root.go SURFACE.txt +git commit -m "feat(support): urlbox support opens the contact page" +``` + +Expected new SURFACE.txt entries: `urlbox support` plus `--agent --jq --output-format --profile`. + +--- + +### Task 7: Docs, skill, changelog, final gate + +**Files:** +- Modify: `skills/SKILL.md` (add report + support to the agent skill) +- Modify: `README.md` and `npm/README.md` (command list entries) +- Modify: `CHANGELOG.md` (unreleased entry) + +**Interfaces:** none new — documentation of Tasks 4-6 surfaces exactly as shipped. + +- [ ] **Step 1: Update skills/SKILL.md** + +Two rows in the `## Available commands` table (line 151): `urlbox report` after the `urlbox video ` row, `urlbox support` after the `urlbox dashboard` row: + +```markdown +| `urlbox report ` | Report a bad render to the Urlbox team (category + comment) | +| `urlbox support` | Open the Urlbox support contact page in the user's browser | +``` + +Two sections after the `## dashboard: open the Urlbox dashboard` section (line 417), matching its shape: + +```markdown +## report: report a bad render + +`urlbox report ` files a render report with the Urlbox team. +Requires a signed-in session — the HUMAN runs `urlbox login` (browser +device flow) once; agents never sign in themselves and the render +secret (`URLBOX_API_SECRET`) cannot file reports. + +Non-interactive use requires both flags; the command never prompts when +stdin is not a terminal: + +```sh +urlbox report 01a0906a-…_ps --category bot-detection --comment "page shows a captcha" --output-format json --jq '.data.id' +``` + +`--category` is a closed set: `bot-detection`, `login-required`, +`missing-content`, `cookie-banner-or-popup`, `render-failed`, `other`. +Where the renderId comes from: async render responses carry +`data.renderId`; sync render JSON carries NO id (use `--async` when you +plan to report). One report per render — re-filing while the report is +open edits it in place; once the team picks it up you get exit 7. + +Exit codes: 0 filed; 1 missing/invalid category or comment; 3 not +logged in; 5 render not found (expired ~60 days or another org's); +7 report locked (already in review). + +## support: open the support contact page + +`urlbox support` opens https://urlbox.com/contact. Same envelope contract +as `dashboard`: json/quiet never launch a browser and always carry +`data.url`; headless prints the URL to stderr. +``` + +- [ ] **Step 2: Update README.md** + +In the `| Command | Does |` table under "Account and context" (README.md:128), add after the `login` / `logout` row block, matching row style: + +```markdown +| `report ` | File a render problem report with the Urlbox team (needs `login`) | +| `support` | Open the support contact page in your browser | +``` + +And a short section after "### Async renders" (README.md:107): + +```markdown +### Reporting a bad render + +```sh +urlbox report 01a0906a-…_ps # interactive: category picker + comment +urlbox report 01a0906a-…_ps --category bot-detection --comment "page shows a captcha" +``` + +Every render prints its id in a hint line under the result. `report` files +the problem straight to the Urlbox team (requires `urlbox login`); the team +follows up on your report from the dashboard. `urlbox support` opens the +contact page for anything else. +``` + +`npm/README.md` needs no change — it is a quick-start that does not +enumerate commands (verify this is still true before skipping). + +- [ ] **Step 3: CHANGELOG entry** + +Add above the `## v1.2.0 — 2026-08-19` heading (CHANGELOG.md:7), matching house style: + +```markdown +## Unreleased + +**Report a bad render from the CLI.** Every render response now prints a +muted hint line with the render's id and the ready-to-run report command +(interactive terminals only — json/quiet/piped output is byte-identical). +`urlbox report ` files the report: `--category` + `--comment` +for agents and scripts, an interactive category picker + comment prompt +for humans. Reports need a signed-in session (`urlbox login`); render +secrets cannot file them. `urlbox support` opens the support contact +page, with the dashboard command's headless and machine-readable +behaviour. +``` + +Version number stays unset (release numbering is Arnold+Gus). + +- [ ] **Step 4: Full gate** + +```bash +make ci +make surface-snapshot +git status --short # SURFACE.txt must be clean (no drift from Task 6) +git add skills/SKILL.md README.md CHANGELOG.md +git commit -m "docs: report and support commands" +``` + +- [ ] **Step 5: Arnold's manual gate (his hands, not the executor's)** + +Hand Arnold this checklist verbatim; do not run it for him: + +```bash +urlbox login # refresh the expired session first +urlbox render https://example.com # expect hint line with a _ps id +urlbox render https://example.com --output-format json # expect NO hint, NO renderId field +urlbox report # interactive picker + comment +urlbox report --category other --comment "edited from CLI" # upsert edits in place +urlbox report doesnotexist --category other --comment x # expect exit 5, expiry hint +urlbox report --category other --comment x --output-format json # DTO envelope +urlbox support # browser lands on /contact +``` + +Note: the first real `urlbox report` against production opens a Crisp +conversation and pings the team Slack — deliberate, once. + +After his pass: squash to one commit on latest origin/main (rebase, force-push with lease) — message proposed to Arnold first. STOP. No PR without his explicit ask. diff --git a/docs/superpowers/specs/2026-09-11-render-report-command-design.md b/docs/superpowers/specs/2026-09-11-render-report-command-design.md new file mode 100644 index 0000000..5c474bd --- /dev/null +++ b/docs/superpowers/specs/2026-09-11-render-report-command-design.md @@ -0,0 +1,204 @@ +# CLI render reporting — design + +**Date:** 2026-09-11 +**Status:** approved by Arnold (brainstorm session); implementation not started +**Branch:** new branch off latest `origin/main` (main tip `c9d96d6`, v1.2.0) +**Handoff:** `docs/superpowers/HANDOFF-2026-09-11-render-report-command.md` + +Reporting a bad render from the CLI, via the mono's public `/v2` render-report +endpoints. Three pieces: a post-render hint line, `urlbox report `, +and `urlbox support`. + +## Rulings (Arnold, 2026-09-11) + +- **Session-only auth.** The render-report routes accept only a logged-in + session (device-flow bearer token) or a better-auth org API key the CLI + doesn't hold. The project render secret (`ubx_sk_…`) is never accepted — + verified live against production (401 as `Authorization: Bearer`, 401 + "Invalid API key" as `x-api-key`). This is the intended direction, not a + limitation: session is the new way. Agents don't log in themselves — they + ask the human to run the browser-launched `urlbox login`, then inherit the + session from config. +- **Hint line, not a keypress.** The post-render affordance is one muted + stderr line containing the ready-to-run report command. The process exits + immediately; nothing listens for keys, nothing blocks. +- **Hint on every render response** — sync or async, success or failure — + whenever the response yields a render id. Every render, no backoff, no + config knob. +- **No last-render memory.** The CLI persists nothing after a render. + `urlbox report` always takes an explicit renderId; the hint line in + scrollback is the memory. No `--last` flag. +- **No JSON envelope changes.** The render id captured for the hint stays + in-memory only. Sync JSON output keeps its current shape (no `renderId` + field added). Agents that need the id use `--async` (its body carries it). +- **Limited reports:** category + comment only. `rects` deliberately out + (dashboard's click-to-highlight concept). +- **Interactive flow collects both** category (house `SelectOne`) and a + one-line comment — the API requires both. +- **`urlbox support` opens `https://urlbox.com/contact`.** (`/support` + serves a 200 in production but has no page in the dashboard source; not + the CLI's problem to solve.) +- Out of scope for this branch: the `respond` endpoint, a report *list* + command, org-API-key auth, CLI-self-problem reporting, mining the archived + `urlbox/cli` repo, any rating mechanic. + +## API contract (pinned from mono origin/main 2026-09-11 — re-verify at build time) + +`POST /v2/organisation/{org}/render-reports` — `{org}` is the organisation +public id (`org_…`, from the profile's `active_org`). + +Body: `renderId` (string 1–255), `category` (enum below), `comment` +(string 1–2000, required), `rects` (optional; CLI never sends it). +Categories: `bot-detection`, `login-required`, `missing-content`, +`cookie-banner-or-popup`, `render-failed`, `other`. + +Response DTO: `id` (`rpt_…`), `renderId`, `category`, `comment`, `rects`, +`state` (`open` | `in_review` | `resolved`), `outputUrl`, `outputFormat`, +`resolutionMessage`, `resolutionExampleOptions`, `resolutionExampleUrl`, +`hasUnseenResolution`, `createdAt`. + +Semantics that shape CLI behaviour (verified in origin/main source): + +- **The render must exist and belong to the caller's org**, else a uniform + 404 "Render not found" (no existence oracle). Renders expire from Mongo + after ~60 days. (The handoff's "missing renderIds accepted by design" is + stale — corrected.) +- **One report per render (upsert).** Re-creating while the report is `open` + edits it in place. Once `in_review`/`resolved` it is locked → 409 CONFLICT + "This report is being looked at by the team and can no longer be edited." +- Auth: session bearer via `Authorization: Bearer ` (same as + every shipped account-management command). All member roles carry + `renderReport: create + read`. Non-member org → 404. +- Side effects on create: opens a Crisp conversation tied to the reporter's + email + Slack ping (soft-fail, never blocks filing). +- List exists (`GET`, paginated `{renderReports, page, totalPages}`) and + `respond` exists (`POST …/{report}/respond`) — both out of CLI scope. + +Live wire shapes captured 2026-09-11 (use as fixture sources): + +- Sync success: body has NO render id; the id is the + `x-urlbox-request-id` response header (e.g. `01a0906a-…_ps`), which + resolves on `GET /v1/render/{id}` to `{"renderId": …, "status": + "succeeded", "renderUrl": …}`. +- Sync failure (HTTP 400): no header; body carries the id: + `{"error":{"message":"Invalid URL","code":"InvalidURLError"}, + "requestId":"01a09082-…_ps"}`. The failed render exists server-side + (`status: "failed"` with `reason`), so it is reportable. +- Async accepted: body `{"status":"created","renderId":"…_pa", + "statusUrl":"https://api.urlbox.com/v1/render/…"}`. +- Unauthenticated `/v2` error: `{"defined":false,"code":"UNAUTHORIZED", + "status":401,"message":"Authentication required"}`. + +## Piece 1 — the hint line + +After any render-family command (`render`, `screenshot`, `pdf`, `video`) +receives an API response containing a render id, print one muted line to +stderr under the result/error: + +``` +✓ Rendered: https://renders.urlbox.com/…png + Something wrong? urlbox report 01a0906a-…_ps +``` + +Wording is a placeholder; final copy is Arnold's, set at implementation +review before merge. + +Where the id comes from, per response type (in-memory only, never persisted, +never added to envelopes): + +| Response | Id source | +|---|---| +| Sync success | `x-urlbox-request-id` response header | +| Sync failure | `requestId` field in the error body | +| Async accepted | `renderId` in the body | +| Async submit failure | `requestId` in the error body, when present | + +Gates — the hint prints only when ALL hold: + +- resolved output format is `text` (never json/quiet) +- stderr is a TTY (`isStderrTTY`, same gate as the root banner) +- the response yielded a render id (auth/network/timeout failures without a + `requestId` print nothing extra) + +Never on `--dry-run`/`--curl` (no API call → no response), and NOT on +`urlbox status` (the render-time hint already covered that id). + +The HTTP client must expose the response header + parsed error body to the +render command for this; that plumbing stays internal to the client/command +(`internal/api/http_client.go`, `internal/cmd/render.go`). + +## Piece 2 — `urlbox report ` + +Session command modelled on the account-management commands: guard = +`loadSession` + `requireActiveOrg`; call = `SessionClient.PostJSON` to +`/v2/organisation/{active_org}/render-reports` with +`{renderId, category, comment}`. Standard session retry flags +(`--max-retries`/`--no-retry`) attach; safe because create is an upsert. + +Surface: + +``` +urlbox report [--category ] [--comment ] +``` + +- `--category` — one of the six API values verbatim; validated client-side, + invalid value → usage error listing the valid set. +- `--comment` — 1–2000 chars; validated client-side. +- Both flags present → no prompts (agent path). +- TTY + missing flag(s) → interactive: `prompt.SelectOne` over the six + categories (human-readable labels, API values on the wire), then a + one-line comment input (`huh` input, required non-empty). Interactive shows + the renderId being reported before filing. +- Non-TTY + missing flag(s) → `ErrUsage` naming the missing flag; never + hangs (`prompt.ErrNotInteractive` path). + +Output (house rules — mutation with named summary, no view): + +- text: `✓ Report filed for render ` (exact copy Arnold's) +- json: success envelope, `data` = the API report DTO verbatim +- quiet: the report public id (`rpt_…`) via `writeEnvelopeWithQuietData` +- no breadcrumb (no follow-up CLI command exists in this scope) + +Error mapping (closed set): + +| API | CLI code | Exit | Hint | +|---|---|---|---| +| 401 | `auth` | 3 | unified login hint (`loginHint`) | +| 404 org or render | `not_found` | 5 | render not found — expired (~60 days) or not from this org | +| 409 locked report | `conflict` | 7 | surface the API message (report is with the team) | +| 400/422 validation | `validation` | 2 | from API body | +| 429 / 5xx | `rate_limit` / `server` | 6 / 10 | standard mapping | + +## Piece 3 — `urlbox support` + +Clone of the `dashboard` command shape (`internal/cmd/dashboard.go` + +`internal/browser.Opener`), target `https://urlbox.com/contact`: + +- text + TTY, not headless → open the browser; opener failure → `ErrServer` + with the URL in the hint +- headless → print `Support URL: … (open in any browser)` to stderr, still + emit the success envelope +- json/quiet → envelope only, never launches a browser +- no auth required + +## Testing & delivery + +- TDD failing-first per repo rules; `make ci` + `make surface-snapshot` per + task; SURFACE.txt committed with the code (new entries: `report` + its + flags, `support`). +- Fixtures are production-shape only, pinned from the live wire shapes in + this spec (captured 2026-09-11) and the origin/main zod contract. No + invented shapes. +- Prompt injection for tests follows the house `pickFunc` / + `Set…ForTest` pattern; TTY overrides via `stderrTTYOverride` / + `stdinTTYOverride`. +- `skills/SKILL.md` (agent-relevant command) and README updated. +- Live verification at Arnold's manual gate: needs a fresh `urlbox login` + (his local session token is expired). Positive create against production + files a real report (Crisp conversation + Slack ping) — do it once, + deliberately, at the gate. +- Mono fumadocs + command reference ride a later effort, page-by-page with + Arnold. +- Commits per repo rules: propose diff + message, wait for approval; author + `Arnold Cubici-Jones <108676317+AJCJ1@users.noreply.github.com>`; squash + to one before PR; never push without Arnold's word. diff --git a/internal/api/download_client.go b/internal/api/download_client.go index d5304e5..0382b5f 100644 --- a/internal/api/download_client.go +++ b/internal/api/download_client.go @@ -1,4 +1,4 @@ -// internal/api/download_client.go — v1.0.4 Class 2.1. +// internal/api/download_client.go // // Hardened HTTP client for binary render-output fetches. Separate from // the JSON-API client (NewHTTPClient) because: diff --git a/internal/api/download_client_test.go b/internal/api/download_client_test.go index aacae80..1792718 100644 --- a/internal/api/download_client_test.go +++ b/internal/api/download_client_test.go @@ -1,4 +1,4 @@ -// internal/api/download_client_test.go — v1.0.4 Class 2.1. +// internal/api/download_client_test.go // // Pins the hardened render-download HTTP client contract: TLS 1.2 min, // no non-http(s) redirects, no HTTPS→HTTP downgrades, no unbounded diff --git a/internal/api/http_client.go b/internal/api/http_client.go index b847370..8f9e6cf 100644 --- a/internal/api/http_client.go +++ b/internal/api/http_client.go @@ -17,8 +17,7 @@ import ( "github.com/urlbox/urlbox-cli/internal/version" ) -// Endpoint paths locked from urlbox-mono spec -// (apps/api/src/modules/render/render.routes.ts). Exported so callers like +// Endpoint paths locked to the Urlbox render API. Exported so callers like // `urlbox render --curl` can reference the same paths the HTTPClient uses. const ( // PathSync is the synchronous render endpoint. @@ -131,7 +130,11 @@ func (c *HTTPClient) do(ctx context.Context, method, path string, body any) (*Re } if resp.StatusCode >= 400 { - return nil, mapStatusToCLIError(resp, respBody) + cliErr := mapStatusToCLIError(resp, respBody) + if rid := extractRequestID(respBody); rid != "" { + return nil, &RenderIDError{Err: cliErr, RenderID: rid} + } + return nil, cliErr } // Success path: the API returns the response body directly (no envelope @@ -143,10 +146,10 @@ func (c *HTTPClient) do(ctx context.Context, method, path string, body any) (*Re } } - // If the API surfaced the upstream HTTP status (under data.response per - // urlbox-mono apps/api/src/lib/utils.ts:86-122), promote it to top-level - // agent-friendly fields. statusCodeInitial captures the first request's - // code before redirects — a 401→302→200 chain must mark upstreamOk=false. + // If the API surfaced the upstream HTTP status (under data.response), + // promote it to top-level agent-friendly fields. statusCodeInitial + // captures the first request's code before redirects — a 401→302→200 + // chain must mark upstreamOk=false. if respObj, ok := data["response"].(map[string]any); ok { if status, ok := respObj["statusCode"].(float64); ok { data["upstreamStatus"] = status @@ -168,7 +171,7 @@ func (c *HTTPClient) do(ctx context.Context, method, path string, body any) (*Re } } - return &Response{OK: true, Data: data}, nil + return &Response{OK: true, Data: data, RenderID: resp.Header.Get("x-urlbox-request-id")}, nil } // mapStatusToCLIError maps a non-2xx response to a typed *output.CLIError. @@ -265,11 +268,10 @@ func isAuthErrorCode(code string) bool { // option-validation failure. These come back as 4xx but should map to // ErrValidation (not the generic ErrUsage default) so the v0.9.0 schema-as-docs // contract is honored: --json passes through, and when the API rejects, the -// CLI surfaces a validation envelope. Mirrors the ClientError subclasses in -// urlbox-mono apps/api/src/lib/errors.ts (ValidateRequestErrors namespace -// + a few peers used at the same layer). +// CLI surfaces a validation envelope. Mirrors the API's request-validation +// error codes. // -// Round 5 First-2: target-URL rejection codes (InvalidURLError) also +// Target-URL rejection codes (InvalidURLError) also // land here. The user passed a syntactically-valid URL the API couldn't // reach — ErrValidation ("your input was rejected") reads more // accurately than ErrUsage ("you misused the CLI") for retry-logic and @@ -330,3 +332,27 @@ func extractAPIError(body []byte) (msg, code string) { } return trimmed, code } + +// extractRequestID reads the requestId field Urlbox error bodies carry +// alongside the error object. Returns "" for non-JSON or absent field. +func extractRequestID(body []byte) string { + var parsed map[string]any + if err := json.Unmarshal(body, &parsed); err != nil { + return "" + } + rid, _ := parsed["requestId"].(string) + return rid +} + +// RenderIDError decorates a CLIError from a failed render call with the +// render id the error body carried, so the render command can offer +// `urlbox report ` for the failed render. Unwrap keeps errors.As +// resolution to *output.CLIError intact everywhere else. +type RenderIDError struct { + Err *output.CLIError + RenderID string +} + +func (e *RenderIDError) Error() string { return e.Err.Error() } + +func (e *RenderIDError) Unwrap() error { return e.Err } diff --git a/internal/api/http_client_test.go b/internal/api/http_client_test.go index 990921d..37a6504 100644 --- a/internal/api/http_client_test.go +++ b/internal/api/http_client_test.go @@ -327,8 +327,7 @@ func TestHTTPClient_Render_400_ApiKeyNotFound_MapsToAuth(t *testing.T) { // When the API response includes data.response.statusCode, propagate it // through Response.Data so the render command can surface it in the -// envelope. The Urlbox API nests upstream status under "response" (see -// urlbox-mono apps/api/src/lib/utils.ts:86-122). +// envelope. The Urlbox API nests upstream status under "response". func TestHTTPClient_Render_UpstreamStatus_Propagates(t *testing.T) { m := apitest.New(apitest.SuccessJSON(`{ "renderUrl": "https://renders.urlbox.com/x.png", @@ -451,8 +450,8 @@ func TestHTTPClient_Render_NonJSONErrorBody_FallsBackToBodyString(t *testing.T) } } -// TestHTTPClient_Render_InvalidURLError_MapsToValidation pins Round 5 -// First-2: when the API returns HTTP 400 with apiCode="InvalidURLError" +// TestHTTPClient_Render_InvalidURLError_MapsToValidation pins a regression: +// when the API returns HTTP 400 with apiCode="InvalidURLError" // (typical for unreachable target URLs like https://nonexistent.invalid), // the CLI used to map it to ErrUsage (exit 1) — implying the user // misused the CLI. But the user passed a syntactically-valid URL; the @@ -472,7 +471,7 @@ func TestHTTPClient_Render_InvalidURLError_MapsToValidation(t *testing.T) { t.Fatalf("err=%v, want *output.CLIError", err) } if cli.Code != output.ErrValidation { - t.Errorf("Code=%q, want %q (Round 5 First-2)", cli.Code, output.ErrValidation) + t.Errorf("Code=%q, want %q", cli.Code, output.ErrValidation) } if !strings.Contains(cli.Message, "Invalid URL") { t.Errorf("Message=%q should surface the API's text", cli.Message) diff --git a/internal/api/render_id_test.go b/internal/api/render_id_test.go new file mode 100644 index 0000000..618e962 --- /dev/null +++ b/internal/api/render_id_test.go @@ -0,0 +1,94 @@ +package api_test + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/urlbox/urlbox-cli/internal/api" + "github.com/urlbox/urlbox-cli/internal/api/apitest" + "github.com/urlbox/urlbox-cli/internal/output" +) + +func TestRender_SyncSuccess_CapturesRenderIDHeader(t *testing.T) { + srv := apitest.New(apitest.ScriptedResponse{ + Status: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Urlbox-Request-Id": []string{"01a0906a-fff5-7565-8dc5-02868f11a4fa_ps"}, + }, + Body: `{"renderUrl":"https://renders.urlbox.com/x.png","size":17752}`, + }) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + resp, err := c.Render(context.Background(), map[string]any{"url": "https://example.com"}) + if err != nil { + t.Fatalf("Render: %v", err) + } + if resp.RenderID != "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps" { + t.Fatalf("RenderID=%q", resp.RenderID) + } + if _, ok := resp.Data["renderId"]; ok { + t.Fatalf("renderId must NOT be injected into Data: %v", resp.Data) + } +} + +func TestRender_SyncSuccess_NoHeader_RenderIDEmpty(t *testing.T) { + srv := apitest.New(apitest.SuccessJSON(`{"renderUrl":"https://renders.urlbox.com/x.png","size":1}`)) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + resp, err := c.Render(context.Background(), map[string]any{"url": "https://example.com"}) + if err != nil { + t.Fatalf("Render: %v", err) + } + if resp.RenderID != "" { + t.Fatalf("RenderID=%q, want empty", resp.RenderID) + } +} + +func TestRender_Failure_WrapsRenderIDError(t *testing.T) { + srv := apitest.New(apitest.ScriptedResponse{ + Status: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"}`, + }) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + c.Retry = api.NoRetryConfig() + _, err := c.Render(context.Background(), map[string]any{"url": "https://x.invalid"}) + var ridErr *api.RenderIDError + if !errors.As(err, &ridErr) { + t.Fatalf("want RenderIDError, got %T: %v", err, err) + } + if ridErr.RenderID != "01a09082-25cf-74bd-9188-022c4f5bf25f_ps" { + t.Fatalf("RenderID=%q", ridErr.RenderID) + } + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("errors.As must still reach the CLIError through Unwrap") + } + if cliErr.Code != output.ErrValidation { + t.Fatalf("code=%q, want validation (InvalidURLError)", cliErr.Code) + } +} + +func TestRender_Failure_NoRequestID_PlainCLIError(t *testing.T) { + srv := apitest.New(apitest.ScriptedResponse{ + Status: http.StatusUnauthorized, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid token","code":"ApiKeyInvalid"}}`, + }) + t.Cleanup(srv.Close) + c := api.NewHTTPClient(srv.URL(), "pk", "sk") + c.Retry = api.NoRetryConfig() + _, err := c.Render(context.Background(), map[string]any{"url": "https://example.com"}) + var ridErr *api.RenderIDError + if errors.As(err, &ridErr) { + t.Fatalf("no requestId in body must yield a plain CLIError, got RenderIDError") + } + var cliErr *output.CLIError + if !errors.As(err, &cliErr) || cliErr.Code != output.ErrAuth { + t.Fatalf("want auth CLIError, got %v", err) + } +} diff --git a/internal/api/smoke_test.go b/internal/api/smoke_test.go index c726296..9ead4cf 100644 --- a/internal/api/smoke_test.go +++ b/internal/api/smoke_test.go @@ -197,11 +197,11 @@ func TestSmoke_v090_PassthroughTotallyMadeUp(t *testing.T) { // - message is non-empty and signals options-level rejection // - hint is non-empty and points the agent at next steps // -// What we CAN'T verify here (deferred to a follow-up urlbox-mono PR): -// the API's response body does not include `info.errors` (the Zod tree -// with field names) — only the generic "Invalid options, please check -// errors" message + the `InvalidOptions` code. Field-level detail would -// require an API change to add `info.errors` to the wire response. +// What we CAN'T verify here (would need an API-side change): +// the API's response body does not include field-level validation detail +// — only the generic "Invalid options, please check errors" message + the +// `InvalidOptions` code. Surfacing per-field errors would require the API +// to add them to the wire response. func TestSmoke_v090_KnownBadType_APIReturnsMeaningfulError(t *testing.T) { c := smokeClient(t) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) diff --git a/internal/api/types.go b/internal/api/types.go index d0544ae..af9f614 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -40,6 +40,11 @@ type Response struct { Error string `json:"error,omitempty"` Code string `json:"code,omitempty"` Hint string `json:"hint,omitempty"` + // RenderID is the render id the API surfaced OUTSIDE the body — the + // x-urlbox-request-id response header. Sync render bodies carry no id, + // so this is the only place a sync render's id exists. In-memory only: + // json:"-" keeps envelopes unchanged. + RenderID string `json:"-"` } // Breadcrumb is a "next step" hint emitted alongside a successful Response. diff --git a/internal/api/types_test.go b/internal/api/types_test.go index 315dcfd..536fda4 100644 --- a/internal/api/types_test.go +++ b/internal/api/types_test.go @@ -11,7 +11,7 @@ import ( func TestResponse_JSON_RoundTrip(t *testing.T) { // `data` payload uses camelCase to match the locked Urlbox API wire - // format (renderUrl, renderId — see urlbox-mono apps/api). + // format (renderUrl, renderId). src := `{ "ok": true, "command": "render", diff --git a/internal/browser/opener.go b/internal/browser/opener.go index 54e0927..c9eaa25 100644 --- a/internal/browser/opener.go +++ b/internal/browser/opener.go @@ -23,7 +23,7 @@ var ErrUnopenableURL = errors.New("refused to open URL: scheme must be http or h // via a hostile api_host that returned an arbitrary renderUrl) and // the OS handler would happily launch executables for some of them. // -// v1.0.4 Class 2.2: pre-1.0.4 Open() forwarded any string, turning +// Pre-1.0.4 Open() forwarded any string, turning // --open into a remote-launch primitive on Windows (cmd /c start // happily resolves UNC paths and file: schemes to executables). func isOpenableURL(raw string) bool { diff --git a/internal/browser/opener_test.go b/internal/browser/opener_test.go index 2e0232c..2fdcc19 100644 --- a/internal/browser/opener_test.go +++ b/internal/browser/opener_test.go @@ -36,14 +36,14 @@ func TestNoopOpener_DoesNothing(t *testing.T) { } } -// ─── Class 2.2 (v1.0.4) ─── scheme check before OS exec ──────────── +// ─── scheme check before OS exec ──────────── // // Invariant: any URL handed to the OS URL handler is scheme-checked // (http or https only) before exec.Command. Pre-v1.0.4 Open() forwarded // any string. With a hostile api_host returning an attacker-controlled // renderUrl, the Windows `cmd /c start "" ` path would launch // UNC paths and file:// schemes — turning --open into a remote-launch -// primitive. Defense-in-depth: even if Class 1 (validation) closes +// primitive. Defense-in-depth: even if input validation closes // every overlay/profile path today, this stops a future regression // from re-arming the primitive. diff --git a/internal/cmd/auth_preflight.go b/internal/cmd/auth_preflight.go index 5c1f3b1..55daff0 100644 --- a/internal/cmd/auth_preflight.go +++ b/internal/cmd/auth_preflight.go @@ -1,5 +1,3 @@ -// internal/cmd/auth_preflight.go — v1.0.4 Class 5. -// // Client-side pre-flight checks so predictable failures fail fast with // the CLI's own vocabulary, not the API's. Pre-1.0.4 a missing-secret // `urlbox render ` returned the API's confusing diff --git a/internal/cmd/commands.go b/internal/cmd/commands.go index 7a46d8c..c95865f 100644 --- a/internal/cmd/commands.go +++ b/internal/cmd/commands.go @@ -103,10 +103,10 @@ func buildCommandInfo(cmd *cobra.Command) CommandInfo { info.Flags = append(info.Flags, buildFlagInfo(f)) }) - // Round 8 MM: recurse into sub-subcommands so agents reading + // Recurse into sub-subcommands so agents reading // `urlbox commands --output-format json` see `config get`, // `config profile create`, etc. — not just the top-level - // parent names. Adv-3 M1 flagged the drift between `commands` + // parent names. There was drift between `commands` // and `surface`: surface listed all 263 invocations; commands // listed 14 top-level only. for _, c := range cmd.Commands() { diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 650c45f..75257a9 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -21,7 +21,7 @@ var supportedConfigKeys = []string{"api_key", "api_secret", "api_host", "default // profileNameRE pins the allowed shape of a profile name: must start with // an alphanumeric, then 0–63 more alphanumerics / underscore / hyphen, -// totalling 1–64 chars. Round 5 Adv-4: profile names previously accepted +// totalling 1–64 chars. Profile names previously accepted // path separators (`/`, `..`), control chars (\n, \r, \t), null bytes // (silent truncation collisions: a\0b vs a), leading whitespace or dots, // and arbitrarily long strings. All footguns in their own way. @@ -98,8 +98,8 @@ func newProfileCreateCmd() *cobra.Command { if cliErr != nil { return cliErr } - // Validate the resolved secret value when one was provided. Round 6 - // class-fix: profile create must go through the same gate every + // Validate the resolved secret value when one was provided — + // profile create must go through the same gate every // secret-writing path uses. An empty resolvedSecret here means // no flag was passed — that's allowed for profile create (the // profile can be created secretless and have the secret added @@ -111,7 +111,7 @@ func newProfileCreateCmd() *cobra.Command { } resolvedSecret = validated } - // Round 8 GG: same gate for api_host as for api_secret — + // Same gate for api_host as for api_secret — // validates scheme, rejects embedded creds + CRLF + control // chars before persisting. if apiHost != "" { @@ -121,8 +121,8 @@ func newProfileCreateCmd() *cobra.Command { } apiHost = validated } - // Atomic check + create under the config-file lock (Round 7 CC - // class-fix): the previous Load -> check -> Save sequence raced + // Atomic check + create under the config-file lock — the + // previous Load -> check -> Save sequence raced // when parallel `config profile create` calls hit the same // XDG_CONFIG_HOME — 20 parallel calls used to lose 5-6. if err := config.Update(func(cfg *config.Config) error { @@ -185,7 +185,7 @@ func newProfileListCmd() *cobra.Command { "name": n, "api_host": p.APIHost, "masked_secret": maskSecret(p.APISecret), - // Round 8 MM: was string("true"/"false") because the + // Was string("true"/"false") because the // row was typed map[string]string. Now bool so JSON // consumers (agents) can use it directly without // string-comparison. @@ -312,7 +312,7 @@ func newConfigGetCmd() *cobra.Command { Long: `Read a config value from the resolved profile. For api_secret and session_token, the raw value is masked by default -(Round 1 UX I1) to avoid leaking into scrollback / clipboard / log +to avoid leaking into scrollback / clipboard / log capture. Pass --reveal to print the unmasked value (intended for clipboard-copy workflows with eyes on the screen).`, Args: cobra.ExactArgs(1), @@ -383,7 +383,7 @@ profile count.`, } // Validate api_secret / session_token values through the same gate // every secret-writing path uses. Rejects empty / whitespace / - // control chars. Round 6 class-fix. + // control chars. if key == "api_secret" || key == "session_token" { validated, vErr := config.ValidateSecretValue(val) if vErr != nil { @@ -391,7 +391,7 @@ profile count.`, } val = validated } - // Round 8 GG: api_host gets the same treatment — was accepting + // api_host gets the same treatment — was accepting // javascript:, file://, embedded credentials, CRLF before. if key == "api_host" { validated, vErr := config.ValidateAPIHost(val) @@ -439,7 +439,7 @@ profile count.`, // Per-profile key (api_key / api_secret / api_host). Profile // resolution + overwrite-guard + write all happen under one - // Update so the read-modify-write window is atomic. Round 7 CC. + // Update so the read-modify-write window is atomic. var profileName string if err := config.Update(func(c *config.Config) error { name, perr := resolveTargetProfile(cmd, c) @@ -466,7 +466,7 @@ profile count.`, } return output.NewCLIError(output.ErrServer, "failed to write config", err.Error()) } - // Round 4 M4: mirror the masking that `config get api_secret` + // Mirror the masking that `config get api_secret` // already does — never echo a freshly-set secret back through // the envelope (CI logs, scrollback, --output-format quiet pipes). // The raw value is still persisted on disk; only the @@ -493,13 +493,13 @@ profile count.`, // // Precedence (highest first): // - 0 profiles → ErrUsage "No profiles configured" (setup issue) -// - --profile given → must exist, else ErrNotFound (Round 7 EE: every +// - --profile given → must exist, else ErrNotFound (every // "user named a profile that doesn't exist" site now reports the same // envelope as profile delete/default and the unified config.Resolve) // - URLBOX_PROFILE set → must exist, else ErrNotFound (same class) // - 1 profile, no flag/env → that profile (implicit) -// - 2+ profiles, default_profile set and exists → default_profile (Round 5 -// CI-2: matches how render/status/link resolve) +// - 2+ profiles, default_profile set and exists → default_profile +// (matches how render/status/link resolve) // - 2+ profiles, no default_profile → ErrUsage "--profile is required" // (ambiguity — user didn't specify which, not a name lookup miss) func resolveTargetProfile(cmd *cobra.Command, c *config.Config) (string, error) { @@ -537,7 +537,7 @@ func resolveTargetProfile(cmd *cobra.Command, c *config.Config) (string, error) } } // 2+ profiles, no flag, no env: fall back to default_profile if set. - // Round 5 CI-2 — config get/set used to require --profile here, but + // Config get/set used to require --profile here, but // render/status/link already resolved default_profile transparently, // breaking CI scripts that ran `config set api_key X` after creating // a second profile. diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index 195696b..0637328 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -82,8 +82,8 @@ func TestConfigSet_Get_RoundTrip_SingleProfile(t *testing.T) { stdout.Reset() stderr.Reset() - // --reveal: round-trip verifies raw value identity (default is now masked - // per UX I1; the masking behavior gets its own dedicated tests). + // --reveal: round-trip verifies raw value identity (default is now masked; + // the masking behavior gets its own dedicated tests). if exit := cmd.Execute([]string{"config", "get", "api_secret", "--reveal", "--output-format", "quiet"}, &stdout, &stderr); exit != 0 { t.Fatalf("get: exit=%d", exit) } @@ -138,8 +138,8 @@ func TestConfigSet_NoProfiles_Errors(t *testing.T) { } // Historical TestConfigSet_MultipleProfiles_NoFlag_Errors asserted that -// 2+ profiles always errored without --profile. Round 5 CI-2 changed -// that to honor default_profile transparently (matching how render / +// 2+ profiles always errored without --profile. That changed +// to honor default_profile transparently (matching how render / // status / link resolve). Coverage moved to: // - TestConfigSet_MultipleProfiles_UsesDefaultProfile (positive) // - TestConfigSet_MultipleProfiles_NoDefaultProfile_Errors (fallback) @@ -188,7 +188,7 @@ func TestConfigSet_DefaultProfileKey_AlwaysWrites(t *testing.T) { } } -// TestConfigSet_DefaultProfile_UnknownName_Errors pins Round 7 EE class-fix: +// TestConfigSet_DefaultProfile_UnknownName_Errors pins a regression: // naming a non-existent profile (here `ghost`) returns ErrNotFound exit 5, // not the legacy ErrUsage exit 1. This aligns with `profile default`/ // `profile delete` and with the unified config.Resolve for render/status/ @@ -454,7 +454,7 @@ func must(t *testing.T, err error) { } } -// TestConfigGet_APISecret_MaskedByDefault pins UX I1: `config get api_secret` +// TestConfigGet_APISecret_MaskedByDefault pins the masking rule: `config get api_secret` // masks the raw value to prevent accidental scrollback / log / clipboard // leakage. `config profile list` already masks; `config get api_secret` // was the outlier. @@ -556,9 +556,9 @@ func TestConfigGet_APIKey_NotMasked(t *testing.T) { } } -// TestConfigSet_APISecret_MaskedInEnvelope pins Round 4 M4: `config set +// TestConfigSet_APISecret_MaskedInEnvelope pins a regression: `config set // api_secret ` echoed the raw secret back in .data.value and in -// the summary string. UX I1 (Round 1) only fixed `config get`. The set +// the summary string. An earlier fix only covered `config get`. The set // path still leaked into CI logs and terminal scrollback. func TestConfigSet_APISecret_MaskedInEnvelope(t *testing.T) { dir := t.TempDir() @@ -613,11 +613,11 @@ func TestConfigSet_APIKey_NotMasked(t *testing.T) { } } -// TestConfigGet_UnknownFlagProfile_Errors pins Round 7 EE class-fix: +// TestConfigGet_UnknownFlagProfile_Errors pins a regression: // `config get --profile X` where X doesn't exist returns ErrNotFound // exit 5 with command="config get", matching profile delete/default + // config.Resolve (used by render/status/link/doctor). Pre-fix this -// returned ErrUsage exit 1 with command="" — the original Round 7 +// returned ErrUsage exit 1 with command="" — the original // finding ("envelope shape misalignment"). func TestConfigGet_UnknownFlagProfile_Errors(t *testing.T) { dir := t.TempDir() @@ -680,10 +680,10 @@ func TestConfigSet_UnknownFlagProfile_Errors(t *testing.T) { } } -// TestConfigGet_UnknownEnvProfile_Errors pins Round 5 Adv-2 + -// Round 7 EE: a typo in URLBOX_PROFILE silently fell back to the -// default profile, leaking the default's secret. Round 5 closed the -// silent-fallback (correctly errored with ErrUsage). Round 7 EE +// TestConfigGet_UnknownEnvProfile_Errors pins two fixes: +// a typo in URLBOX_PROFILE silently fell back to the +// default profile, leaking the default's secret. The first fix closed the +// silent-fallback (correctly errored with ErrUsage); the second // aligns the envelope to ErrNotFound exit 5 with command="config get" // — same shape as profile delete/default and the unified // config.Resolve. The class is "user named a profile that doesn't @@ -750,7 +750,7 @@ func TestConfigGet_ValidEnvProfile_TargetsThatProfile(t *testing.T) { } } -// TestConfigProfileCreate_RejectsDangerousNames pins Round 5 Adv-4: +// TestConfigProfileCreate_RejectsDangerousNames pins a regression: // profile names could contain path separators, control chars, and null // bytes. Null-byte truncation in particular is a footgun — "a\x00b" // silently collides with "a" because the JSON store keys by the @@ -817,7 +817,7 @@ func TestConfigProfileCreate_AcceptsSafeNames(t *testing.T) { } } -// TestConfigSet_MultipleProfiles_UsesDefaultProfile pins Round 5 CI-2: +// TestConfigSet_MultipleProfiles_UsesDefaultProfile pins the default-profile rule: // when 2+ profiles exist AND default_profile is set, config get/set // should target the default rather than require --profile. Previously // a CI script that ran `config set api_key X` after creating a second @@ -905,7 +905,7 @@ func TestConfigSet_MultipleProfiles_NoDefaultProfile_Errors(t *testing.T) { } } -// TestConfigSet_APISecret_RejectsBadValues pins Round 6 Adv-4 + Adv-5: +// TestConfigSet_APISecret_RejectsBadValues pins a regression: // config set api_secret silently accepted whitespace-only and empty // strings, AND its empty-string path silently cleared the saved secret // (bypassing the auth overwrite guard entirely). Now: validateSecretValue @@ -1041,7 +1041,7 @@ func TestConfigProfileCreate_RejectsBadSecretValues(t *testing.T) { } } -// TestConfigProfileList_IsDefault_IsBool pins Round 8 MM: is_default +// TestConfigProfileList_IsDefault_IsBool pins a regression: is_default // used to be string "true"/"false" because the row was typed // map[string]string. JSON consumers had to string-compare instead of // branching on a bool. Now bool. diff --git a/internal/cmd/dashboard.go b/internal/cmd/dashboard.go index b275b86..1033b56 100644 --- a/internal/cmd/dashboard.go +++ b/internal/cmd/dashboard.go @@ -86,9 +86,9 @@ Exit codes: func runDashboard(c *cobra.Command) error { data := map[string]any{"url": dashboardURL} - // Round 8 MM: when the user asks for structured output (json/quiet), + // When the user asks for structured output (json/quiet), // they're scripting around the URL — don't fire a browser side-effect - // in addition to printing the envelope. Adv-4 M4: agents calling + // in addition to printing the envelope. Agents calling // `dashboard --output-format json` got an unexpected browser tab. formatFlag, _ := c.Root().PersistentFlags().GetString("output-format") resolvedFormat := output.ResolveFormat(formatFlag, c.OutOrStdout()) diff --git a/internal/cmd/dashboard_test.go b/internal/cmd/dashboard_test.go index 3740aa8..f05696f 100644 --- a/internal/cmd/dashboard_test.go +++ b/internal/cmd/dashboard_test.go @@ -40,8 +40,8 @@ func TestDashboard_OpensBrowser_WhenTextFormat(t *testing.T) { } } -// TestDashboard_JSONFormat_NoBrowserSideEffect pins Round 8 MM / -// Adv-4 M4: when the user asks for json (or quiet), they're scripting +// TestDashboard_JSONFormat_NoBrowserSideEffect pins a regression: +// when the user asks for json (or quiet), they're scripting // around the URL — don't launch a browser tab as a side effect. The // envelope still carries the URL. func TestDashboard_JSONFormat_NoBrowserSideEffect(t *testing.T) { @@ -81,7 +81,7 @@ func TestDashboard_HeadlessFallback_PrintsURL(t *testing.T) { t.Cleanup(cmd.ResetDashboardOpenerForTest) // Use text format so we hit the headless code path (json path now - // short-circuits before reaching the headless check — Round 8 MM). + // short-circuits before reaching the headless check). var stdout, stderr bytes.Buffer exit := cmd.Execute([]string{"dashboard", "--output-format", "text"}, &stdout, &stderr) if exit != 0 { @@ -103,7 +103,7 @@ func TestDashboard_OpenError_ServerExit(t *testing.T) { t.Cleanup(cmd.ResetHeadlessDetectorForTest) // Use text format so we exercise the actual opener path — json - // mode never calls the opener now (Round 8 MM). + // mode never calls the opener now. var stdout, stderr bytes.Buffer exit := cmd.Execute([]string{"dashboard", "--output-format", "text"}, &stdout, &stderr) if exit != 10 { diff --git a/internal/cmd/doctor.go b/internal/cmd/doctor.go index cd255cd..304ff5f 100644 --- a/internal/cmd/doctor.go +++ b/internal/cmd/doctor.go @@ -23,7 +23,7 @@ import ( // httpTimeout caps each individual HTTP check (api_reachable and the // render_credential live probe). // Set to 10s rather than the original 5s to absorb cold-container -// startup costs — Round 5 CI-1 reproed a false-fail on the first +// startup costs — a false-fail reproed on the first // invocation in a fresh container because DNS+TCP+TLS to api.urlbox.com // blew through the 5s budget even though warm-cache curl returned in // ~350ms. The outer doctor context is sized to fit all checks at this @@ -50,12 +50,12 @@ Exits non-zero if any check fails.`, RunE: func(cmd *cobra.Command, _ []string) error { // Sized to fit session + DNS + api_reachable + the // render_credential probe (each httpTimeout = 10s) plus a - // little headroom. Round 5 CI-1 bumped the per-check timeout + // little headroom. The per-check timeout was bumped // to absorb cold-start latency. ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) defer cancel() - // Round 6 Z class-fix: doctor previously called + // Doctor previously called // config.ResolveAPISecret() directly, which always looks at // the default profile and ignores --profile / URLBOX_PROFILE. // Now it goes through config.Resolve — the same path @@ -137,7 +137,7 @@ Exits non-zero if any check fails.`, case jqExpr != "": writeErr = output.WriteEnvelopeWithJQ(stdout, env, jqExpr, format == output.FormatQuiet) case format == output.FormatQuiet: - // Round 8 JJ: quiet mode used to print the whole checks + // Quiet mode used to print the whole checks // tree (broke the "single scalar" contract). Print the // overall status string instead — agents can pipe it. _, writeErr = fmt.Fprintln(stdout, overall) @@ -150,7 +150,7 @@ Exits non-zero if any check fails.`, } if anyFail { - // Round 8 JJ: pick the exit code based on which checks + // Pick the exit code based on which checks // failed, rather than always returning ErrServer (10). // The contract: // 3 (auth) — credential / api_secret problem diff --git a/internal/cmd/doctor_internal_test.go b/internal/cmd/doctor_internal_test.go index e6169b7..06ae17f 100644 --- a/internal/cmd/doctor_internal_test.go +++ b/internal/cmd/doctor_internal_test.go @@ -10,7 +10,7 @@ import ( "time" ) -// TestDoctor_HttpTimeout_AtLeast10s pins Round 5 CI-1: the per-check +// TestDoctor_HttpTimeout_AtLeast10s pins a regression: the per-check // HTTP timeout used to be 5s, which false-failed on cold-container // invocations where DNS+TCP+TLS to api.urlbox.com exceeded the budget // even though the warm-cache request returned in ~350ms. 10s is the @@ -18,7 +18,7 @@ import ( func TestDoctor_HttpTimeout_AtLeast10s(t *testing.T) { const minTimeout = 10 * time.Second if httpTimeout < minTimeout { - t.Errorf("httpTimeout = %v, want >= %v for cold-start DNS resilience (Round 5 CI-1)", httpTimeout, minTimeout) + t.Errorf("httpTimeout = %v, want >= %v for cold-start DNS resilience", httpTimeout, minTimeout) } } diff --git a/internal/cmd/doctor_test.go b/internal/cmd/doctor_test.go index 3acfcf6..b08a5b2 100644 --- a/internal/cmd/doctor_test.go +++ b/internal/cmd/doctor_test.go @@ -220,7 +220,7 @@ func TestDoctor_CredentialValid_PassesRenderCredential(t *testing.T) { } } -// TestDoctor_CredentialBadRequest_FailsRenderCredential pins Round 4 H2 +// TestDoctor_CredentialBadRequest_FailsRenderCredential pins a regression // through the fold: the probe previously only treated 401/403/5xx as // failure. A real-world 400 from /v1/user/me with body // {"error":{"code":"ApiKeyNotFound",...}} fell into the default arm and @@ -306,7 +306,7 @@ func TestDoctor_HasBreadcrumbs(t *testing.T) { } } -// TestDoctor_HonorsProfileFlag pins Round 6 class-fix: doctor used to +// TestDoctor_HonorsProfileFlag pins a regression: doctor used to // silently ignore --profile / URLBOX_PROFILE and always look at the // default profile's secret. Now: profile resolution is uniform with // every other command — unknown name errors, valid name targets that @@ -352,7 +352,7 @@ func TestDoctor_HonorsEnvProfile_UnknownErrors(t *testing.T) { } // TestDoctor_HonorsProfileFlag_ValidTargetsThatProfile pins the positive -// case for the Round 6 Z class-fix: --profile work makes doctor check +// case for the --profile fix: --profile work makes doctor check // the work profile's secret, not default's. func TestDoctor_HonorsProfileFlag_ValidTargetsThatProfile(t *testing.T) { // Use a httptest server so the auth check is hermetic. The handler @@ -394,7 +394,7 @@ func TestDoctor_HonorsProfileFlag_ValidTargetsThatProfile(t *testing.T) { } } -// TestDoctor_QuietMode_PrintsScalar pins Round 8 JJ: doctor's quiet +// TestDoctor_QuietMode_PrintsScalar pins a regression: doctor's quiet // mode used to dump the full JSON tree, violating the "quiet = single // useful scalar" contract. Now prints the overall status ("ok" or // "fail") on one line. @@ -414,7 +414,7 @@ func TestDoctor_QuietMode_PrintsScalar(t *testing.T) { } } -// TestDoctor_ExitCode_AuthFail pins Round 8 JJ: when only credential +// TestDoctor_ExitCode_AuthFail pins a regression: when only credential // checks fail (no api_secret), exit code should be 3 (auth), not 10 // (server). The contract maps exit 10 to upstream-server problems, // which is misleading when the actual issue is local config. diff --git a/internal/cmd/e2e_test.go b/internal/cmd/e2e_test.go index 245f57d..ac8496d 100644 --- a/internal/cmd/e2e_test.go +++ b/internal/cmd/e2e_test.go @@ -279,7 +279,7 @@ func TestE2E_UnknownCommand_ErrorEnvelope_JSON(t *testing.T) { } func TestE2E_UnknownCommand_ErrorEnvelope_Text(t *testing.T) { - // v1.0.4 Class 3.2: text-mode errors are human messages → stderr. + // Text-mode errors are human messages → stderr. // Pre-1.0.4 they went to stdout, violating the CLAUDE.md // "stdout for data, stderr for human messages" contract. stdout, stderr, exitCode := runCLI(t, "--output-format", "text", "nonexistent") diff --git a/internal/cmd/error_hints_test.go b/internal/cmd/error_hints_test.go index e87be27..9e2a180 100644 --- a/internal/cmd/error_hints_test.go +++ b/internal/cmd/error_hints_test.go @@ -101,7 +101,6 @@ var ghostCommandSubstrings = []string{ // TestNoGhostCommandsInHints walks production .go files and fails when // any line contains a substring naming a command/flag that doesn't exist. -// Caught Round 1 review C1/C2 (config.go:306,359 and render.go:487,520). func TestNoGhostCommandsInHints(t *testing.T) { root, err := repoRoot() if err != nil { diff --git a/internal/cmd/help_agent_test.go b/internal/cmd/help_agent_test.go index 741de4b..c949108 100644 --- a/internal/cmd/help_agent_test.go +++ b/internal/cmd/help_agent_test.go @@ -66,7 +66,7 @@ func TestHelpAgent_NotSet_FallsThroughToDefault(t *testing.T) { } } -// v1.0.4 Class 3.3 — --output-format json --help triggers agent help. +// --output-format json --help triggers agent help. // // Pre-1.0.4 only --agent --help produced JSON; --output-format json // --help silently fell through to plain text. Agents probing the diff --git a/internal/cmd/link.go b/internal/cmd/link.go index c9a8b55..d2d407c 100644 --- a/internal/cmd/link.go +++ b/internal/cmd/link.go @@ -72,7 +72,7 @@ If you actually want the rendered asset, use: } func runLink(cmd *cobra.Command, args []string, f *linkFlags) error { - // Round 5 First-1: accept a positional URL like `render` does so + // Accept a positional URL like `render` does so // `urlbox link https://example.com` is the obvious entry point. --url // still wins when both are present, matching render's precedence. if len(args) == 1 && f.urlFlag == "" { diff --git a/internal/cmd/link_test.go b/internal/cmd/link_test.go index 84d8b34..06efe56 100644 --- a/internal/cmd/link_test.go +++ b/internal/cmd/link_test.go @@ -493,7 +493,7 @@ func TestLink_BadJSON_ValidationError(t *testing.T) { } } -// TestLink_PositionalURL pins Round 5 First-1: link now accepts a +// TestLink_PositionalURL pins a regression: link now accepts a // positional URL like render does. Previously `urlbox link // https://example.com` errored with "unknown command", forcing the // user to discover --url. The inconsistency was confusing — render diff --git a/internal/cmd/overlay.go b/internal/cmd/overlay.go index a5c0e20..e28e2d8 100644 --- a/internal/cmd/overlay.go +++ b/internal/cmd/overlay.go @@ -1,10 +1,10 @@ -// internal/cmd/overlay.go — Round 8 Class C (HH): wires up the +// internal/cmd/overlay.go — wires up the // per-repo overlay loader. Before this commit, internal/config/repo.go // defined LoadRepoOverlay and internal/config/resolve.go handled the // "repo" precedence slot, but NO command actually called the loader. // README + SKILL.md + CHANGELOG all advertised `.urlbox/config.json` -// support, while in reality the file was silently ignored — Round 8 -// Adv-4 found this by setting an overlay and seeing render use the +// support, while in reality the file was silently ignored — caught +// by setting an overlay and seeing render use the // global default instead. package cmd diff --git a/internal/cmd/overlay_test.go b/internal/cmd/overlay_test.go index 261f7d6..8e8a283 100644 --- a/internal/cmd/overlay_test.go +++ b/internal/cmd/overlay_test.go @@ -11,7 +11,7 @@ import ( "github.com/urlbox/urlbox-cli/internal/cmd" ) -// TestOverlay_RenderDryRun_PicksUpOverlay pins Round 8 HH: before this +// TestOverlay_RenderDryRun_PicksUpOverlay pins a regression: before this // commit, `.urlbox/config.json` was advertised but never read. With // overlay loading wired in, the api_host from the overlay should // surface through the resolver (verified via `urlbox link` whose diff --git a/internal/cmd/render.go b/internal/cmd/render.go index 1403866..3dfcae7 100644 --- a/internal/cmd/render.go +++ b/internal/cmd/render.go @@ -169,7 +169,7 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { f.url = args[0] } - // Round 5 Adv-5: reject negative --timeout up front. Without this, + // Reject negative --timeout up front. Without this, // `--timeout -5s` produced the nonsense diagnostic "Render timed out // after -5s" — the per-attempt context immediately expired and the // timeout-error path interpolated the negative duration verbatim. @@ -196,7 +196,7 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { f.apiSecret = resolved } - // Round 4 M2: reject numeric flags outside the JSON safe-int range + // Reject numeric flags outside the JSON safe-int range // (±2^53-1). Go int64 accepts values JSON marshalling silently rounds // to a nearby float64, so the payload that reaches the API differs // from what the user typed. Catch it locally with a precise error. @@ -247,7 +247,7 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { applyFlagsToMap(cmd, f, merged) // 4. Require url somewhere — and require it to be a non-empty string. - // Round 5 Adv-7: --json '{"url":""}' and --json '{"url":null}' used to + // --json '{"url":""}' and --json '{"url":null}' used to // bypass this check because it only verified the key's presence. urlVal, urlPresent := merged["url"] missing := !urlPresent @@ -293,7 +293,7 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { if vErr != nil { return vErr } - // v1.0.4 Class 3.4 — warning routing per format: + // Warning routing per format: // - text mode: print "warning: ..." inline on stderr (humans // expect them near the success line; the text formatter does // not render envelope.warnings). @@ -318,8 +318,8 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { return cliErr } - // 5.6. Pre-flight --output sandbox + writability before any API call - // (Round 4 M1, M6). Without this, --dry-run silently passed paths + // 5.6. Pre-flight --output sandbox + writability before any API call. + // Without this, --dry-run silently passed paths // outside CWD and real renders burned a credit before discovering // the target was unwritable. Skipped for --async since --output is // not honored on the async path (the renderId is returned instead). @@ -336,7 +336,7 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { // 6. --dry-run short-circuits with the validated payload in the envelope. if f.dryRun { - // Round 5 Adv-3: flag the silent precedence when --dry-run wins + // Flag the silent precedence when --dry-run wins // over --curl / --output. Without this the user has no signal // that their --curl print / --output save wasn't performed. breadcrumbs := []output.Breadcrumb{ @@ -408,7 +408,7 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { cli.Hint = networkHint(errors.New(cli.Message), true, f.timeout) } } - return err + return appendReportHint(cmd, err) } // 9. --output: download the rendered file to a sandboxed local path. @@ -449,7 +449,9 @@ func runRender(cmd *cobra.Command, args []string, f *renderFlags) error { breadcrumbsForResp(resp, f), ) env.Warnings = warnings - return writeRenderEnvelope(cmd, env) + werr := writeRenderEnvelope(cmd, env) + printReportHint(cmd, renderIDForHint(resp)) + return werr } // breadcrumbsForResp returns the right next-step breadcrumbs based on what @@ -487,16 +489,16 @@ const jsonSafeIntMax = 9007199254740991 // they become float64. Then it converts the surviving numbers back to // float64 to preserve the existing map[string]any contract downstream. // -// Round 6 class-fix: walks the ENTIRE JSON tree (nested maps + arrays). +// Walks the ENTIRE JSON tree (nested maps + arrays). // The previous version only checked top-level keys width/height/delay/ // quality, leaving every other integer — including legitimate fields // like {"viewport":{"width":...}} — unchecked. The fundamental rule is // "no integer anywhere should silently round when marshaled to JSON", // independent of which key it sits under. func parseJSONWithIntRangeCheck(jsonBytes []byte) (map[string]any, *output.CLIError) { - // Round 8 LL: detect duplicate keys before main parse. Standard - // json.Unmarshal silently last-wins on dup keys, which Adv-2 found - // in `--json '{"url":"a","url":"b"}'` (signed b, no warning). Fail + // Detect duplicate keys before main parse. Standard + // json.Unmarshal silently last-wins on dup keys — + // e.g. `--json '{"url":"a","url":"b"}'` (signed b, no warning). Fail // fast so hand-edited JSON typos surface as errors. if cliErr := checkDuplicateJSONKeys(jsonBytes); cliErr != nil { return nil, cliErr @@ -667,10 +669,10 @@ func walkAndCheckInts(v any, path string) *output.CLIError { // they exceed the safe range — fractional/sci floats already // have float64 precision and the API validates value semantics. // - // Round 8 LL: the previous version called v.Int64() and treated + // The previous version called v.Int64() and treated // any error as "pass through". But Int64() errors on TWO cases: // fractional numbers AND integers > 2^63. The latter were silent- - // rounding to floats downstream (Adv-2 demo: width=99999999999999999999 + // rounding to floats downstream (e.g. width=99999999999999999999 // → query 1e+20). Now we inspect the literal string and reject // out-of-int64 integers explicitly. s := string(v) @@ -764,7 +766,6 @@ func convertNumberValue(v any) any { // validateIntFlagRanges rejects --width / --height / --delay / --quality // / --max-retries values outside ±2^53-1. The Go int64 flag accepts much // larger; we have to catch it here before json.Marshal silently rounds. -// Round 4 M2. func validateIntFlagRanges(f *renderFlags) *output.CLIError { checks := []struct { flag string @@ -939,7 +940,7 @@ func buildRenderClient(cmd *cobra.Command, f *renderFlags) (api.Client, *output. ) } - // v1.0.4 Class 5.1 — detect missing secret client-side with the + // Detect missing secret client-side with the // CLI's own vocabulary. Pre-1.0.4 we let the API return its // confusing "Api Key does not exist" message, costing a network // round-trip and a vocabulary mismatch (CLI says "API secret" @@ -992,8 +993,6 @@ func summariseRenderResp(resp *api.Response) string { // - sync + --output saved a file → the absolute saved path (savedTo) // - sync + no --output → the hosted renderUrl // - async → the renderId for follow-up `urlbox status` calls -// -// Round 5 First-3 / Power-1. func writeRenderEnvelope(cmd *cobra.Command, env *output.Envelope) error { formatFlag, _ := cmd.Root().PersistentFlags().GetString("output-format") jqExpr, _ := cmd.Root().PersistentFlags().GetString("jq") diff --git a/internal/cmd/render_output.go b/internal/cmd/render_output.go index b82e206..5c3439b 100644 --- a/internal/cmd/render_output.go +++ b/internal/cmd/render_output.go @@ -159,8 +159,8 @@ func canonicalizeExistingPrefix(path string) string { // // The download is bounded by api.DownloadTimeout even if the caller's // context has no deadline — a stalled CDN connection mustn't hang the -// CLI indefinitely. Body size is capped at api.DownloadMaxBytes (v1.0.4 -// Class 2.1 — pre-1.0.4 the body was unbounded, letting a misconfigured +// CLI indefinitely. Body size is capped at api.DownloadMaxBytes (pre-1.0.4 +// the body was unbounded, letting a misconfigured // or malicious renderUrl fill the disk). // checkOutputWritable verifies the user's --output path can be written to // BEFORE the API call, so a bad path doesn't burn a render credit. Mkdirs @@ -168,8 +168,7 @@ func canonicalizeExistingPrefix(path string) string { // a unique name so the user's actual --output target is untouched). // // Returns ErrValidation — the user owns the path; misclassifying this as -// ErrServer (as the pre-Round-4 download path did) blames the wrong party. -// Round 4 M6. +// ErrServer (as the old download path did) blames the wrong party. func checkOutputWritable(abs string) *output.CLIError { parent := filepath.Dir(abs) if err := os.MkdirAll(parent, 0o750); err != nil { diff --git a/internal/cmd/render_output_test.go b/internal/cmd/render_output_test.go index 8c8f104..2ff4ffb 100644 --- a/internal/cmd/render_output_test.go +++ b/internal/cmd/render_output_test.go @@ -28,7 +28,7 @@ func TestResolveOutputPath_RejectsEmpty(t *testing.T) { // TestResolveOutputPath_RejectsOutsideCWD_HintMentionsCdWorkaround pins // the rejection hint includes the `cd && urlbox render ...` escape -// hatch. Round 1 UX I4 — agents bouncing off /tmp/foo.png deserved to be +// hatch. Agents bouncing off /tmp/foo.png deserved to be // told the workaround inline, not just "pass a path under CWD". func TestResolveOutputPath_RejectsOutsideCWD_HintMentionsCdWorkaround(t *testing.T) { cwd := t.TempDir() diff --git a/internal/cmd/render_test.go b/internal/cmd/render_test.go index cf1fa42..8cecaf5 100644 --- a/internal/cmd/render_test.go +++ b/internal/cmd/render_test.go @@ -281,7 +281,7 @@ func TestRender_KnownKeyBadType_PassesThroughToAPI(t *testing.T) { // v0.9.0 schema-as-docs contract: unknown key with a fuzzy match emits // an agent-consumable warning; the request still goes through verbatim. // -// v1.0.4 Class 3.4: in JSON/quiet mode the warning rides in +// In JSON/quiet mode the warning rides in // envelope.warnings, not as plain stderr text alongside the JSON on // stdout (the pre-1.0.4 behaviour mixed streams in a way that broke // agents who consumed either stream alone). @@ -297,7 +297,7 @@ func TestRender_FuzzyCorrection_JSONMode_WarningInEnvelope(t *testing.T) { if exit != 0 { t.Fatalf("exit=%d, want 0 (passthrough); stdout=%s stderr=%s", exit, stdout.String(), stderr.String()) } - // v1.0.4 Class 3.4: no plain-text warning on stderr in JSON mode. + // No plain-text warning on stderr in JSON mode. if strings.Contains(stderr.String(), "warning:") { t.Errorf("JSON mode: warning must NOT leak to stderr; got %q", stderr.String()) } @@ -910,7 +910,7 @@ func TestRender_WaitUntilHelp_ListsRealEnumValues(t *testing.T) { // TestRender_APISecretStdin_ConflictsWithJSONStdin pins that --api-secret-stdin // and --json - cannot share stdin; the runRender entry rejects the combo -// before either reader touches stdin. Round 1 S-C2 follow-up. +// before either reader touches stdin. func TestRender_APISecretStdin_ConflictsWithJSONStdin(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) t.Setenv("URLBOX_API_SECRET", "sec_test") @@ -932,7 +932,7 @@ func TestRender_APISecretStdin_ConflictsWithJSONStdin(t *testing.T) { } } -// TestRender_DryRun_ValidatesOutputSandbox pins Round 4 M1: --dry-run +// TestRender_DryRun_ValidatesOutputSandbox pins a regression: --dry-run // must catch --output paths that escape the CWD sandbox. Before this fix, // "ok: true, payload validated" came back for paths the real run would // reject — defeating dry-run as a pre-flight tool. @@ -957,7 +957,7 @@ func TestRender_DryRun_ValidatesOutputSandbox(t *testing.T) { } } -// TestRender_OutputPath_WritabilityPreflight pins Round 4 M6: when +// TestRender_OutputPath_WritabilityPreflight pins a regression: when // --output points at a directory the user can't write, the failure must // be detected BEFORE the API call (no wasted credit) and surfaced as // ErrValidation (not ErrServer — that misclassified the user error as @@ -1012,7 +1012,7 @@ func TestRender_OutputPath_WritabilityPreflight(t *testing.T) { } } -// TestRender_NumericFlag_BeyondJSONSafeInt_Errors pins Round 4 M2: +// TestRender_NumericFlag_BeyondJSONSafeInt_Errors pins a regression: // width/height/etc. accepted Go int64 values past 2^53 and JSON-marshaled // them as float64 — silently rounding to a different value than the user // passed. The schema documents [-9007199254740991, 9007199254740991]; the @@ -1071,8 +1071,8 @@ func TestRender_NumericFlag_AtBoundary_Allowed(t *testing.T) { } } -// TestRender_JSONPath_BeyondJSONSafeInt_Errors pins Round 5 Adv-1 (High): -// the M2 flag-parser fix doesn't apply to values passed through --json, +// TestRender_JSONPath_BeyondJSONSafeInt_Errors pins a regression: +// the flag-parser fix doesn't apply to values passed through --json, // even though the flag-parser's own error message warns about exactly // this rounding. The dry-run output silently substituted // 9007199254740993 → 9007199254740992 (float64 rounding). @@ -1186,8 +1186,8 @@ func TestRender_JSONPath_NestedSlice_NoStrayJSONNumber(t *testing.T) { } } -// TestRender_QuietMode_EmitsSingleUsefulValue pins Round 5 First-3 / -// Power-1: render --output-format quiet used to dump the full data +// TestRender_QuietMode_EmitsSingleUsefulValue pins a regression: +// render --output-format quiet used to dump the full data // JSON. Other quiet-mode commands (link, config get, config path) // emit a single bare value suitable for piping. Render should follow // suit, contextually: @@ -1236,7 +1236,7 @@ func TestRender_QuietMode_EmitsSingleUsefulValue(t *testing.T) { }) } -// TestRender_TextMode_SummaryOnly pins Round 5 Power-2: --output-format +// TestRender_TextMode_SummaryOnly pins a regression: --output-format // text used to print the green ✓ summary AND a raw JSON dump of .data. // "text" should be the human-readable format — summary only. Users who // want JSON should pass --output-format json. @@ -1263,7 +1263,7 @@ func TestRender_TextMode_SummaryOnly(t *testing.T) { } } -// TestRender_DryRunWithCurl_WarnsAboutSilentDrop pins Round 5 Adv-3: +// TestRender_DryRunWithCurl_WarnsAboutSilentDrop pins a regression: // when --dry-run AND --curl are passed together, --dry-run wins // silently. The agent's complaint was "the user has no signal that // --curl was dropped". Now: a breadcrumb in the dry-run envelope @@ -1315,7 +1315,7 @@ func TestRender_DryRunWithOutput_WarnsAboutSilentDrop(t *testing.T) { } } -// TestRender_NegativeTimeout_Errors pins Round 5 Adv-5: --timeout -5s +// TestRender_NegativeTimeout_Errors pins a regression: --timeout -5s // used to produce the nonsense diagnostic "Render timed out after -5s". // Negative durations should be rejected at parse time with a clear // usage error. @@ -1342,7 +1342,7 @@ func TestRender_NegativeTimeout_Errors(t *testing.T) { } } -// TestRender_JSONEmptyURL_Errors pins Round 5 Adv-7: --json '{"url":""}' +// TestRender_JSONEmptyURL_Errors pins a regression: --json '{"url":""}' // and --json '{"url":null}' bypassed the "missing url" guard because // it only checked the KEY's presence, not the value. Empty / null URL // is structurally equivalent to "no URL" — reject both. @@ -1379,14 +1379,14 @@ func TestRender_JSONEmptyURL_Errors(t *testing.T) { } } -// TestRender_JSONPath_RecursiveBigIntGuard pins Round 6 Adv-1 class-fix: -// the original guard (Round 5 Commit O) only walked the top-level keys +// TestRender_JSONPath_RecursiveBigIntGuard pins a regression: +// the original guard only walked the top-level keys // width/height/delay/quality, leaving every other integer in the JSON // tree unchecked. The adversarial agent demonstrated the bypass with // {"viewport":{"width":9007199254740993}} — same incident class, one // level deeper. // -// The class-fix walks the entire tree (nested objects + arrays + deep +// The guard walks the entire tree (nested objects + arrays + deep // combinations). Any integer past ±2^53-1 anywhere is rejected with // a path-qualified error message. func TestRender_JSONPath_RecursiveBigIntGuard(t *testing.T) { @@ -1397,7 +1397,7 @@ func TestRender_JSONPath_RecursiveBigIntGuard(t *testing.T) { // Top-level (already worked, regression guard) {"top-level width", `{"url":"https://e.com","width":9007199254740993}`}, - // Nested object — the original Adv-1 Round 6 repro + // Nested object — the original repro {"nested in viewport", `{"url":"https://e.com","viewport":{"width":9007199254740993}}`}, // Array element @@ -1488,9 +1488,9 @@ func TestRender_JSONPath_RecursiveBigIntGuard_AcceptsValid(t *testing.T) { } } -// TestRender_JSON_LargeIntBeyondInt64_Rejected pins Round 8 LL: numbers -// > 2^63-1 were silently coerced to float scientific notation (the Y -// commit's walker called Int64() and silently passed through on error, +// TestRender_JSON_LargeIntBeyondInt64_Rejected pins a regression: numbers +// > 2^63-1 were silently coerced to float scientific notation (the earlier +// walker called Int64() and silently passed through on error, // missing the case where the error was "out of int64 range" not "not // an integer"). func TestRender_JSON_LargeIntBeyondInt64_Rejected(t *testing.T) { @@ -1521,9 +1521,9 @@ func TestRender_JSON_LargeIntBeyondInt64_Rejected(t *testing.T) { } } -// TestRender_JSON_DuplicateKey_Rejected pins Round 8 LL: standard +// TestRender_JSON_DuplicateKey_Rejected pins a regression: standard // json.Unmarshal silently takes the last value when keys repeat. The -// Adv-2 demo `{"url":"a","url":"b"}` signed b with no warning. +// repro `{"url":"a","url":"b"}` signed b with no warning. func TestRender_JSON_DuplicateKey_Rejected(t *testing.T) { cases := []struct { name, payload string @@ -1580,7 +1580,7 @@ func TestRender_JSON_FractionalAndScientific_PassThrough(t *testing.T) { } } -// ─── v1.0.4 Class 5.1 — auth pre-flight ───────────────────────────── +// ─── auth pre-flight ───────────────────────────── // // Invariant: missing-secret renders fail fast on the CLI side with the // CLI's own vocabulary, not a wasted round-trip to the API's confusing diff --git a/internal/cmd/report.go b/internal/cmd/report.go new file mode 100644 index 0000000..e90e5aa --- /dev/null +++ b/internal/cmd/report.go @@ -0,0 +1,216 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/spf13/cobra" + + "github.com/urlbox/urlbox-cli/internal/output" + "github.com/urlbox/urlbox-cli/internal/prompt" +) + +// reportCategory pairs the API enum value with the human label the +// interactive picker shows. Order is the picker order. +type reportCategory struct { + value string + label string +} + +// reportCategories is the closed category set the render-reports API +// accepts, in picker order. +var reportCategories = []reportCategory{ + {value: "bot-detection", label: "Bot detection / blocked"}, + {value: "login-required", label: "Login required"}, + {value: "missing-content", label: "Missing content"}, + {value: "cookie-banner-or-popup", label: "Cookie banner or popup"}, + {value: "render-failed", label: "Render failed"}, + {value: "other", label: "Other"}, +} + +func categoryValues() []string { + vals := make([]string, len(reportCategories)) + for i, c := range reportCategories { + vals[i] = c.value + } + return vals +} + +func isValidCategory(v string) bool { + for _, c := range reportCategories { + if c.value == v { + return true + } + } + return false +} + +const maxReportCommentLen = 2000 + +// renderReportsPath builds the org-scoped create/list path, matching the +// credkind orgListPath convention. +func renderReportsPath(org string) string { + return "/v2/organisation/" + org + "/render-reports" +} + +func newReportCmd() *cobra.Command { + var category, comment string + c := &cobra.Command{ + Use: "report ", + Short: "Report a problem with a render", + Long: `File a render report with the Urlbox team. + +The render id is printed after every render, and async responses carry it +as renderId. Requires a signed-in session (urlbox login) with an active +organisation. Reports are limited to a category plus a short comment; the +team follows up via support. + +Without --category/--comment in an interactive terminal, the command +prompts for the missing values. In non-interactive use both flags are +required. + +Exit codes: + 0 report filed + 1 usage (missing/invalid category or comment in non-interactive use) + 3 not logged in + 5 render not found (expired or not from your organisation) + 7 the report is already with the team and locked`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runReport(cmd, args[0], category, comment) + }, + } + c.Flags().StringVar(&category, "category", "", "Problem category: "+strings.Join(categoryValues(), ", ")) + c.Flags().StringVar(&comment, "comment", "", "Short description of what went wrong (required with --category)") + attachSessionRetryFlags(c) + return c +} + +func runReport(cmd *cobra.Command, renderID, category, comment string) error { + sess, cliErr := loadSession(cmd) + if cliErr != nil { + return cliErr + } + org, orgErr := requireActiveOrg(sess) + if orgErr != nil { + return orgErr + } + category, comment, inErr := resolveReportInputs(cmd, category, comment) + if inErr != nil { + return inErr + } + body := map[string]any{ + "renderId": renderID, + "category": category, + "comment": comment, + } + ctx := context.Background() + var created map[string]any + if err := sess.Client.PostJSON(ctx, renderReportsPath(org), body, &created); err != nil { + return refineReportError(err) + } + env := output.NewEnvelope("report", created, + fmt.Sprintf("Report saved for render %s", renderID), nil) + return writeEnvelopeWithQuietData(cmd, env, valueOrEmpty(created["id"])) +} + +// resolveReportInputs validates flag-provided values and fills missing ones +// interactively (text mode + TTY); non-interactive runs get a usage error +// naming the missing flags. +func resolveReportInputs(cmd *cobra.Command, category, comment string) (resolvedCategory, resolvedComment string, cliErr *output.CLIError) { + if category != "" && !isValidCategory(category) { + return "", "", output.NewCLIError(output.ErrUsage, + fmt.Sprintf("unknown --category %q", category), + "Use one of: "+strings.Join(categoryValues(), ", ")+".") + } + comment = strings.TrimSpace(comment) + if len(comment) > maxReportCommentLen { + return "", "", output.NewCLIError(output.ErrUsage, + fmt.Sprintf("--comment is too long (%d chars, max %d)", len(comment), maxReportCommentLen), + "Shorten the comment; detail can follow via urlbox support.") + } + if category != "" && comment != "" { + return category, comment, nil + } + if interactiveText(cmd) { + return promptReportInputs(cmd, category, comment) + } + return "", "", missingReportFlagsError(category, comment) +} + +func missingReportFlagsError(category, comment string) *output.CLIError { + missing := []string{} + if category == "" { + missing = append(missing, "--category") + } + if comment == "" { + missing = append(missing, "--comment") + } + return output.NewCLIError(output.ErrUsage, + "missing "+strings.Join(missing, " and "), + "Pass --category ("+strings.Join(categoryValues(), ", ")+") and --comment, or run in an interactive terminal.") +} + +var ( + reportCategoryPick pickFunc = promptPick + reportCommentInput func(string, func(string) error) (string, error) = prompt.TextInput +) + +// SetReportPromptsForTest swaps both report prompts. Pair with +// t.Cleanup(ResetReportPromptsForTest). +func SetReportPromptsForTest(pick pickFunc, input func(string, func(string) error) (string, error)) { + reportCategoryPick = pick + reportCommentInput = input +} + +// ResetReportPromptsForTest restores the production prompts. +func ResetReportPromptsForTest() { + reportCategoryPick = promptPick + reportCommentInput = prompt.TextInput +} + +func promptReportInputs(cmd *cobra.Command, category, comment string) (resolvedCategory, resolvedComment string, cliErr *output.CLIError) { + if category == "" { + labels := make([]string, len(reportCategories)) + for i, c := range reportCategories { + labels[i] = c.label + } + idx, err := reportCategoryPick("What went wrong with this render?", labels, -1) + if err != nil { + return "", "", missingReportFlagsError(category, comment) + } + category = reportCategories[idx].value + } + if comment == "" { + typed, err := reportCommentInput("Add a short description:", func(s string) error { + if strings.TrimSpace(s) == "" { + return errors.New("a short description is required") + } + if len(s) > maxReportCommentLen { + return fmt.Errorf("keep it under %d characters", maxReportCommentLen) + } + return nil + }) + if err != nil { + return "", "", missingReportFlagsError(category, comment) + } + comment = typed + } + return category, comment, nil +} + +// refineReportError swaps the generic 404/409 hints (written for render +// lookups) for report-specific recovery text. Everything else passes +// through asCLIError untouched. +func refineReportError(err error) error { + cli := asCLIError(err) + switch cli.Code { + case output.ErrNotFound: + cli.Hint = "Renders are kept for around 60 days and must belong to your active organisation. Check the id against a recent render." + case output.ErrConflict: + cli.Hint = "The team already has this report in review. Use `urlbox support` if you need to add something." + } + return cli +} diff --git a/internal/cmd/report_hint.go b/internal/cmd/report_hint.go new file mode 100644 index 0000000..5f4bd97 --- /dev/null +++ b/internal/cmd/report_hint.go @@ -0,0 +1,63 @@ +package cmd + +import ( + "errors" + "fmt" + + "github.com/spf13/cobra" + + "github.com/urlbox/urlbox-cli/internal/api" + "github.com/urlbox/urlbox-cli/internal/output" +) + +// reportHintLine is the post-render affordance: one ready-to-run command +// carrying the render id. +func reportHintLine(renderID string) string { + return "Something wrong with this render? Report it: urlbox report " + renderID +} + +// renderIDForHint picks the id the hint should name: async bodies carry +// renderId; sync bodies carry nothing, so the header-captured Response +// field is the fallback. +func renderIDForHint(resp *api.Response) string { + if id, ok := resp.Data["renderId"].(string); ok && id != "" { + return id + } + return resp.RenderID +} + +// hintGatesPass centralises the affordance rulings: text output only, +// stderr TTY only. json/quiet stay byte-identical for agents. +func hintGatesPass(cmd *cobra.Command) bool { + formatFlag, _ := cmd.Root().PersistentFlags().GetString("output-format") + if output.ResolveFormat(formatFlag, cmd.OutOrStdout()) != output.FormatText { + return false + } + return isStderrTTY(cmd.ErrOrStderr()) +} + +// printReportHint writes the muted hint line to stderr after a render +// result. No-op when any gate fails or no id was captured. +func printReportHint(cmd *cobra.Command, renderID string) { + if renderID == "" || !hintGatesPass(cmd) { + return + } + styles := output.NewStylesForWriter(cmd.ErrOrStderr()) + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), styles.Muted.Render(" "+reportHintLine(renderID))) +} + +// appendReportHint rides the failed render's existing Hint (printed under +// the Error: line in text mode) so the report affordance lands below the +// error rather than above it. The mutation only happens when the text+TTY +// gates pass, so json/quiet error envelopes never carry it. +func appendReportHint(cmd *cobra.Command, err error) error { + var ridErr *api.RenderIDError + if !errors.As(err, &ridErr) || ridErr.RenderID == "" || !hintGatesPass(cmd) { + return err + } + if ridErr.Err.Hint != "" { + ridErr.Err.Hint += "\n" + } + ridErr.Err.Hint += reportHintLine(ridErr.RenderID) + return err +} diff --git a/internal/cmd/report_hint_test.go b/internal/cmd/report_hint_test.go new file mode 100644 index 0000000..36d1462 --- /dev/null +++ b/internal/cmd/report_hint_test.go @@ -0,0 +1,157 @@ +package cmd + +import ( + "bytes" + "net/http" + "strings" + "testing" + + "github.com/urlbox/urlbox-cli/internal/api/apitest" +) + +const syncSuccessBody = `{"renderUrl":"https://renders.urlbox.com/x.png","size":17752}` + +func syncSuccessWithHeader() apitest.ScriptedResponse { + return apitest.ScriptedResponse{ + Status: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Urlbox-Request-Id": []string{"01a0906a-fff5-7565-8dc5-02868f11a4fa_ps"}, + }, + Body: syncSuccessBody, + } +} + +func runRenderForHint(t *testing.T, script apitest.ScriptedResponse, args ...string) (stdout, stderr string, code int) { + t.Helper() + dir := t.TempDir() + writeCompatConfig(t, dir, false) + t.Setenv("XDG_CONFIG_HOME", dir) + srv := apitest.New(script) + t.Cleanup(srv.Close) + t.Setenv("URLBOX_API_HOST", srv.URL()) + var out, errBuf bytes.Buffer + code = Execute(args, &out, &errBuf) + return out.String(), errBuf.String(), code +} + +func TestRenderHint_SyncSuccess_TextTTY_PrintsHintToStderr(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + stdout, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout, stderr) + } + if !strings.Contains(stderr, "urlbox report 01a0906a-fff5-7565-8dc5-02868f11a4fa_ps") { + t.Fatalf("stderr missing hint: %q", stderr) + } + if strings.Contains(stdout, "urlbox report") { + t.Fatalf("hint leaked to stdout: %q", stdout) + } +} + +func TestRenderHint_SyncSuccess_NonTTYStderr_NoHint(t *testing.T) { + SetStderrTTYForTest(false) + t.Cleanup(ResetStderrTTYForTest) + _, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stderr, "urlbox report") { + t.Fatalf("hint must be absent off-TTY: %q", stderr) + } +} + +func TestRenderHint_JSONMode_NoHint(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + stdout, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "json") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stderr, "urlbox report") || strings.Contains(stdout, "urlbox report") { + t.Fatalf("hint must be absent in json mode\nstdout=%q\nstderr=%q", stdout, stderr) + } + if strings.Contains(stdout, "renderId") { + t.Fatalf("sync json envelope must not gain a renderId field: %q", stdout) + } +} + +func TestRenderHint_QuietMode_NoHint(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + stdout, stderr, code := runRenderForHint(t, syncSuccessWithHeader(), + "render", "https://example.com", "--output-format", "quiet") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stdout+stderr, "urlbox report") { + t.Fatalf("hint must be absent in quiet mode") + } +} + +func TestRenderHint_SyncSuccess_NoHeader_NoHint(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + _, stderr, code := runRenderForHint(t, apitest.SuccessJSON(syncSuccessBody), + "render", "https://example.com", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.Contains(stderr, "urlbox report") { + t.Fatalf("no id captured, no hint: %q", stderr) + } +} + +func TestRenderHint_Async_UsesBodyRenderID(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + _, stderr, code := runRenderForHint(t, + apitest.SuccessJSON(`{"status":"created","renderId":"01a09072-9ec6-76ca-bcc7-bbf5cc35a9e4_pa","statusUrl":"https://api.urlbox.com/v1/render/x"}`), + "render", "https://example.com", "--async", "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if !strings.Contains(stderr, "urlbox report 01a09072-9ec6-76ca-bcc7-bbf5cc35a9e4_pa") { + t.Fatalf("async hint missing: %q", stderr) + } +} + +func TestRenderHint_Failure_TextTTY_HintUnderError(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + fail := apitest.ScriptedResponse{ + Status: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"}`, + } + stdout, stderr, code := runRenderForHint(t, fail, + "render", "https://x.invalid", "--output-format", "text", "--no-retry") + if code != 2 { + t.Fatalf("exit %d, want 2 (validation)\n%s\n%s", code, stdout, stderr) + } + if !strings.Contains(stderr, "urlbox report 01a09082-25cf-74bd-9188-022c4f5bf25f_ps") { + t.Fatalf("failure hint missing from stderr: %q", stderr) + } +} + +func TestRenderHint_Failure_JSONMode_NoHintInEnvelope(t *testing.T) { + SetStderrTTYForTest(true) + t.Cleanup(ResetStderrTTYForTest) + fail := apitest.ScriptedResponse{ + Status: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"error":{"message":"Invalid URL","code":"InvalidURLError"},"requestId":"01a09082-25cf-74bd-9188-022c4f5bf25f_ps"}`, + } + stdout, _, code := runRenderForHint(t, fail, + "render", "https://x.invalid", "--output-format", "json", "--no-retry") + if code != 2 { + t.Fatalf("exit %d, want 2", code) + } + if strings.Contains(stdout, "urlbox report") { + t.Fatalf("json error envelope must not carry the report hint: %q", stdout) + } +} diff --git a/internal/cmd/report_test.go b/internal/cmd/report_test.go new file mode 100644 index 0000000..5224e94 --- /dev/null +++ b/internal/cmd/report_test.go @@ -0,0 +1,255 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/urlbox/urlbox-cli/internal/api/apitest" +) + +const reportDTOJSON = `{"id":"rpt_x1","renderId":"01a0906a-fff5-7565-8dc5-02868f11a4fa_ps","category":"bot-detection","comment":"page shows a captcha","rects":[],"state":"open","outputUrl":null,"outputFormat":null,"resolutionMessage":null,"resolutionExampleOptions":null,"resolutionExampleUrl":null,"hasUnseenResolution":false,"createdAt":"2026-09-11T12:00:00.000Z"}` + +func runReportCmd(t *testing.T, scripts []apitest.ScriptedResponse, args ...string) (stdout, stderr string, code int, srv *apitest.Server) { + t.Helper() + dir := t.TempDir() + writeCompatConfig(t, dir, true) + t.Setenv("XDG_CONFIG_HOME", dir) + srv = apitest.New(scripts...) + t.Cleanup(srv.Close) + t.Setenv("URLBOX_API_HOST", srv.URL()) + var out, errBuf bytes.Buffer + code = Execute(args, &out, &errBuf) + return out.String(), errBuf.String(), code, srv +} + +func TestReport_FlagsPath_PostsAndSummarises(t *testing.T) { + stdout, stderr, code, srv := runReportCmd(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--category", "bot-detection", "--comment", "page shows a captcha", + "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout, stderr) + } + reqs := srv.Requests() + if reqs[0].Method != "POST" || reqs[0].Path != "/v2/organisation/org_compat/render-reports" { + t.Fatalf("request: %+v", reqs[0]) + } + var body map[string]any + if err := json.Unmarshal(reqs[0].Body, &body); err != nil { + t.Fatalf("body: %v", err) + } + if body["renderId"] != "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps" || + body["category"] != "bot-detection" || + body["comment"] != "page shows a captcha" { + t.Fatalf("body: %v", body) + } + if _, present := body["rects"]; present { + t.Fatalf("rects must not be sent: %v", body) + } + if !strings.Contains(stdout, "Report saved for render 01a0906a-fff5-7565-8dc5-02868f11a4fa_ps") { + t.Fatalf("summary missing: %q", stdout) + } +} + +func TestReport_JSONMode_EmitsDTOEnvelope(t *testing.T) { + stdout, _, code, _ := runReportCmd(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--category", "other", "--comment", "x", + "--output-format", "json") + if code != 0 { + t.Fatalf("exit %d\n%s", code, stdout) + } + var env map[string]any + if err := json.Unmarshal([]byte(stdout), &env); err != nil { + t.Fatalf("envelope: %v", err) + } + if env["ok"] != true || env["command"] != "report" { + t.Fatalf("envelope: %v", env) + } + data, _ := env["data"].(map[string]any) + if data["id"] != "rpt_x1" || data["state"] != "open" { + t.Fatalf("data: %v", data) + } +} + +func TestReport_QuietMode_EmitsReportID(t *testing.T) { + stdout, _, code, _ := runReportCmd(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--category", "other", "--comment", "x", + "--output-format", "quiet") + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.TrimSpace(stdout) != `"rpt_x1"` { + t.Fatalf("quiet stdout=%q, want the report id", stdout) + } +} + +func TestReport_InvalidCategory_UsageErrorListsValues(t *testing.T) { + _, stderr, code, _ := runReportCmd(t, nil, + "report", "01a0_ps", "--category", "captcha", "--comment", "x", + "--output-format", "text") + if code != 1 { + t.Fatalf("exit %d, want 1 (usage)", code) + } + for _, want := range []string{"bot-detection", "login-required", "missing-content", "cookie-banner-or-popup", "render-failed", "other"} { + if !strings.Contains(stderr, want) { + t.Fatalf("error must list %q: %q", want, stderr) + } + } +} + +func TestReport_EmptyComment_UsageError(t *testing.T) { + _, _, code, _ := runReportCmd(t, nil, + "report", "01a0_ps", "--category", "other", "--comment", " ", + "--output-format", "text") + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } +} + +func TestReport_MissingFlags_NonInteractive_UsageError(t *testing.T) { + stdout, _, code, _ := runReportCmd(t, nil, + "report", "01a0_ps", "--output-format", "json") + if code != 1 { + t.Fatalf("exit %d, want 1 (never hangs)", code) + } + if !strings.Contains(stdout, `"usage"`) || !strings.Contains(stdout, "--category") { + t.Fatalf("json error envelope must name the missing flags: %q", stdout) + } +} + +func TestReport_NotLoggedIn_AuthError(t *testing.T) { + dir := t.TempDir() + writeCompatConfig(t, dir, false) + t.Setenv("XDG_CONFIG_HOME", dir) + var out, errBuf bytes.Buffer + code := Execute([]string{"report", "01a0_ps", "--category", "other", "--comment", "x", "--output-format", "text"}, &out, &errBuf) + if code != 3 { + t.Fatalf("exit %d, want 3 (auth)", code) + } + if !strings.Contains(errBuf.String(), "urlbox login") { + t.Fatalf("auth error must carry the login hint: %q", errBuf.String()) + } +} + +func TestReport_RenderNotFound_Exit5WithExpiryHint(t *testing.T) { + notFound := apitest.ScriptedResponse{ + Status: http.StatusNotFound, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"defined":false,"code":"NOT_FOUND","status":404,"message":"Render not found"}`, + } + _, stderr, code, _ := runReportCmd(t, []apitest.ScriptedResponse{notFound}, + "report", "01a0906a-gone_ps", "--category", "other", "--comment", "x", + "--output-format", "text") + if code != 5 { + t.Fatalf("exit %d, want 5", code) + } + if !strings.Contains(stderr, "Render not found") { + t.Fatalf("API message must surface: %q", stderr) + } + if !strings.Contains(stderr, "60 days") { + t.Fatalf("hint must mention render expiry: %q", stderr) + } +} + +func TestReport_LockedReport_Exit7SurfacesAPIMessage(t *testing.T) { + locked := apitest.ScriptedResponse{ + Status: http.StatusConflict, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: `{"defined":false,"code":"CONFLICT","status":409,"message":"This report is being looked at by the team and can no longer be edited."}`, + } + _, stderr, code, _ := runReportCmd(t, []apitest.ScriptedResponse{locked}, + "report", "01a0_ps", "--category", "other", "--comment", "x", + "--output-format", "text") + if code != 7 { + t.Fatalf("exit %d, want 7", code) + } + if !strings.Contains(stderr, "being looked at by the team") { + t.Fatalf("API conflict message must surface: %q", stderr) + } +} + +func TestReport_Interactive_PromptsForCategoryAndComment(t *testing.T) { + SetReportPromptsForTest( + func(label string, options []string, active int) (int, error) { + if len(options) != 6 { + t.Fatalf("picker options: %v", options) + } + return 0, nil + }, + func(title string, validate func(string) error) (string, error) { + if validate == nil { + t.Fatal("comment input must validate non-empty") + } + if err := validate(""); err == nil { + t.Fatal("empty comment must fail validation") + } + return "page shows a captcha", nil + }, + ) + t.Cleanup(ResetReportPromptsForTest) + stdout, stderr, code, srv := runReportCmd(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0906a-fff5-7565-8dc5-02868f11a4fa_ps", + "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout, stderr) + } + var body map[string]any + if err := json.Unmarshal(srv.Requests()[0].Body, &body); err != nil { + t.Fatalf("body: %v", err) + } + if body["category"] != "bot-detection" || body["comment"] != "page shows a captcha" { + t.Fatalf("prompted values not sent: %v", body) + } +} + +func TestReport_Interactive_FlagCategoryOnlyPromptsComment(t *testing.T) { + picked := false + SetReportPromptsForTest( + func(label string, options []string, active int) (int, error) { + picked = true + return 0, nil + }, + func(title string, validate func(string) error) (string, error) { + return "left half is blank", nil + }, + ) + t.Cleanup(ResetReportPromptsForTest) + _, _, code, srv := runReportCmd(t, + []apitest.ScriptedResponse{apitest.SuccessJSON(reportDTOJSON)}, + "report", "01a0_ps", "--category", "missing-content", + "--output-format", "text") + if code != 0 { + t.Fatalf("exit %d", code) + } + if picked { + t.Fatal("category picker must not run when --category was given") + } + var body map[string]any + if err := json.Unmarshal(srv.Requests()[0].Body, &body); err != nil { + t.Fatalf("body: %v", err) + } + if body["category"] != "missing-content" || body["comment"] != "left half is blank" { + t.Fatalf("body: %v", body) + } +} + +func TestReport_Interactive_NonTTYStdin_FallsBackToUsage(t *testing.T) { + ResetReportPromptsForTest() + _, stderr, code, _ := runReportCmd(t, nil, + "report", "01a0_ps", "--output-format", "text") + if code != 1 { + t.Fatalf("exit %d, want 1 (prompt.ErrNotInteractive path)", code) + } + if !strings.Contains(stderr, "--category") { + t.Fatalf("usage error must name the flags: %q", stderr) + } +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index a011811..d81665b 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -70,12 +70,12 @@ func Execute(args []string, stdout, stderr io.Writer) int { if !cliErr.Silent { env := output.NewErrorEnvelope(calledCommandFromArgs(rootCmd, args), cliErr) - // Round 8 OO: honor --jq on error envelopes too. Before this, + // Honor --jq on error envelopes too. Before this, // `urlbox … --jq '.code'` extracted on success but the failure // path always dumped the full envelope, breaking agents that // scripted around a single jq expression across both paths. jqExpr, _ := rootCmd.PersistentFlags().GetString("jq") - // v1.0.4 Class 3.2 — route errors per the CLAUDE.md + // Route errors per the CLAUDE.md // "stdout for data, stderr for human messages" contract: // - JSON envelope is structured data (agents pipe it) → stdout. // - Text-mode "Error:" line is a human message → stderr. @@ -105,7 +105,7 @@ func Execute(args []string, stdout, stderr io.Writer) int { // retained on every code path, so deriving the path from args via // Find() is the dependable source. // -// Round 8 II: when no subcommand matched (bare `urlbox`, unknown +// When no subcommand matched (bare `urlbox`, unknown // command, unknown flag at root), returns the root name rather than // "". The contract is that every error envelope carries a command — // "urlbox" is more useful for agents than "". @@ -166,7 +166,7 @@ func newRootCmd(stdout, stderr io.Writer) *cobra.Command { "Use one of: json, text, quiet.", ) } - // Round 8 NN: --profile flag value validation. The Adv-4 + // --profile flag value validation. The // `validateProfileName` rule applied to creation but not to // flag-resolution-time. So `--profile ""`, `--profile " "`, // `--profile $'\t'` silently behaved like "no flag" (the @@ -207,7 +207,7 @@ func newRootCmd(stdout, stderr io.Writer) *cobra.Command { defaultHelp := cmd.HelpFunc() cmd.SetHelpFunc(func(c *cobra.Command, args []string) { agent, _ := c.Flags().GetBool("agent") - // v1.0.4 Class 3.3 — --output-format json on --help is the + // --output-format json on --help is the // agent-discoverable equivalent of --agent. Pre-1.0.4 only // --agent --help returned JSON; agents probing the obvious // combo (the existing format flag) silently got plain text. @@ -237,11 +237,13 @@ func newRootCmd(stdout, stderr io.Writer) *cobra.Command { cmd.AddCommand(newProjectsCmd()) cmd.AddCommand(newProxiesCmd()) cmd.AddCommand(newRenderCmd()) + cmd.AddCommand(newReportCmd()) cmd.AddCommand(newSchemaCmd()) cmd.AddCommand(newScreenshotCmd()) cmd.AddCommand(newSkillCmd()) cmd.AddCommand(newStatusCmd()) cmd.AddCommand(newStorageCmd()) + cmd.AddCommand(newSupportCmd()) cmd.AddCommand(newUsageCmd()) cmd.AddCommand(newVersionCmd()) cmd.AddCommand(newVideoCmd()) @@ -255,7 +257,7 @@ func newRootCmd(stdout, stderr io.Writer) *cobra.Command { // cobra.NoArgs) and returns the closest known IMMEDIATE-subcommand name // of that parent, if any. // -// Round 6 BB class-fix: the previous version always walked root.Commands(), +// The previous version always walked root.Commands(), // which meant `urlbox schema list` (where schema has no `list`) cross- // suggested top-level commands like `link`. Now the parent path is // parsed from the error string and the candidate pool is scoped to @@ -302,7 +304,7 @@ func suggestUnknownCommand(root *cobra.Command, msg string) (string, bool) { continue } if sub.Name() == typed { - continue // belt-and-braces (matches AA's class-fix shape) + continue // belt-and-braces: never suggest the typed token back } candidates = append(candidates, sub.Name()) } @@ -314,7 +316,7 @@ func suggestUnknownCommand(root *cobra.Command, msg string) (string, bool) { // long flag name on the ACTIVE subcommand (plus its inherited persistent // flags). The flag prefix (`--`) is NOT included in the returned name. // -// Round 6 AA class-fix: the candidate pool used to be the union of every +// The candidate pool used to be the union of every // flag in the command tree. A typo on one command could match a flag // from an unrelated command — including the rejected flag itself when // it happened to live on a sibling. `urlbox render --url` would suggest diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index fd93958..ec1100f 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -291,7 +291,7 @@ func TestUnknownCommand_FarTypo_NoSuggestion(t *testing.T) { } } -// TestRoot_DidYouMean_NoSelfSuggestion pins Round 6 Tester #2 + Adv-8: +// TestRoot_DidYouMean_NoSelfSuggestion pins a regression: // the suggester used to return the exact rejected flag back as the // suggestion, because its candidate pool was the union of every flag // across every command in the tree. If the typed flag existed on ANY @@ -308,7 +308,7 @@ func TestRoot_DidYouMean_NoSelfSuggestion(t *testing.T) { args []string }{ // `render` has no --url (it takes positional URL). Without the - // class-fix, this suggests --url (from link). + // scoped pool, this suggests --url (from link). {"render --url tautology", []string{"render", "--url", "https://example.com"}}, // `link` accepts only api-* + format + jq + json + output-format + profile + url. // --width is render's. Used to suggest --width back to itself. @@ -353,7 +353,7 @@ func TestRoot_DidYouMean_NoSelfSuggestion(t *testing.T) { } // TestRoot_DidYouMean_TyposStillWork pins that legitimate typos still -// get a useful suggestion — the class-fix narrows the pool, it doesn't +// get a useful suggestion — the fix narrows the pool, it doesn't // eliminate suggestions. func TestRoot_DidYouMean_TyposStillWork(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) @@ -372,7 +372,7 @@ func TestRoot_DidYouMean_TyposStillWork(t *testing.T) { } } -// TestRoot_UnknownSubcommand_ScopedSuggestion pins Round 6 Adv-9 bonus +// TestRoot_UnknownSubcommand_ScopedSuggestion pins a review // finding: `urlbox schema list` suggested "link" — a sibling top-level // command — because the suggester walked root.Commands() instead of // the actual parent's subcommands. Class-fix: scope candidates to the @@ -410,7 +410,7 @@ func TestRoot_UnknownSubcommand_ScopedSuggestion(t *testing.T) { // TestRoot_EnvelopeOkMatchesExit_Contract is the meta-test: every // command/scenario that produces ok:false in the envelope MUST also -// exit non-zero, and vice versa. Round 6 Adv-9 surfaced sibling +// exit non-zero, and vice versa. A review surfaced sibling // suspicions on schema list / config profile show; this test walks a // representative slice of error paths and pins the contract. func TestRoot_EnvelopeOkMatchesExit_Contract(t *testing.T) { @@ -453,7 +453,7 @@ func TestRoot_EnvelopeOkMatchesExit_Contract(t *testing.T) { } } -// ─── v1.0.4 Class 3.2 — error stream routing per format ──────────── +// ─── error stream routing per format ──────────── // // Invariant: the output stream a byte goes to depends on what kind of // byte it is, not on where the code happens to be. diff --git a/internal/cmd/secret_input.go b/internal/cmd/secret_input.go index 16eedaa..10f40e4 100644 --- a/internal/cmd/secret_input.go +++ b/internal/cmd/secret_input.go @@ -58,7 +58,7 @@ const maxSecretBytes = 4096 // directExplicit distinguishes `--api-secret ""` (passed explicitly with // an empty value) from `--api-secret` not being passed at all. cobra // collapses both to direct == "" at the Go level — only Flags().Changed() -// can tell them apart. Explicit empty is a usage error (Round 4 M3) so +// can tell them apart. Explicit empty is a usage error so // the user isn't silently fed env/profile when they were trying to test // "what happens with no auth?". // diff --git a/internal/cmd/secret_input_test.go b/internal/cmd/secret_input_test.go index 322389a..a4e0efb 100644 --- a/internal/cmd/secret_input_test.go +++ b/internal/cmd/secret_input_test.go @@ -204,8 +204,8 @@ func TestResolveAPISecretInput_File_MissingHintIsFriendly(t *testing.T) { } } -// TestResolveAPISecretInput_File_MissingMessage_NoPathDuplicate pins -// Round 3 UX nit: os.Open's error already includes the path, so wrapping +// TestResolveAPISecretInput_File_MissingMessage_NoPathDuplicate pins a polish fix: +// os.Open's error already includes the path, so wrapping // it without unwrapping produced "failed to read --api-secret-file /p: // open /p: no such file or directory" with the path appearing twice. // Unwrap via errors.As(*os.PathError) and use the inner cause. diff --git a/internal/cmd/skill.go b/internal/cmd/skill.go index 778ce36..543276d 100644 --- a/internal/cmd/skill.go +++ b/internal/cmd/skill.go @@ -155,7 +155,7 @@ Interactive (humans, on a TTY): return output.NewCLIError(output.ErrServer, "could not resolve install path", err.Error()) } - // v1.0.4 Class 4 — every CLI-initiated write to a user-owned + // Every CLI-initiated write to a user-owned // path goes through SafeWriteUserFile: Lstat refuses symlinks, // atomic rename, no clobber without Force. Pre-1.0.4 this used // bare os.WriteFile, silently destroying user-edited skill diff --git a/internal/cmd/skill_test.go b/internal/cmd/skill_test.go index d97b686..4da3f4f 100644 --- a/internal/cmd/skill_test.go +++ b/internal/cmd/skill_test.go @@ -130,7 +130,7 @@ func TestSkill_DocumentsRenderSurface(t *testing.T) { "renderId", // status command's positional arg term "HMAC", // load-bearing crypto term for link "headless", // dashboard fallback behaviour - // v1.0.4 Class 5.2 — the --output sandbox failure mode is the most + // The --output sandbox failure mode is the most // common bounce-off point for agents; SKILL.md must teach it up // front, not just bury it in `render --help`. Pin the section // header + the two escape hatches. @@ -168,7 +168,7 @@ func TestSkillInstall_NonTTY_NoTarget_Errors(t *testing.T) { // TestSkillInstall_SupportedTargetsListIsStableSorted pins that the // "Supported targets" hint is alphabetically sorted, not map-iteration -// random. Caught Round 1 review (Arch I8 + UX I8). +// random. func TestSkillInstall_SupportedTargetsListIsStableSorted(t *testing.T) { var stdout, stderr bytes.Buffer exit := cmd.Execute([]string{ @@ -515,7 +515,7 @@ func TestSkillInstall_Opencode_Project_WritesUnderCWD(t *testing.T) { } } -// ─── v1.0.4 Class 4.2 — skill install uses SafeWriteUserFile ──────── +// ─── skill install uses SafeWriteUserFile ──────── // // Invariant (via config.SafeWriteUserFile): every CLI-initiated write // to a user-owned path Lstats first (refuses symlinks), atomic-renames diff --git a/internal/cmd/status.go b/internal/cmd/status.go index 3a0972c..a2fe22d 100644 --- a/internal/cmd/status.go +++ b/internal/cmd/status.go @@ -42,11 +42,11 @@ func SetStatusClockForTest(c clock.Clock) { statusClock = c } func ResetStatusClockForTest() { statusClock = clock.New() } // defaultStatusTimeout is the per-call deadline for the status GET. Status -// is cheap; users who want long polling reach for --wait + --timeout (Task 4). +// is cheap; users who want long polling reach for --wait + --timeout. const defaultStatusTimeout = 60 * time.Second // defaultStatusPollInterval is the time between successive GETs when --wait -// is in use (Task 4 wires up the polling loop; Task 3 only registers the flag). +// is in use. const defaultStatusPollInterval = 2 * time.Second // statusFlags carries every convenience flag the status command supports. @@ -319,8 +319,8 @@ func writeStatusEnvelope(cmd *cobra.Command, resp *api.Response, renderID string default: // In-flight: created, retrying, processing, or any future enum // value the API might add. Default to ok=true so future enums - // don't auto-bomb the agent — Task 4's --wait will gate on terminal - // states explicitly. If the API ever introduces a new terminal + // don't auto-bomb the agent — --wait gates on terminal states + // explicitly. If the API ever introduces a new terminal // status (cancelled, expired, etc.), warn on stderr so an operator // notices the misclassification even though we keep the agent path // non-fatal. @@ -403,7 +403,7 @@ func buildStatusClient(cmd *cobra.Command, f *statusFlags) (api.Client, *output. ) } - // v1.0.4 Class 5.1 — detect missing secret client-side. See the + // Detect missing secret client-side. See the // matching comment in render.go.buildRenderClient. if cli := requireSecret(resolved); cli != nil { return nil, cli diff --git a/internal/cmd/status_test.go b/internal/cmd/status_test.go index cdc4d9d..d51161d 100644 --- a/internal/cmd/status_test.go +++ b/internal/cmd/status_test.go @@ -403,7 +403,7 @@ func TestStatus_Wait_FailedTerminal_Exit10(t *testing.T) { // TestStatus_Wait_TimesOut_TimeoutExit confirms that when --timeout elapses // before a terminal status is observed, runStatusWait returns ErrTimeout // (exit 11) with a message naming the renderId, the duration, and the last -// status. Round 1 review (Arch I2 + UX I7): a deadline-exceeded outcome is +// status. A deadline-exceeded outcome is // what ErrTimeout exists for; exit 1 (ErrUsage) conflated poll-timeouts with // "bad flag" usage errors and broke agent retry classification. func TestStatus_Wait_TimesOut_TimeoutExit(t *testing.T) { @@ -462,7 +462,7 @@ func TestStatus_Wait_TimesOut_TimeoutExit(t *testing.T) { } // TestStatus_Wait_LaterPollContextTimeout_DoesNotSayShorterThanOneCall -// pins Arch I1: the "shorter than a single API call" friendly message +// pins a regression: the "shorter than a single API call" friendly message // must only fire on attempt 0. If a per-poll context-deadline fires on // attempt N>0 (e.g., a slow GET several polls into a long --wait), the // friendly message lies — by definition the first call already succeeded. diff --git a/internal/cmd/support.go b/internal/cmd/support.go new file mode 100644 index 0000000..1025a38 --- /dev/null +++ b/internal/cmd/support.go @@ -0,0 +1,88 @@ +package cmd + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/urlbox/urlbox-cli/internal/browser" + "github.com/urlbox/urlbox-cli/internal/output" +) + +// supportURL is the Urlbox contact page — the canonical support entry +// point; /support has no dedicated page. +const supportURL = "https://urlbox.com/contact" + +// supportOpener is the browser opener used by the support command. Own var +// so test injection doesn't cross-contaminate dashboard's. +var supportOpener browser.Opener = browser.NewOSOpener() + +// SetSupportOpenerForTest swaps in a fake browser.Opener. Pair with +// t.Cleanup(ResetSupportOpenerForTest). +func SetSupportOpenerForTest(o browser.Opener) { supportOpener = o } + +// ResetSupportOpenerForTest restores the production OSOpener. +func ResetSupportOpenerForTest() { supportOpener = browser.NewOSOpener() } + +func newSupportCmd() *cobra.Command { + return &cobra.Command{ + Use: "support", + Short: "Open the Urlbox support contact page in your browser", + Long: `Opens https://urlbox.com/contact in your default browser. + +On headless environments the URL is printed to stderr instead. The +standard envelope is still emitted on stdout so agents and pipelines can +rely on the same shape regardless of host. + +Exit codes: + 0 browser launched, or URL printed in headless mode + 10 the OS browser handler returned an error (URL is in the hint)`, + Args: cobra.NoArgs, + RunE: func(c *cobra.Command, _ []string) error { + return runSupport(c) + }, + } +} + +func runSupport(c *cobra.Command) error { + data := map[string]any{"url": supportURL} + formatFlag, _ := c.Root().PersistentFlags().GetString("output-format") + resolvedFormat := output.ResolveFormat(formatFlag, c.OutOrStdout()) + if resolvedFormat == output.FormatJSON || resolvedFormat == output.FormatQuiet { + return writeSupportEnvelope(c, data, + "Support URL emitted (no browser launched in machine-readable mode)", + []output.Breadcrumb{{Action: "copy", Cmd: supportURL}}) + } + if isHeadless() { + _, _ = fmt.Fprintln(c.ErrOrStderr(), + "Support URL: "+supportURL+" (open in any browser)") + return writeSupportEnvelope(c, data, + "Support URL printed (no graphical session detected)", + []output.Breadcrumb{{Action: "copy", Cmd: supportURL}}) + } + if err := supportOpener.Open(supportURL); err != nil { + return output.NewCLIError( + output.ErrServer, + "Failed to open browser: "+err.Error(), + "Open this URL manually: "+supportURL, + ) + } + return writeSupportEnvelope(c, data, + "Opened "+supportURL, + []output.Breadcrumb{{Action: "browse", Cmd: supportURL}}) +} + +// writeSupportEnvelope mirrors writeDashboardEnvelope: quiet emits the +// bare URL for pipelines, everything else the standard envelope. +func writeSupportEnvelope(c *cobra.Command, data map[string]any, summary string, breadcrumbs []output.Breadcrumb) error { + formatFlag, _ := c.Root().PersistentFlags().GetString("output-format") + jqExpr, _ := c.Root().PersistentFlags().GetString("jq") + stdout := c.OutOrStdout() + format := output.ResolveFormat(formatFlag, stdout) + if format == output.FormatQuiet && jqExpr == "" { + _, err := fmt.Fprintln(stdout, supportURL) + return err + } + env := output.NewEnvelope("support", data, summary, breadcrumbs) + return writeEnvelope(c, env) +} diff --git a/internal/cmd/support_test.go b/internal/cmd/support_test.go new file mode 100644 index 0000000..d6d2a27 --- /dev/null +++ b/internal/cmd/support_test.go @@ -0,0 +1,89 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "strings" + "testing" +) + +type fakeSupportOpener struct{ opened []string } + +func (f *fakeSupportOpener) Open(url string) error { + f.opened = append(f.opened, url) + return nil +} + +func TestSupport_TextMode_OpensContactPage(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + SetHeadlessDetectorForTest(func() bool { return false }) + t.Cleanup(ResetHeadlessDetectorForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "text"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, stdout.String(), stderr.String()) + } + if len(fake.opened) != 1 || fake.opened[0] != "https://urlbox.com/contact" { + t.Fatalf("opened: %v", fake.opened) + } +} + +func TestSupport_JSONMode_NoBrowserEmitsEnvelope(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "json"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d", code) + } + if len(fake.opened) != 0 { + t.Fatalf("json mode must not launch a browser: %v", fake.opened) + } + var env map[string]any + if err := json.Unmarshal(stdout.Bytes(), &env); err != nil { + t.Fatalf("envelope: %v", err) + } + data, _ := env["data"].(map[string]any) + if env["command"] != "support" || data["url"] != "https://urlbox.com/contact" { + t.Fatalf("envelope: %v", env) + } +} + +func TestSupport_Headless_PrintsURLToStderr(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + SetHeadlessDetectorForTest(func() bool { return true }) + t.Cleanup(ResetHeadlessDetectorForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "text"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d", code) + } + if len(fake.opened) != 0 { + t.Fatalf("headless must not launch: %v", fake.opened) + } + if !strings.Contains(stderr.String(), "https://urlbox.com/contact") { + t.Fatalf("stderr must carry the URL: %q", stderr.String()) + } +} + +func TestSupport_QuietMode_EmitsBareURL(t *testing.T) { + fake := &fakeSupportOpener{} + SetSupportOpenerForTest(fake) + t.Cleanup(ResetSupportOpenerForTest) + var stdout, stderr bytes.Buffer + code := Execute([]string{"support", "--output-format", "quiet"}, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit %d", code) + } + if strings.TrimSpace(stdout.String()) != "https://urlbox.com/contact" { + t.Fatalf("quiet stdout=%q", stdout.String()) + } + if len(fake.opened) != 0 { + t.Fatalf("quiet mode must not launch: %v", fake.opened) + } +} diff --git a/internal/cmd/surface.go b/internal/cmd/surface.go index 15c5a72..c94dda3 100644 --- a/internal/cmd/surface.go +++ b/internal/cmd/surface.go @@ -14,7 +14,7 @@ func newSurfaceCmd(root *cobra.Command) *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { lines := surface.Snapshot(root) - // Round 8 MM: only emit JSON envelope when the user + // Only emit JSON envelope when the user // EXPLICITLY passes --output-format json. The Makefile's // `surface > SURFACE.txt` redirects stdout (not a TTY) and // would otherwise auto-resolve to json — wrecking the @@ -30,7 +30,7 @@ func newSurfaceCmd(root *cobra.Command) *cobra.Command { return writeEnvelope(cmd, env) } w := cmd.OutOrStdout() - // v1.0.4 Class 6: emit the exclusion-rule header so SURFACE.txt + // Emit the exclusion-rule header so SURFACE.txt // is self-documenting. Header lines are '#'-prefixed comments // readers can filter when scripting. for _, h := range surface.Header() { diff --git a/internal/cmd/upgrade_test.go b/internal/cmd/upgrade_test.go index 11e5197..311c0f6 100644 --- a/internal/cmd/upgrade_test.go +++ b/internal/cmd/upgrade_test.go @@ -37,7 +37,7 @@ func TestUpgrade_DetectsScoop(t *testing.T) { } func TestUpgrade_DetectsNpm(t *testing.T) { - method := cmd.DetectInstallMethod("/Users/cjr/.nvm/versions/node/v22.17.0/lib/node_modules/@urlbox/cli/urlbox") + method := cmd.DetectInstallMethod("/Users/user/.nvm/versions/node/v22.17.0/lib/node_modules/@urlbox/cli/urlbox") if method != "npm" { t.Errorf("expected 'npm', got %q", method) } diff --git a/internal/cmd/version.go b/internal/cmd/version.go index 768a3f5..2ff213c 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -1,4 +1,4 @@ -// internal/cmd/version.go — Round 8 II: cobra's --version flag emits a +// internal/cmd/version.go — cobra's --version flag emits a // plain-text line via VersionTemplate, which doesn't honor // --output-format. For agents that want structured version info, this // subcommand emits the same envelope every other command does. diff --git a/internal/cmd/version_test.go b/internal/cmd/version_test.go index 1ae8364..3f29889 100644 --- a/internal/cmd/version_test.go +++ b/internal/cmd/version_test.go @@ -9,7 +9,7 @@ import ( "github.com/urlbox/urlbox-cli/internal/cmd" ) -// TestVersion_Subcommand_EmitsEnvelope pins Round 8 II: the new +// TestVersion_Subcommand_EmitsEnvelope pins that the // `version` subcommand emits an envelope, unlike --version which is // plain text only. JSON consumers (agents) can call this. func TestVersion_Subcommand_EmitsEnvelope(t *testing.T) { @@ -52,10 +52,9 @@ func TestVersion_Subcommand_QuietPrintsScalar(t *testing.T) { } } -// TestRootError_CommandFieldNotEmpty pins Round 8 II / Adv-3 H2: -// root-level errors (unknown command, unknown flag, bare `urlbox`) -// must carry a non-empty `command` field. EE fixed sub-subcommand -// path; this fixes the root-level case. +// TestRootError_CommandFieldNotEmpty pins that root-level errors +// (unknown command, unknown flag, bare `urlbox`) must carry a +// non-empty `command` field, the same as sub-subcommand errors. func TestRootError_CommandFieldNotEmpty_UnknownCommand(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) var stdout, stderr bytes.Buffer @@ -84,7 +83,7 @@ func TestRootError_CommandFieldNotEmpty_UnknownFlag(t *testing.T) { } } -// TestProfileFlag_EmptyOrWhitespace_Rejected pins Round 8 NN: an +// TestProfileFlag_EmptyOrWhitespace_Rejected pins a regression: an // empty or whitespace-only --profile value used to silently behave // like "no flag" because the resolver's `if flagProfile != ""` check // skipped them. Confusing for agents that programmatically set the @@ -108,7 +107,7 @@ func TestProfileFlag_EmptyOrWhitespace_Rejected(t *testing.T) { } } -// TestJQ_AppliedToErrorEnvelope pins Round 8 OO / Adv-4 M3: --jq used +// TestJQ_AppliedToErrorEnvelope pins a regression: --jq used // to be ignored on the error path, so `urlbox render --jq '.code'` // emitted a clean string on success but dumped the full envelope on // failure. Now both paths honor the same jq expression. diff --git a/internal/config/apihost_validate.go b/internal/config/apihost_validate.go index b8931f1..8023ed9 100644 --- a/internal/config/apihost_validate.go +++ b/internal/config/apihost_validate.go @@ -1,4 +1,4 @@ -// internal/config/apihost_validate.go — Round 8 Class B (GG): single +// internal/config/apihost_validate.go — single // gate for api_host values. Before this commit, every write/read site // accepted any string verbatim, including: // @@ -82,7 +82,7 @@ func ValidateAPIHost(raw string) (string, *output.CLIError) { "The Urlbox API only speaks HTTP(S). Schemes like javascript:, file://, ftp:// are rejected as either paste corruption or a phishing attempt.", ) } - // v1.0.4 Class 1.2: plain http:// is only allowed for loopback hosts. + // Plain http:// is only allowed for loopback hosts. // The Urlbox API endpoint is HTTPS; permitting http:// for arbitrary // hosts turned a careless URLBOX_API_HOST or a hostile overlay into a // cleartext-downgrade primitive on the Authorization header. Loopback diff --git a/internal/config/apihost_validate_test.go b/internal/config/apihost_validate_test.go index e93b9b5..ffe95a1 100644 --- a/internal/config/apihost_validate_test.go +++ b/internal/config/apihost_validate_test.go @@ -6,7 +6,7 @@ import ( "github.com/urlbox/urlbox-cli/internal/config" ) -// TestValidateAPIHost_RejectsHostileSchemes pins Round 8 Class B (GG): +// TestValidateAPIHost_RejectsHostileSchemes pins a regression: // the adversarial repro accepted javascript:, file://, ftp://, and // embedded-credential URLs verbatim. Now rejected as ErrUsage. func TestValidateAPIHost_RejectsHostileSchemes(t *testing.T) { @@ -111,7 +111,7 @@ func TestValidateAPIHost_AcceptsValid(t *testing.T) { } } -// ─── Class 1.2 (v1.0.4) — http:// only for loopback ───────────────── +// ─── http:// only for loopback ───────────────── // // Invariant: plain http:// is rejected unless the host is loopback // (127.0.0.1, ::1, localhost). Closes a downgrade path where a diff --git a/internal/config/config.go b/internal/config/config.go index 53e1e45..dc3d1be 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -60,11 +60,11 @@ func Load() (*Config, error) { } // LoadOrCLIError wraps Load with the exit-code-class mapping every cmd-side -// caller wants. Round 8 review M1: bare Load errors used to be wrapped as +// caller wants. Bare Load errors used to be wrapped as // ErrServer (exit 10) at 6 call sites, but the contract reserves 10 for // upstream-server problems. Local I/O failures (chmod 000, fs corruption) // should be ErrForbidden (exit 4) and malformed JSON ErrUsage (exit 2). -// Symmetric with the Update/write path's KK class-fix. +// Symmetric with the Update/write path's error mapping. func LoadOrCLIError() (*Config, *output.CLIError) { cfg, err := Load() if err == nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1adcb0a..f6c4873 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -288,10 +288,10 @@ func TestConfig_Save_AtomicViaRename(t *testing.T) { } } -// TestLoadOrCLIError_PermissionDenied pins Round 8 review M1: config-read +// TestLoadOrCLIError_PermissionDenied pins a regression: config-read // permission errors used to be wrapped as ErrServer at 6 cmd-side call // sites. Now they map to ErrForbidden via LoadOrCLIError, symmetric with -// the Update/write path's KK class-fix. +// the Update/write path's error mapping. func TestLoadOrCLIError_PermissionDenied(t *testing.T) { if os.Getuid() == 0 { t.Skip("running as root; chmod 0000 won't block reads") diff --git a/internal/config/repo.go b/internal/config/repo.go index 336fae2..a18442b 100644 --- a/internal/config/repo.go +++ b/internal/config/repo.go @@ -36,7 +36,7 @@ func LoadRepoOverlay(start, boundary string) (*RepoOverlay, error) { if err == nil { var o RepoOverlay if err := json.Unmarshal(b, &o); err != nil { - // Round 8 HH: wrap the path in so callers can produce + // Wrap the path in so callers can produce // actionable error messages ("fix the JSON at "). return nil, fmt.Errorf("%s: %w", candidate, err) } diff --git a/internal/config/resolve.go b/internal/config/resolve.go index 57cb9af..ad313d3 100644 --- a/internal/config/resolve.go +++ b/internal/config/resolve.go @@ -67,9 +67,9 @@ func ProfileName(flagProfile, envProfile string, overlay *RepoOverlay, cfg *Conf // // Errors: // - FlagProfile or EnvProfile names a profile that doesn't exist in -// opts.Config (Round 5 Adv-2 + Round 7 EE). +// opts.Config. // - Any credential/host value (flag/env/overlay/profile) fails its -// validator. v1.0.4 Class 1 closed three gaps in Round 8's FF+GG: +// validator. v1.0.4 closed three remaining gaps: // RepoOverlay.APIHost/APISecret were consumed verbatim, FlagAPISecret // relied on upstream auth-command validation only, and profile values // loaded from disk bypassed every write-time gate. @@ -150,7 +150,7 @@ func Resolve(opts ResolveOptions) (*Resolved, error) { var profile Profile if opts.Config != nil { p, ok := opts.Config.Profiles[r.Profile] - // Round 5 Adv-2: error symmetrically when EnvProfile names a + // Error symmetrically when EnvProfile names a // non-existent profile. Before the fix, only --profile rejected // unknown names; URLBOX_PROFILE silently fell through to env / // flag credentials, leaking the wrong profile's behaviour. @@ -170,7 +170,7 @@ func Resolve(opts ResolveOptions) (*Resolved, error) { } profile = p - // v1.0.4 Class 1 — defense-in-depth: profile values were validated + // Defense-in-depth: profile values were validated // at write time (config set, profile create), but a manually-edited // ~/.config/urlbox/config.json bypasses every write-time gate. // Validate on read so the read path is the same single chokepoint diff --git a/internal/config/resolve_test.go b/internal/config/resolve_test.go index 0fc60e7..5c7572a 100644 --- a/internal/config/resolve_test.go +++ b/internal/config/resolve_test.go @@ -142,7 +142,7 @@ func TestResolve_EnvProfile_PicksProfile(t *testing.T) { } } -// TestResolve_UnknownEnvProfile_Errors pins Round 5 Adv-2: when +// TestResolve_UnknownEnvProfile_Errors pins a regression: when // URLBOX_PROFILE names a profile that doesn't exist, Resolve must // return ErrNotFound rather than silently fall through to env/flag // credentials on the (empty) Profile struct — that fallthrough @@ -266,7 +266,7 @@ func TestResolve_APIKey_FromRepoOverlay(t *testing.T) { } } -// TestResolve_EnvAPISecret_InvalidUTF8_Rejected pins Round 8 FF: +// TestResolve_EnvAPISecret_InvalidUTF8_Rejected pins a regression: // URLBOX_API_SECRET used to bypass ValidateSecretValue entirely. The // adversarial demo: signing HMACs with control-char-corrupted env // bytes. Resolve now applies the same gate the flag/stdin/file paths @@ -342,10 +342,10 @@ func TestResolve_EnvAPISecret_Trimmed(t *testing.T) { } } -// ─── Class 1 (v1.0.4) ───────────────────────────────────────────── +// ─── value validation ───────────────────────────────────────────── // Invariant: every credential/host value reaching the API client has // been through ValidateSecretValue / ValidateAPIHost, regardless of -// source. Round 8 FF+GG closed env + flag paths but the overlay, +// source. Earlier fixes closed env + flag paths but the overlay, // FlagAPISecret, and profile-from-disk paths were unvalidated. // // These tests deliberately use cases that fail under the EXISTING diff --git a/internal/config/safe_write.go b/internal/config/safe_write.go index 7c43c63..eee0fe2 100644 --- a/internal/config/safe_write.go +++ b/internal/config/safe_write.go @@ -1,5 +1,3 @@ -// internal/config/safe_write.go — v1.0.4 Class 4. -// // SafeWriteUserFile is the single helper every CLI-initiated write to // a user-owned path goes through. The contract: // diff --git a/internal/config/safe_write_test.go b/internal/config/safe_write_test.go index fbaa1f6..98c380d 100644 --- a/internal/config/safe_write_test.go +++ b/internal/config/safe_write_test.go @@ -1,5 +1,3 @@ -// internal/config/safe_write_test.go — v1.0.4 Class 4. -// // Pins the SafeWriteUserFile contract: Lstat refuses symlinks (no // write-anywhere primitive), atomic rename (no half-written files // visible mid-write), refuses to clobber existing content without diff --git a/internal/config/secret_validate.go b/internal/config/secret_validate.go index 5eebd29..7c28977 100644 --- a/internal/config/secret_validate.go +++ b/internal/config/secret_validate.go @@ -1,17 +1,14 @@ -// internal/config/secret_validate.go — Round 8 FF: moved from +// internal/config/secret_validate.go — moved from // internal/cmd/secret_validate.go so config.Resolve can call it on the // URLBOX_API_SECRET env path. The cmd-side wrapper (still named // validateSecretValue in package cmd) is now a one-line forward to // config.ValidateSecretValue. // -// History: -// - Round 6 X: introduced as the single gate for direct-input secret -// paths (auth flag, stdin, file, config set api_secret, profile create). -// - Round 7 DD: extended to reject Unicode Cf "Format" chars -// (invisible: zero-width, BOM, bidi). -// - Round 8 FF: extended to reject invalid UTF-8 and Mn "Mark, -// Nonspacing" (combining marks, variation selectors) plus moved -// here so the URLBOX_API_SECRET env path also enforces the rule. +// The single gate for every secret input path (auth flag, stdin, file, +// config set api_secret, profile create, URLBOX_API_SECRET): rejects +// empty/whitespace, control chars, Unicode Cf "Format" chars (invisible: +// zero-width, BOM, bidi), Mn "Mark, Nonspacing" (combining marks, +// variation selectors), and invalid UTF-8. package config import ( diff --git a/internal/config/secret_validate_test.go b/internal/config/secret_validate_test.go index 87db872..c51d85c 100644 --- a/internal/config/secret_validate_test.go +++ b/internal/config/secret_validate_test.go @@ -1,5 +1,5 @@ -// internal/config/secret_validate_test.go — class-fix tests for secret-value -// validation. Round 6 surfaced four sibling bypasses of the secret guard: +// internal/config/secret_validate_test.go — regression tests for secret-value +// validation. Four sibling bypasses of the secret guard existed: // whitespace-only --api-secret accepted (auth + config set); control // chars accepted; config set api_secret "" silently cleared the secret // (bypassing the auth overwrite guard); leading/trailing whitespace @@ -96,12 +96,12 @@ func TestValidateSecretValue_TrimsSurroundingWhitespace(t *testing.T) { } } -// TestValidateSecretValue_RejectsInvisibleUnicode pins Round 7 (Med): +// TestValidateSecretValue_RejectsInvisibleUnicode pins a regression: // strings.TrimSpace handles Zs/Zl/Zp (NBSP, line/paragraph separators) but // NOT Cf "Format" characters — zero-width spaces, joiners, BOM, bidi // controls. These are invisible in terminals but persist verbatim in the // stored secret, causing auth to fail with mysterious 401s ("but I copied -// the right secret!"). The class-fix rejects every Cf rune anywhere in +// the right secret!"). The fix rejects every Cf rune anywhere in // the value. // // All invisible chars are spelled with \u escapes so the source file stays @@ -198,7 +198,7 @@ func TestValidateSecretValue_AcceptsNormalSecrets(t *testing.T) { } } -// TestValidateSecretValue_RejectsCombiningMarks pins Round 8 Class A: +// TestValidateSecretValue_RejectsCombiningMarks pins a regression: // Mn "Mark, Nonspacing" Unicode characters (combining marks like U+0300 // COMBINING GRAVE, variation selectors U+FE00..FE0F) are invisible by // design — they decorate the preceding char. Pasting "à" composed as @@ -234,7 +234,7 @@ func TestValidateSecretValue_RejectsCombiningMarks(t *testing.T) { } } -// TestValidateSecretValue_RejectsInvalidUTF8 pins Round 8 Class A: +// TestValidateSecretValue_RejectsInvalidUTF8 pins a regression: // invalid UTF-8 byte sequences — lone surrogates (U+D800..U+DFFF range // when encoded as raw bytes), overlong encodings (e.g. C0 80 for NUL), // bare 5th/6th-byte continuation forms (0xFF), truncated multi-byte diff --git a/internal/config/update.go b/internal/config/update.go index 6500446..7ea5202 100644 --- a/internal/config/update.go +++ b/internal/config/update.go @@ -32,9 +32,9 @@ const lockEmptyStaleAfter = 1 * time.Second // Update runs mutate against a freshly-loaded *Config and persists the // result, holding an exclusive file lock for the duration. The lock -// closes the read-modify-write race Round 7 Adv-3 exercised. +// closes the read-modify-write race that parallel profile creates exercised. // -// Round 8 KK: stale-lock recovery. The original O_EXCL sentinel was +// Stale-lock recovery. The original O_EXCL sentinel was // brittle to SIGKILL/SIGTERM (the deferred os.Remove never ran, leaving // a zero-byte .lock that wedged every subsequent write for 5s). Now // the lock file contains the holder's PID; on a contended acquire, we @@ -84,7 +84,7 @@ func withFileLock(path string, fn func() error) error { // acquireLock tries to create lockPath atomically. If a lock already // exists, it reads the PID inside, checks if that process is alive, -// and clobbers the file if not (stale-lock recovery — Round 8 KK). +// and clobbers the file if not (stale-lock recovery). // Returns a typed *output.CLIError so callers don't need to wrap. func acquireLock(lockPath string) error { deadline := time.Now().Add(lockAcquireTimeout) @@ -100,7 +100,7 @@ func acquireLock(lockPath string) error { // Permission denied, parent dir missing, etc. — local FS // problem, not a contention issue. Map to ErrForbidden so // the exit code reflects "you can't touch this", not "server - // problem" (Round 8 Adv-1 M1). + // problem". return output.NewCLIError( output.ErrForbidden, "could not create config lock: "+err.Error(), @@ -143,7 +143,7 @@ func tryCleanStaleLock(lockPath string) bool { if pidStr == "" { // Zero-byte lockfile — could be either: // (a) the previous writer SIGKILL'd before writing its PID, - // leaving the empty O_EXCL'd file forever (pre-KK bug) + // leaving the empty O_EXCL'd file forever (the old bug) // (b) the CURRENT writer just succeeded the O_EXCL Open and // hasn't called Write yet — millisecond race // Distinguish by file age: an empty file >1s old is definitely diff --git a/internal/config/update_test.go b/internal/config/update_test.go index 21d98eb..33b11c0 100644 --- a/internal/config/update_test.go +++ b/internal/config/update_test.go @@ -15,7 +15,7 @@ import ( "github.com/urlbox/urlbox-cli/internal/output" ) -// TestUpdate_ConcurrentProfileCreates_AllPersist pins Round 7 Adv-3 (High): +// TestUpdate_ConcurrentProfileCreates_AllPersist pins a regression: // 20 goroutines each calling config.Update to add a distinct profile must // all persist. Before the lock, the Load -> mutate -> Save sequence raced — // processes started from the same "before" state and silently overwrote @@ -80,7 +80,7 @@ func TestUpdate_MutateFnErrorPropagates(t *testing.T) { } } -// TestUpdate_StaleLock_AfterKill_SelfHeals pins Round 8 KK: the pre-KK +// TestUpdate_StaleLock_AfterKill_SelfHeals pins a regression: the previous // O_EXCL approach left a 0-byte .lock file after SIGKILL of the // previous writer, wedging every subsequent write for 5s. Now we // detect dead-PID/zero-byte lock files and clobber them on the spot. @@ -158,8 +158,8 @@ func TestUpdate_StaleLock_DeadPID_SelfHeals(t *testing.T) { } } -// TestUpdate_LockAcquireTimeout_ReturnsConflictNotServer pins Round 8 -// KK / Adv-1 M1: local-IO / lock-contention errors used to surface as +// TestUpdate_LockAcquireTimeout_ReturnsConflictNotServer pins a regression: +// local-IO / lock-contention errors used to surface as // code:"server" (exit 10), which the contract reserves for upstream // server problems. Now they're code:"conflict" (exit 7). func TestUpdate_LockAcquireTimeout_ReturnsConflictNotServer(t *testing.T) { diff --git a/internal/output/envelope.go b/internal/output/envelope.go index be13d54..2fb9929 100644 --- a/internal/output/envelope.go +++ b/internal/output/envelope.go @@ -16,7 +16,7 @@ type Envelope struct { Breadcrumbs []Breadcrumb `json:"breadcrumbs,omitempty"` // Warnings carries non-fatal advisories agents should surface but // shouldn't fail on (e.g. "unknown option 'fromat' — did you mean - // 'format'?"). v1.0.4 Class 3 — pre-1.0.4 these were emitted as + // 'format'?"). Pre-1.0.4 these were emitted as // plain stderr text alongside the JSON envelope on stdout, breaking // agents that read either stream alone. Now they ride inside the // envelope for json/quiet modes; text mode still prints them inline diff --git a/internal/output/envelope_test.go b/internal/output/envelope_test.go index 2a5385e..b6ededa 100644 --- a/internal/output/envelope_test.go +++ b/internal/output/envelope_test.go @@ -114,7 +114,7 @@ func containsKey(jsonStr, key string) bool { return ok } -// ─── v1.0.4 Class 3.1 — Warnings on envelopes ────────────────────── +// ─── Warnings on envelopes ────────────────────── // Invariant: agent-consumable warnings (fuzzy-typo hints, --json key // suggestions) travel inside the envelope as a structured field, not // as plain stderr text alongside the JSON envelope on stdout. diff --git a/internal/output/jq.go b/internal/output/jq.go index fa4fb1c..202d747 100644 --- a/internal/output/jq.go +++ b/internal/output/jq.go @@ -73,8 +73,8 @@ func WriteEnvelopeWithJQ(w io.Writer, env *Envelope, jqExpr string, quiet bool) return err } -// WriteErrorEnvelopeWithJQ is the error-envelope analogue. Round 8 OO: -// before this commit, --jq was only applied to success envelopes — +// WriteErrorEnvelopeWithJQ is the error-envelope analogue. +// Before this commit, --jq was only applied to success envelopes — // error envelopes always printed the full structure regardless of // --jq. Agents that did `urlbox … --jq '.code'` expecting a single // code string got the whole envelope on the failure path. diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index 46be111..00f01ad 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -90,3 +90,22 @@ func TypeToConfirm(title, expected string) error { } return nil } + +// TextInput draws a single-line text input to stderr and returns the +// trimmed value. validate (optional) runs inside the form so the user can +// correct in place. It returns ErrNotInteractive when stdin is not a +// terminal. +func TextInput(title string, validate func(string) error) (string, error) { + if !term.IsTerminal(int(os.Stdin.Fd())) { //nolint:gosec // file descriptors fit in int on every platform Go supports + return "", ErrNotInteractive + } + var typed string + input := huh.NewInput().Title(title).Value(&typed) + if validate != nil { + input = input.Validate(validate) + } + if err := input.WithTheme(theme()).Run(); err != nil { + return "", err + } + return strings.TrimSpace(typed), nil +} diff --git a/internal/prompt/textinput_test.go b/internal/prompt/textinput_test.go new file mode 100644 index 0000000..1543ab1 --- /dev/null +++ b/internal/prompt/textinput_test.go @@ -0,0 +1,13 @@ +package prompt + +import ( + "errors" + "testing" +) + +func TestTextInput_NonInteractive_ReturnsErrNotInteractive(t *testing.T) { + _, err := TextInput("Add a short description:", nil) + if !errors.Is(err, ErrNotInteractive) { + t.Fatalf("want ErrNotInteractive, got %v", err) + } +} diff --git a/internal/surface/snapshot.go b/internal/surface/snapshot.go index c2a9ca7..05d87ec 100644 --- a/internal/surface/snapshot.go +++ b/internal/surface/snapshot.go @@ -1,7 +1,7 @@ // Package surface generates a deterministic snapshot of the CLI's command and flag surface. // The snapshot is committed to SURFACE.txt and checked in CI to prevent silent breaking changes. // -// Exclusion rule (v1.0.4 Class 6 — documented explicitly via Header()): +// Exclusion rule (documented explicitly via Header()): // - Cobra builtins (`help` subcommand, `--help` / `--version`) are // skipped. They're stable framework-level surfaces we don't own // and can't break; tracking them adds noise without a guarantee. diff --git a/internal/surface/snapshot_test.go b/internal/surface/snapshot_test.go index 33b3468..58499a9 100644 --- a/internal/surface/snapshot_test.go +++ b/internal/surface/snapshot_test.go @@ -64,7 +64,7 @@ func TestSnapshot_Sorted(t *testing.T) { } } -// ─── v1.0.4 Class 6 — explicit exclusion-rule header ──────────────── +// ─── explicit exclusion-rule header ──────────────── // // Invariant: the surface contract documents what it covers AND what it // deliberately excludes, so a reader of SURFACE.txt knows the bounds diff --git a/internal/validation/validate.go b/internal/validation/validate.go index afe2158..ba63700 100644 --- a/internal/validation/validate.go +++ b/internal/validation/validate.go @@ -96,7 +96,7 @@ func loadSchemaFrom(b []byte) (*jsonschema.Schema, []string, error) { // // Returns (payload, warnings, nil) on success. Returns (nil, nil, *CLIError) // for the local hard errors above. Warnings are owned by the caller — no -// package-global state is mutated (Arch I3 from Round 1 review). +// package-global state is mutated. func ValidatePayload(b []byte) (payload map[string]any, warnings []string, cliErr *output.CLIError) { if err := SanitizeRaw(b); err != nil { return nil, nil, err diff --git a/internal/validation/validate_test.go b/internal/validation/validate_test.go index a1f5f73..edcb48d 100644 --- a/internal/validation/validate_test.go +++ b/internal/validation/validate_test.go @@ -123,8 +123,7 @@ func TestValidatePayload_RejectsURLControlChars(t *testing.T) { // refactor and the field list drifted). v0.9.0's "schema as documentation" // model handles this gracefully — unknown fields pass through verbatim and the // API decides. This test guarantees the unknown-passthrough path stays open -// for these dashboard-supported fields. See urlbox-mono -// packages/types/src/render/render.types.ts:1277-1292 for API definitions. +// for these dashboard-supported fields. func TestValidatePayload_AcceptsUnknownFields_WithWarning(t *testing.T) { payload := []byte(`{ "url": "https://example.com", @@ -198,7 +197,7 @@ func TestValidatePayload_KnownKey_BadType_PassesThrough(t *testing.T) { } } -// TestValidatePayload_Concurrent_NoCrossPollination is the Arch I3 guard: +// TestValidatePayload_Concurrent_NoCrossPollination is the no-global-state guard: // the package no longer holds a global lastWarnings slice, so concurrent // callers must each see their own warning set without race / clobber. // Run with -race to make this assertion meaningful. diff --git a/schema/schema.go b/schema/schema.go index 8076f60..792ea4b 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -4,8 +4,8 @@ package schema import _ "embed" // RenderJSON is the embedded JSON Schema describing the render request payload. -// Sourced from urlbox-mono/packages/types/src/render/render.types.ts and synced -// via the .github/workflows/sync-cli-schema.yml workflow in the monorepo. +// Generated from the Urlbox API's render option types and kept in sync +// automatically. // //go:embed render.json var RenderJSON []byte diff --git a/skills/SKILL.md b/skills/SKILL.md index f8eb4e0..b71fc0c 100644 --- a/skills/SKILL.md +++ b/skills/SKILL.md @@ -161,6 +161,7 @@ documents the well-known options, but the API accepts more. | `urlbox config profile default` | Switch the default profile | | `urlbox config profile delete` | Delete a non-default profile | | `urlbox dashboard` | Open the Urlbox dashboard in the user's browser | +| `urlbox support` | Open the Urlbox support contact page in the user's browser | | `urlbox doctor` | Diagnose install, config, network, credentials | | `urlbox link` | Generate an HMAC-signed render URL with no API call | | `urlbox login` | Browser sign-in (agents: use `URLBOX_API_SECRET` or `config profile create`) | @@ -177,6 +178,7 @@ documents the well-known options, but the API accepts more. | `urlbox screenshot ` | Alias for `render --format png` (also `urlbox shot`) | | `urlbox pdf ` | Alias for `render --format pdf --full-page` | | `urlbox video ` | Alias for `render --format mp4` | +| `urlbox report ` | Report a bad render to the Urlbox team (category + comment) | | `urlbox schema render` | Print the JSON Schema for the render request payload | | `urlbox skill` | Show this skill content (`urlbox skill show`) | | `urlbox status ` | Check / poll the status of an async render | @@ -430,6 +432,37 @@ urlbox dashboard --output-format json --jq '.data.url' Exit codes: 0 on success (browser launched or URL printed); 10 if the OS browser handler returned an error (URL is in the hint). +## report: report a bad render + +`urlbox report ` files a render report with the Urlbox team. +Requires a signed-in session — the HUMAN runs `urlbox login` (browser +device flow) once; agents never sign in themselves and the render +secret (`URLBOX_API_SECRET`) cannot file reports. + +Non-interactive use requires both flags; the command never prompts when +stdin is not a terminal: + +```sh +urlbox report 01a0906a-…_ps --category bot-detection --comment "page shows a captcha" --output-format json --jq '.data.id' +``` + +`--category` is a closed set: `bot-detection`, `login-required`, +`missing-content`, `cookie-banner-or-popup`, `render-failed`, `other`. +Where the renderId comes from: async render responses carry +`data.renderId`; sync render JSON carries NO id (use `--async` when you +plan to report). One report per render — re-filing while the report is +open edits it in place; once the team picks it up you get exit 7. + +Exit codes: 0 filed; 1 missing/invalid category or comment; 3 not +logged in; 5 render not found (expired ~60 days or another org's); +7 report locked (already in review). + +## support: open the support contact page + +`urlbox support` opens https://urlbox.com/contact. Same envelope contract +as `dashboard`: json/quiet never launch a browser and always carry +`data.url`; headless prints the URL to stderr. + ## Common workflows ```sh