2 Commits

Author SHA1 Message Date
xarmian 756d91acad fix(ci): gofmt + bump race-detector timeout to 30m (#299)
CI on main has been failing since the PLAN-866 attachment work
landed. Two independent issues:

1. gofmt failures (golangci-lint) — seven files in the attachments
   path had trailing-comment alignment that gofmt wanted nudged a
   column. Pure whitespace; ran `gofmt -w` across the affected
   files. golangci-lint's gofmt linter caught it on every PR /
   push since TASK-870 but we hadn't been watching those signals.

   Files cleaned: internal/attachments/{fs_store_test,mime,
   mime_test,processor_test}.go, internal/server/{
   handlers_attachments_download_test,handlers_attachments_transform,
   render/attachments_test}.go.

   Local guard: `gofmt -l ./...` now exits clean.

2. Race-detector tests timed out at 20m on the GitHub-hosted runner.
   Two contributors:
     - PostgreSQL adds latency on every CREATE/DROP plus on the
       bcrypt hash inside auth/bootstrap (~3s per call under -race
       on the runner). Tests that bootstrap a fresh user (e.g.
       TestSessionIPChange_*) pay the full cost each time.
     - The PLAN-866 image-processing tests (thumbnail derivation,
       rotate / crop transform) added ~2-3 minutes of decode/encode
       work on top of the existing suite.

   The previous "20m gives margin without papering over a hang"
   comment was right at the time it was written; we now genuinely
   need more headroom. Bumped to 30m on both the SQLite and
   PostgreSQL race steps. Genuine deadlocks would still trip this
   and produce the goroutine-dump panic — we just stop confusing
   "slow but progressing" with "permanently hung".

   Reference points before / after:
     - TASK-875 main run #294: Go (PostgreSQL) finished in 17m48s ✓
     - TASK-880 main run #298: Go (PostgreSQL) hit 20m timeout ✗
     - Local: my new tests under -race add ~63s on a developer laptop
       (TestThumbnails + TestTransform + TestProcessor combined).

Verification:
  go test ./...             — pass
  go vet ./...              — clean
  gofmt -l (recursively)    — clean
2026-04-29 15:41:12 -04:00
xarmian 5af54ddc05 feat(attachments): markdown reference resolver for pad-attachment:UUID (TASK-874) (#291)
* feat(attachments): markdown reference resolver for pad-attachment:UUID (TASK-874)

Add the shared step that translates `pad-attachment:UUID` markdown
references into rendered HTML for image embeds, file chips, and missing
placeholders. Wired into the editor preview path; Go-side helpers seed
the future server-side rendering pipeline (export / shared item view).

TS side (`web/src/lib/markdown/attachments.ts`):
  - Pure helpers: parseAttachmentHref, attachmentDownloadUrl, isImageMime,
    formatAttachmentSize, renderAttachmentImage/Chip/Missing
  - resolveAttachmentImage / resolveAttachmentLink for the marked hooks
  - Image MIME → <img src=...?variant=thumb-md data-attachment-id=...>
  - Non-image MIME (or link syntax) → file chip with download attribute
  - Missing/deleted → "Missing attachment" placeholder span

`web/src/lib/utils/markdown.ts`:
  - renderer.image override (defaulting to marked's standard image when
    href is not pad-attachment:)
  - renderer.link checks for pad-attachment: prefix before the existing
    external/internal-link logic
  - renderMarkdown gains an optional attachmentResolver parameter; the
    resolver is threaded via a per-call module slot (synchronous render)
  - DOMPurify allowlist extended with data-attachment-id, download,
    width, height — ALLOW_DATA_ATTR stays false so only this single
    data-* attribute slips through

Go side (`internal/server/render/attachments.go`):
  - Mirror of the TS API so server-rendered output matches client output
    byte-for-byte for the same input
  - ResolveAttachmentReferences scans markdown source via regex,
    skipping fenced code blocks (backtick + tilde), substitutes both
    image and link forms
  - Comprehensive table-driven tests (24 cases) covering: href parsing,
    URL building, MIME detection, size formatting, image/chip/missing
    rendering, escape safety against script-tag injection in alt /
    filename / display text, fenced-code skip, tilde fences, title
    suffix on link destinations, nil resolver pass-through, no false
    positives on non-attachment URLs, deterministic round-trip

References are stored as opaque `pad-attachment:UUID` so a backend
migration (FS → S3) can rewrite storage_keys without touching item
content. See DOC-865 for the architecture.

Parent: PLAN-866 (Attachments Phase 1).

* fix(attachments): chip label double-escape + escaped-bracket lockstep per Codex review (round 1)

Two findings from the round-1 Codex review:

1. TS chip labels were double-escaped. renderer.link was passing the
   parseInline(tokens) HTML output to resolveAttachmentLink, which feeds
   it into renderAttachmentChip → escapeHtml. A label like
   `[**Report**](pad-attachment:id)` rendered literal
   `&lt;strong&gt;Report&lt;/strong&gt;` instead of plain text. Switched
   to the link token's raw `text` field; markdown emphasis inside chip
   labels now degrades to literal markers (acceptable for filename-style
   labels) and matches what the Go regex extracts.

2. Go regex didn't accept CommonMark `\]` / `\\` escapes inside link/image
   labels, so `[Q1 \] report](pad-attachment:id)` resolved on the TS side
   (marked handles escapes) but stayed literal on the Go side — breaking
   the documented lock-step contract. Updated the regex to accept escaped
   characters inside the alt/text capture, and added unescapeMarkdownText
   to mirror marked's behavior of dropping the backslash before the label
   reaches the render helpers.

Tests added: TestResolveAttachmentReferences_EscapedBrackets covers
image alt, link text, and combined backslash/bracket escapes;
TestUnescapeMarkdownText is the unit-level table for the unescape
helper (including dangling-backslash and non-punctuation pass-through).
2026-04-29 13:09:42 -04:00