fix(attachments): download real bytes and hint --type for bucket-scoped recordings

[nnemirovsky]

Two related fixes so basecamp attachments download <card> and basecamp cards show --download-attachments=... actually retrieve the file bytes for non-image attachments on Kanban cards.

Repro

Attach a non-image file (e.g. a small report.csv) to a card's description, then try to download:

$ basecamp cards show <card-id> --download-attachments=/tmp/x --json
# "succeeds" but writes a ~3.8 KB green SVG file-type icon to /tmp/x/report.csv
# instead of the real CSV bytes

$ basecamp attachments download <card-id>
# {"ok":false,"error":"Resource not found: https://3.basecampapi.com/<account>/recordings/<card-id>.json","code":"not_found"}

Image attachments worked by coincidence, which is why this went unnoticed.

Root causes

1. ParsedAttachment.DisplayURL() returned the preview URL instead of the download href.
Every <bc-attachment> element carries both a url (preview.3.basecamp.com/.../previews/full) and an href (storage.3.basecamp.com/.../download/<filename>). DisplayURL() preferred url. For non-image content types the preview endpoint returns a generic SVG file-type icon rather than the real blob, so every caller downstream (cards show --download-attachments, attachments list, attachments download, downloadableAttachments) silently got the wrong bytes with the right filename.

Fix: DisplayURL() now prefers Href and falls back to URL. All internal callers are download paths, so the flip is safe.

2. fetchItemContent returned a bare "Resource not found" for cards.
The generic /recordings/<id>.json endpoint is not addressable at the account scope for Kanban::Card — the recording is only reachable via the bucket-scoped /buckets/<bucket>/card_tables/cards/<id>.json. The function already has a friendly "Specify a type" usage hint for the 204 branch but routed 404s through convertSDKError, hiding the real cause.

Fix: on 404 from the generic path, when no --type was provided, emit the same usage hint. Narrow — only fires when isGenericType(recordType) and the SDK error is CodeNotFound. Any path that already worked is untouched. The hint string is now a package const shared between the 204 and 404 branches so they cannot drift.

Test coverage gap this exposed

internal/richtext/richtext_test.go had a "URL wins" case, and internal/commands/attachments_test.go's TestAttachmentMeta "uses DisplayURL" asserted the same. Both pinned the buggy behavior. Updated to assert Href wins over preview URL, with a separate case for the URL-only fallback path.

Added TestAttachmentsList404GenericPathSuggestsType covering the 404 → --type hint conversion, mirroring the existing TestFetchItemContentRefetchesLineWithLargeParentID fixture style.

Commits

1. fix(richtext): prefer Href over preview URL in DisplayURL
2. fix(attachments): suggest --type on 404 from generic recording lookup

Local verification

• make fmt-check clean
• go vet ./... clean
• make lint (golangci-lint) 0 issues
• go test ./... all packages green

---

Summary by cubic

Fixes attachment downloads to return the actual file bytes and adds a --type hint for bucket‑scoped recordings (e.g., cards). Impacts basecamp cards show --download-attachments, basecamp attachments list, and basecamp attachments download.

• Bug Fixes
  • Prefer Href (download link) over preview URL in ParsedAttachment.DisplayURL() so non-image attachments fetch the real file; URL remains a fallback.
  • On 404 from /recordings/<id>.json with no --type, show a "Specify a type" hint to use the typed endpoint (needed for cards and similar); do not show the hint if a type was provided.

^{Written for commit a284543. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 4 files

[Copilot]

Pull request overview

Fixes attachment downloading for non-image files by ensuring the CLI uses the real download URL (blob) instead of the preview URL, and improves the error experience when generic recording lookups can’t resolve bucket-scoped items (notably Kanban cards).

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

• Update ParsedAttachment.DisplayURL() to prefer Href (download endpoint) over URL (preview endpoint).
• Adjust fetchItemContent to surface a --type usage hint when the generic /recordings/<id>.json lookup returns 404.
• Update/add tests to lock in Href preference and the new 404-to-hint behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
internal/richtext/richtext.go	Prefer downloadable Href over preview URL for attachment URLs.
internal/richtext/richtext_test.go	Update DisplayURL expectations (Href wins; URL fallback).
internal/commands/attachments.go	Add 404 handling on generic lookup and share the hint string via a const.
internal/commands/attachments_test.go	Update attachment meta expectations and add test for 404 generic lookup hinting.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[robzolkos]

Looks good to me. I rebased this locally onto current origin/main, reviewed the changed attachment paths, and live-tested against a Basecamp 5 test account.

Live verification:

• Created a PDF attachment on a comment.
  • Current origin/main downloaded basecamp-cli-447-20260629T173513Z.pdf from preview.app.basecamp.com/.../previews/full; the saved file was actually PNG image data.
  • This PR downloaded the same attachment from storage.app.basecamp.com/.../download/...; the saved file was a valid PDF.
• Created a PDF attachment on a card.
  • Current origin/main again downloaded the preview PNG while saving it with the .pdf filename.
  • This PR downloaded the real PDF bytes.
• Checked the generic card-ID path.
  • Current origin/main returns a raw /recordings/<id>.json 404.
  • This PR returns a useful --type/URL hint.

Targeted tests pass locally:

GOWORK=off go test ./internal/richtext ./internal/commands

I noticed one existing non-blocking oddity: cards can expose both a sparse attachment entry and a hydrated attachment entry for the same file, so the download output may include one skipped entry plus one downloaded entry. That predates this PR and the PR still downloads the correct file.

Given the live repro and fix, this is ready to merge.