Skip to content

Attachments Decisions

Distilled from 9 DECISIONS.md files (KD-0254 had no DECISIONS.md — design refactor with no rejected alternatives). Each entry is a real fork in the road — what we chose, what we passed on, and why. Implementation details that aren't a tradeoff are NOT here.


Multi-upload mechanism: parallel frontend calls, no batch endpoint

  • Chose: Frontend fires N parallel POSTs and aggregates with Promise.allSettled.
  • Rejected: New backend endpoint accepting an array of files.
  • Why: Backend stays unchanged, granular per-file error handling, existing tests stay valid; multi-request cost is negligible at 2-5 files.
  • Source: KD-0183

Multi-upload toast strategy: single aggregate, not per-file

  • Chose: One toast summarizing all/partial/none-succeeded after all uploads settle.
  • Rejected: One toast per uploaded file.
  • Why: Per-file would spam the UI and queue beyond the 4-toast concurrent limit.
  • Source: KD-0183

File composables: change return type to File[]

  • Chose: useFileInput and useFileDrop return arrays; all callers updated.
  • Rejected: New useMultiFileInput / useMultiFileDrop composables alongside the existing ones.
  • Why: Two parallel APIs would drift; natural evolution to support both single and multi via the same hook.
  • Source: KD-0183

Attachment count cap: frontend-only at 10 per entity

  • Chose: Frontend enforces MAX_ATTACHMENTS_PER_ENTITY = 10 with truncation + warning.
  • Rejected: Backend count validation, or both belt-and-suspenders.
  • Why: Internal tool; bypassing via curl is a non-threat at current user base; backend can be added later without frontend changes.
  • Source: KD-0183

Bulk delete: backend endpoint, not frontend loop

  • Chose: New DELETE endpoint with atomic transaction.
  • Rejected: Frontend iterating individual deletes.
  • Why: Atomicity matters for destructive bulk; partial failure would leave inconsistent state.
  • Source: KD-0236

Bulk-delete authorization: defer permission model to a separate issue

  • Chose: deleteAll policy method just checks Member+ role.
  • Rejected: Per-attachment ownership checks for bulk delete.
  • Why: The create policy is also Member+ without parent-entity ownership — fixing the broader permission model is out of scope; per-row checks here would be inconsistent.
  • Source: KD-0236

Bulk-delete UI: actions slot on SectionHeader

  • Chose: Add a named actions slot to the shared SectionHeader.
  • Rejected: Custom inline header replacing SectionHeader inside AttachmentSection.
  • Why: Other sections might gain action buttons; minimal change to a simple shared component.
  • Source: KD-0236

Avatar format pipeline: AVIF + WebP, drop JPEG fallback

  • Chose: Two formats per avatar served via <picture>.
  • Rejected: AVIF + WebP + JPEG (3 formats), or WebP-only.
  • Why: AVIF support is ~95% in 2026; WebP covers the remaining ~5%; the 3rd format is wasted storage and signed URLs.
  • Source: KD-0240

Client-side avatar compression: yes, despite issue scope excluding it

  • Chose: browser-image-compression to ~200KB / 512x512 WebP before upload.
  • Rejected: Server-only compression (the issue's original scope).
  • Why: 5MB photo → 200KB = 96% bandwidth reduction; 8s mobile upload → 0.3s; server still re-validates and re-processes.
  • Source: KD-0240

Avatar storage column: base name, derive paths

  • Chose: Strip extension; store abc123; derive images/abc123.avif / .webp at read time.
  • Rejected: JSON column with both paths, or separate columns per format.
  • Why: Single column, clean derivation, one-time migration to strip existing extensions.
  • Source: KD-0240

Avatar resolution: single 256×256, not multi-resolution

  • Chose: One size × two formats = 2 files per user.
  • Rejected: Multiple resolutions (64/128/256) × two formats = 6 files.
  • Why: Browser downscaling from 256px is imperceptible at 22-96px display sizes; 6 files per user is unjustified pixel-perfectionism.
  • Source: KD-0240

Backend image library: intervention/image v3, not raw GD

  • Chose: Add intervention/image ^3 with GD driver.
  • Rejected: Raw GD function calls (imagecreatefromjpeg(), imageavif(), etc.).
  • Why: Fluent API significantly cleaner for the resize → crop → multi-format encode pipeline.
  • Source: KD-0240

Avatar processing: synchronous, not queued

  • Chose: Process during the upload request.
  • Rejected: Queue job that serves original until processing finishes.
  • Why: ~150ms total for 256×256 × 2 formats; queue infra adds complexity, monitoring, and temporary inconsistency for no real benefit.
  • Source: KD-0240

Race condition fix: disable button, not AbortController

  • Chose: Track isUploading; hide the Clear-files button while active.
  • Rejected: Cancel in-flight uploads via AbortController, or catch-and-ignore upload errors.
  • Why: Simplest solution; cancellation semantics are unclear for partial uploads; ignoring errors masks legitimate failures.
  • Source: KD-0329

Single-file upload error path: try/catch + dangerToast

  • Chose: Wrap upload in try/catch, dangerToast on failure, finally resets state.
  • Rejected: Lean on the global Axios interceptor alone.
  • Why: Interceptors re-throw after handling — without component try/catch the rejection is uncaught; component catch also provides a fallback message when no message field is returned.
  • Source: KD-0329

Attachment compression: separate ProcessAttachmentImageAction

  • Chose: New action mirroring ProcessAvatarAction.
  • Rejected: Inline compression inside CreateAttachmentAction.
  • Why: Single responsibility; compression independently testable, persistence concerns stay clean.
  • Source: KD-0358

Filename after compression: update extension to .webp

  • Chose: DB filename matches actual content (photo.webp).
  • Rejected: Preserve user's original filename (photo.png despite WebP content).
  • Why: Mismatched extension/mime/content invites confusion; consistency wins.
  • Source: KD-0358

Re-encode policy: always WebP, even small images

  • Chose: All uploaded images become WebP regardless of dimensions.
  • Rejected: Skip downscale for small images, leaving them in original format.
  • Why: Uniform format on disk simplifies future Extract agent and yields file-size savings on already-small images too.
  • Source: KD-0358

Animated GIFs: detect and skip

  • Chose: Use Intervention's count() method; static GIFs compress, animated stay as-is.
  • Rejected: Compress all GIFs (lose animation), or skip all GIFs (miss static optimization).
  • Why: Preserves user content while still optimizing the common case.
  • Source: KD-0358

Compression rollout: new uploads only, no backfill

  • Chose: Existing attachments untouched.
  • Rejected: Artisan backfill command processing existing images.
  • Why: Modifying production files carries migration risk for unclear gain; mixed formats on disk are fine.
  • Source: KD-0358

Issue-form attachments: nullable attachable, not pending-uploads table

  • Chose: Make attachable_type/attachable_id nullable; uploads create real Attachment rows; re-parent on issue creation.
  • Rejected: Separate pending_uploads table, or browser-only-until-submit.
  • Why: Reuses existing Attachment model/storage/actions; orphan detection is WHERE attachable_id IS NULL; in-memory until submit blocks upload-during-form-fill UX.
  • Source: KD-0360

Orphan upload section: own collapsible, not inside AI section

  • Chose: New "Attachments" SectionLabel between AI Generation and the form.
  • Rejected: Embedding under AiStoryPrompt, or inside IssueForm advanced fields.
  • Why: Attachments are useful without AI feature flag; embedding under AI hides them; embedding in IssueForm mixes file uploads with metadata fields.
  • Source: KD-0360

Orphan upload endpoint: methods on AttachmentController, not new controller

  • Chose: storeOrphan() / destroyOrphan() on existing AttachmentController.
  • Rejected: Separate PendingAttachmentController with own routes.
  • Why: Logic is identical to existing upload minus parent; fewer files to maintain.
  • Source: KD-0360

Orphan TTL: 24 hours

  • Chose: Daily scheduled cleanup of orphans older than 24h.
  • Rejected: 1-hour aggressive, or 48-hour weekend-friendly.
  • Why: Generous enough to leave a tab overnight; short enough to prevent storage bloat.
  • Source: KD-0360
  • Chose: onBeforeRouteLeave + beforeunload warning; on confirmed leave, batch-delete orphan IDs immediately.
  • Rejected: Warning only (let cron clean up), or no warning at all.
  • Why: Clean UX with immediate cleanup; cron stays as fallback for browser crashes.
  • Source: KD-0360

Image paste ownership: extension factory in tenant layer

  • Chose: imagePasteExtension(upload) factory passed via existing extensions prop.
  • Rejected: Add upload/allowAttachments props to shared RichTextArea.
  • Why: app-boundaries.spec.ts enforces shared files stay domain-agnostic; tenant-only attachment concerns mustn't leak into shared.
  • Source: KD-0394

Pasted image rendering: inline node, not attachment-only

  • Chose: Insert TipTap Image node at cursor; depend on @tiptap/extension-image.
  • Rejected: Upload silently to attachment panel without modifying editor content.
  • Why: User explicitly requested Google Docs-style inline behaviour; AC#4 says "displayed in the editor", not "displayed below it".
  • Source: KD-0394

Upload return type: store method uploadForPaste(file): Promise<string>

  • Chose: New store method returning the download URL.
  • Rejected: Local wrapper in each consuming page constructing URL from response id + base URL.
  • Why: URL pattern ${baseUrl}/${id}/download is internal store detail; pages shouldn't construct it across three call sites.
  • Source: KD-0394

Toast feedback location: caller-supplied wrapper around upload

  • Chose: Pages wrap uploadForPaste with toast calls; the paste extension catches thrown errors.
  • Rejected: Toast calls inside the shared paste extension or inside RichTextArea.
  • Why: RichTextArea is in shared/ and cannot import @tenant/services/toast; wrapper pattern keeps Dutch toast strings co-located with other tenant copy.
  • Source: KD-0394

Paste-orphan persistence: re-parent on save (D7 reverses D5)

  • Chose: Project-level orphan upload at paste, re-parented + URL-rewritten by ReparentPasteOrphanAttachmentsAction on save.
  • Rejected: Leave paste-uploaded orphans untouched forever, hoping the re-parent step would break the markdown URL.
  • Why: D5's "no orphan cleanup exists" assumption was wrong — PruneOrphanedAttachmentsCommand already deletes orphans after 24h, making every Create-page paste a 24-hour bomb.
  • Source: KD-0394

Re-parent concurrency safety: four-condition guard + lockForUpdate

  • Chose: Re-parent only if orphan is still null, same project, same uploader, AND URL appears in description.
  • Rejected: Simpler 1-2 condition guard.
  • Why: Safe under simultaneous submissions, double-tab Create, double-submit, and malicious cross-user URL embedding; aligns with lockForUpdate() inside the caller's transaction.
  • Source: KD-0394

Profile picture controller: mirror AttachmentController::download (stream)

  • Chose: Inject Filesystem $storage; return $storage->response(...).
  • Rejected: Port emmie's GetUserProfilePictureAction (loads bytes into memory).
  • Why: Stream pattern is already arch-tested in this codebase; no full-file memory load; idiomatic.
  • Source: KD-0436

Profile picture authz: auth:sanctum,api only, no per-record policy

  • Chose: Same middleware stack as /api/users; add route URI to RoutesAuthorizationTest exemption list with comment.
  • Rejected: Per-avatar UserPolicy::view gate, or fully public no-auth route.
  • Why: Matches status-quo signed-URL behaviour; per-render policy gate would add overhead to board views with 50+ avatars; full-public exposes avatar existence without session.
  • Source: KD-0436

Avatar caching: Laravel cache.headers middleware, not custom headers

  • Chose: ->middleware('cache.headers:public;max_age=604800;etag') on the route.
  • Rejected: Manual Response::make()->withHeaders(), controller-level headers, or no caching.
  • Why: Zero custom logic; baseName rotation on upload acts as cache-buster, so 7-day TTL is safe; matches previous signed-URL lifetime.
  • Source: KD-0436

Storage::temporaryUrl ban: app-wide, not just Resources

  • Chose: ArchTest scans app/** for any usage.
  • Rejected: Limit ban to app/Http/Resources/** (the two existing call sites).
  • Why: Zero call sites remain after the change; banning everywhere prevents future regressions in Actions/Jobs/Services.
  • Source: KD-0436

Unification scope: serving layer only, not storage model

  • Chose: Both profile pictures and attachments serve via Laravel proxy; profile pictures keep their dedicated column.
  • Rejected: Migrate profile pictures into the attachments table.
  • Why: "Uniform" was about URL serving (the broken signed-URL case), not unifying storage; the latter would force a data migration and muddy attachment-table semantics.
  • Source: KD-0436

URL building: Model returns relative path, Resource composes absolute

  • Chose: Attachment::downloadPath(): string (no facades) + URL::to($attachment->downloadPath()) in the Resource.
  • Rejected: Helper class (Deptrac forbids Resources → Helpers), or model method using URL facade (arch test bans facades in models), or all-inline in Resource.
  • Why: Only this split satisfies both Deptrac and ModelsTest.php's facade ban while keeping the URL composition testable.
  • Source: KD-0436

Cache headers scope: profile pictures only, not attachments

  • Chose: cache.headers only on the avatar route.
  • Rejected: Apply to AttachmentController::download too.
  • Why: Avatars are high-traffic; attachments are user-downloaded files (potentially PII) that may not be safe to browser-cache without a separate review.
  • Source: KD-0436

Markdown rendering: shared DescriptionProse component, not patched v-html

  • Chose: New component owning v-html + post-render DOM enhancement for image hover toolbar.
  • Rejected: Custom marked renderer emitting Vue placeholders, or CSS-only :hover overlay with onclick handlers.
  • Why: Vue placeholders bypass DOMPurify; CSS handlers can't call Vue actions like createModal, and DOMPurify strips onclick anyway.
  • Source: KD-0578

DescriptionProse location: tenant, not shared (D8 reverses D2)

  • Chose: apps/tenant/components/markdown/.
  • Rejected: shared/components/ui/.
  • Why: Plan reviewer flagged the toolbar must import previewRequest/downloadRequest/toast/tenant icons — app-boundaries.spec.ts forbids @shared/* from importing @tenant/*.
  • Source: KD-0578

Inline image lightbox: new component, not reuse AttachmentPreviewModal

  • Chose: New InlineImageLightbox taking only {src, alt}.
  • Rejected: Open AttachmentPreviewModal via createModal(...).
  • Why: AttachmentPreviewModal needs a full Attachment DTO (filename/size/mime/userId); reusing would force DescriptionProse to look up attachments via a page-supplied store, ruining its self-contained contract.
  • Source: KD-0578

Copy action: image bytes, not URL

  • Chose: copyImageToClipboard(src) helper writing ClipboardItem({type: blob}); canvas re-encodes to PNG when the source mime is unsupported by the browser.
  • Rejected: Copy URL via navigator.clipboard.writeText(src).
  • Why: AC says "copy puts the image on the clipboard" (matches Linear); pasting in Slack/Figma/Email yields the picture, not a URL string.
  • Source: KD-0578

Reports markdown rendering: out of scope

  • Chose: Skip ReportDetail (still plain-text rendered).
  • Rejected: Add markdown pipeline + DescriptionProse to ReportDetail to satisfy issue text literally.
  • Why: Adds scope (sanitization rules, attachment URL pattern review) for a polish issue; if reports later get markdown, drop in DescriptionProse with one line.
  • Source: KD-0578

DOM watching: MutationObserver, not nextTick

  • Chose: Single observer registered in onMounted, idempotent enhance(img) function.
  • Rejected: Watch :html + nextTick + manual re-attach of listeners.
  • Why: Manual bookkeeping is easy to forget on caller change patterns; observer is one-time setup that handles all updates.
  • Source: KD-0578

Migration of all 7 prose call sites: yes, including AI planner

  • Chose: All description-prose v-html sites become <DescriptionProse>.
  • Rejected: Skip AI-planner sites (StoryCard, ChatMessage, AiStoryPrompt) since they rarely contain attachment images.
  • Why: Observer is cheap (idle when no <img> present); one prose pattern is easier to maintain than two.
  • Source: KD-0578

Attachment-viewer nav state: ref in the modal, not a composable

  • Chose: Track the current attachment as a ref inside the preview modal, mirroring how the inline image lightbox owns its own keyboard state.
  • Rejected: A useAttachmentNavigation composable.
  • Why: No second consumer exists, so the composable would be a one-method wrapper mirroring its single caller — the shallow, suspect module shape the module-shape lens warns against.
  • Source: KD-0507

CLI attach surface: two commands, not one --from-url flag

  • Chose: Split kendo <entity> attach <ref> <file> (local) from kendo migrate jira-attach <ref> <url> (URL streaming), sharing the underlying PostMultipart helper.
  • Rejected: A single command with --from-url + --basic-auth flags overloading one positional with both file-path and URL semantics.
  • Why: Mixing file/URL semantics on one positional is confusing and --basic-auth leaks credentials via shell history; splitting isolates Jira-specific auth/redirect concerns from the normal upload path.
  • Source: KD-0369

Jira migration credentials: env vars, not flags or stored config

  • Chose: Read JIRA_EMAIL + JIRA_API_TOKEN from the environment for the migration command.
  • Rejected: --jira-email/--jira-token flags, or a persistent kendo auth jira login storing creds in ~/.kendo/config.
  • Why: Flags leak via shell history and ps aux; a persistent login is over-engineered for a one-time migration that a skill already runs with these env vars set.
  • Source: KD-0369

CLI upload validation: backend-only, no local size/mime pre-check

  • Chose: CLI streams the file and lets the backend StoreAttachmentRequest return 413/415; only file-not-found is checked locally.
  • Rejected: Pre-validating file size and mime in the Go CLI before upload.
  • Why: Duplicating Laravel's 20+ extension allowlist in Go drifts silently when the backend changes; a ~2-5s wasted upload on a rare failure is cheaper than a divergent second source of truth.
  • Source: KD-0369

Inline image lightbox: extend InlineImageLightbox, not reuse AttachmentPreviewModal

  • Chose: Give InlineImageLightbox a {images, startIndex} API with internal nav for both internal and external images.
  • Rejected: Feed real Attachment objects into the existing AttachmentPreviewModal (KD-0507) viewer.
  • Why: External images have no attachment record, id, or auth-fetchable URL, so reuse forces a second non-attachment path anyway — erasing the benefit while coupling the generic prose renderer to the attachments domain and re-introducing a refetch/spinner.
  • Source: KD-0575

External image src policy: https-only, fail-closed sanitizer

  • Chose: Keep an <img> only if its src matches the internal attachment pattern or starts with https://; strip http, data:, blob:, javascript:.
  • Rejected: Also allowing http://, or allowing any non-attachment src including data:/blob:.
  • Why: Fail-closed default minimizes the XSS-adjacent surface; the accepted privacy cost (external host sees viewer IP/UA) was a documented opt-in.
  • Source: KD-0575

External-image toolbar: Maximise + Open-in-tab only

  • Chose: Render only Maximise and Open-in-tab for external images; internal images keep all four buttons.
  • Rejected: Showing the full toolbar (also Download + Copy) for external images.
  • Why: Cross-origin Download (<a download>) and Copy (fetch/clipboard) reliably fail under CORS, so the extra buttons would be misleading dead controls.
  • Source: KD-0575

Editor image button API: dedicated upload prop, not generic toolbarActions

  • Chose: A single upload prop on RichTextArea that auto-wires the paste extension and renders the image button.
  • Rejected: A generic toolbarActions?: Action[][] prop letting callers contribute arbitrary button groups.
  • Why: RichTextArea already owns bespoke concepts (raw mode, heading levels) so upload is a natural affordance, not a domain leak; the generic prop is engineering for a hypothetical future use with no current second caller.
  • Source: KD-0849

Paste extension wiring: auto-wire inside RichTextArea, not in callers

  • Chose: RichTextArea wires imagePasteExtension(upload) internally from the upload prop; callers drop it from their extensions arrays.
  • Rejected: Callers keep wiring the paste extension themselves and pass upload separately for the button.
  • Why: Passing the same upload function in two places is fragile — easy to forget one side or let them diverge.
  • Source: KD-0849

Non-image file in editor upload: silent no-op, not server round-trip

  • Chose: Check file.type.startsWith('image/') client-side and do nothing if it fails.
  • Rejected: Forward any picked file and let the server reject it with a toast.
  • Why: A server round-trip for a client-detectable error is wasteful, and RichTextArea lives in @shared and cannot call the @tenant toast service anyway; silent skip mirrors existing paste handling.
  • Source: KD-0849

Editor upload feedback: fire-and-forget, no per-button loading state

  • Chose: Leave the image button interactive during upload; the upload wrapper already toasts on success/failure.
  • Rejected: Disabling the button while in-flight by tracking isUploading.
  • Why: Loading state adds complexity for a rare double-submit race, and is inconsistent with paste, which shows no spinner today.
  • Source: KD-0849