PR Review — Go CLI and REST contract (pass 2)

Diff reviewed: git diff origin/main..HEAD in the worktree. Multi-persona union below. Pass-1 High/Medium findings (upload content-type, annotation-list aggregation/limit/offset/total, N+1 in annotation detail, dead code) are all fixed in the current tree and were re-verified against internal/cli/annotation.go, internal/cli/screenshot.go, and app/services/api/v1/project_scope.rb. No plan-required (Requirements Trace A1–A8) command or endpoint is missing.

High

(none) — all must-have commands, REST endpoints, tests, CI, GoReleaser config, and docs required by the plan are present; prior High findings were resolved.

Medium

[x] AUTO-FIX: [testing] The project-wide annotation list aggregate path — the most complex client logic (allScreenshots, allAnnotations, pageAnnotations, cloneValues in internal/cli/annotation.go:104-218) — has zero test coverage; only the --screenshot passthrough (TestAnnotationListPath) is exercised: multi-screenshot pagination, in-memory limit/offset windowing, true-total reporting, and per-screenshot error-skip are unverified, yet these are exactly the behaviors pass 1 required and the plan (Unit 4) asks command tests to cover. >

Nit

[ ] ESCALATE: [security] screenote config prints the resolved api_key in plaintext to stdout (internal/cli/config.go → writeJSON(a.stdout, resolved); Values.APIKey serializes as api_key), a CI-log leak footgun; consider masking unless an explicit opt-in flag is passed (carryover from pass 1, still unresolved — user product decision). >
[x] RESOLVED/NO-FIX: [consistency] annotation list output fidelity differs by path: the aggregate path re-encodes through the Go Annotation struct (writeJSON), so any server field not modeled in the struct is silently dropped, while the --screenshot path passes raw server JSON through (writeRawJSON). Current fields line up, but the two code paths can drift. >
[x] RESOLVED/NO-FIX: [maintainability] ContractSerializer.annotation_detail (app/serializers/api/v1/contract_serializer.rb:345-356) hardcodes mime_type: "image/png" even when cropped_image_base64 is nil (crop failed/missing), so the field misrepresents an absent crop. >
[x] AUTO-FIX: [type-design] Unused response types in internal/screenote/types.go — PagesResponse, ScreenshotCreateResponse, and CommentCreateResponse are declared but never referenced (all their commands use writeRawJSON); dead declarations. >
[x] AUTO-FIX: [type-design] Client.doJSON (internal/screenote/client.go:1350) models a single optional request body as a variadic body ...io.Reader; a plain io.Reader (nil when absent) would be more idiomatic and prevents callers passing multiple readers. >
[x] AUTO-FIX: [release] .goreleaser.yml uses archives[].builds, the key deprecated in GoReleaser v2 (renamed to ids); it still works but may warn now and break on a future upgrade. >
[x] RESOLVED/NO-FIX: [release] No CI or release workflow invokes GoReleaser on tag; go install works, but GitHub Release archives/checksums require a manual goreleaser release run (plan Unit 6 only requires the config + a next-stage dry-run, so low priority). >
[x] RESOLVED/NO-FIX: [maintainability] The --interactive persistent flag (internal/cli/root.go:677) is wired but never read; no interactive prompting exists, so the flag is inert (carryover; plan A6 defines the interactivity contract and v1 has no prompts, so a placeholder is plan-consistent — noted for completeness only). >