Attachment history drawer#121
Closed
aakhter wants to merge 2 commits into
Closed
Conversation
Builds on the COD-37 registry: surfaces detected/registered attachments as dismissible cards with a first-page thumbnail and an inline preview — the consumer the registry PR deliberately deferred. Backend: - document-thumbnailer: first-page PNG thumbnails (PNG passthrough; PDF via pdftoppm; Office via the preview cache). - document-preview-cache: disk-cached DOCX/PPTX -> PDF conversion (LibreOffice / PowerShell COM), in-flight dedup, multi-converter fallback. - file-routes: serveConvertedPreview / serveThumbnail + four routes — GET .../attachments/:id/preview, .../thumbnail and the workspace-path file-preview / file-thumbnail. Reuses the registry's TOCTOU-safe resolveServableAttachmentPath, so previews stream the freshly-resolved path. - server: enrich detected attachment events with a thumbnail route. - image-watcher: .png now routes to attachment:detected — this PR adds the card consumer, so the screenshot popup is no longer its only handler. Frontend: - panels-ui: attachment cards (addAttachmentCard, lazy stack, Clear-all, per-session cleanup) plus a 3-arg openFilePreview that renders registered attachments inline (image/PDF) or via the server-converted PDF (docx/pptx). - app.js: wire attachment:detected -> _onAttachmentDetected and card state. - styles: attachment-card + stack styling. Verified: tsc / eslint / prettier / frontend-syntax clean; new thumbnailer + preview-cache unit tests pass; full test:ci green (2861 passed); card render + preview overlay + dismiss verified in-browser.
Stacks on COD-38: accumulates a per-session attachment history and exposes it through a slide-in drawer with an unread badge, so attachments stay reachable after their cards are dismissed. Backend: - session-attachment-history: history state — dedupe by source path / relative path, newest-first, 100-item cap, and externalPath sanitization (the absolute host path is server-private and never leaves toState()). - session.ts: _attachmentHistory + getter (sanitized) / upsert / restore / getAttachmentHistoryForPersist; restored from saved state in the constructor. - file-routes: GET /attachments (list — resolves each entry to live metadata + routes; external entries are re-registered) and GET /attachments/:id (metadata poll). The by-id route guards via the registry's TOCTOU-safe resolveServableAttachmentPath. - server.ts: detected/registered attachments upsert into history and persist; the private (externalPath-bearing) history rides on disk under __attachmentHistory, separate from the sanitized public copy, and is restored on mux-session recovery. - types/session.ts: SessionAttachmentHistoryItem + SessionState.attachmentHistory. Frontend: - panels-ui: the drawer (lazy-built), unread badge, list render with per-item preview/download/open/"Card" (reshow) actions, and live refresh of the open drawer on new detections. - app.js: history state + per-session badge/cleanup wiring. - index.html / styles.css / mobile.css: header button + badge and the drawer. Verified: tsc / eslint / prettier / frontend-syntax / public-assets clean; new history-module unit tests pass; full test:ci green (2866 passed); badge, drawer open/render/reshow/close verified in-browser.
Ark0N
pushed a commit
that referenced
this pull request
Jun 14, 2026
#121) Follow-up fixes applied during review of PR #121 (all confirmed minor/nit; no blockers). Security posture verified sound (externalPath never leaves toState()/the list route; re-registration runs the guard). - fix(recovery): restoreAttachmentHistory now skips malformed/legacy saved items (null, non-object, missing source/fileName) instead of throwing inside the Session constructor — a corrupt __attachmentHistory entry could otherwise abort the entire mux-recovery loop. (P1) - fix(routes): the attachment-list route degrades a single failing entry to {missing:true} instead of failing the whole drawer. (INT-4) - fix(ui): give the attachments header button a positioning context so the unread badge anchors to the icon, not the header bar. (F1/CSS-1) - fix(ui): cancel the debounced history refresh on drawer close and guard it against a stale session/closed drawer. (F3) - fix(ui): re-show ("Card") of a detected item now uses the item's own timestamp so the cardId is stable — focuses the existing card instead of stacking duplicates. (F4) - fix(ui): Escape now closes the drawer, matching every other panel. (UX-1) - fix(ui): badge shows "99+" past 99 (was an inconsistent 100/99 cap). (BADGE-1) - style: drop the duplicate @Keyframes notif-badge-pulse (dead CSS). (INT-1/CSS-3) - style: empty-state used three undefined CSS custom properties (--text-primary/--border-color/--bg-tertiary) → use the defined --text/--border-light/--bg-input tokens. (CSS-2) - test: add constructor restore round-trip + malformed-item resilience tests. Deferred (noted for author): broadcasting the full 100-item history in every session-state SSE event (payload bloat), "unread" badge semantics, making the header button opt-in, and app.inject route tests for the two new endpoints. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ark0N
pushed a commit
that referenced
this pull request
Jun 14, 2026
Per-session attachment history with a slide-in drawer, unread badge, and re-show. Rebased onto master (stacked on #120) + review hardening (malformed- history recovery guard, resilient list route, badge positioning, debounce cancel, stable re-show, Escape-to-close, CSS token fixes). See PR #121. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Merged into Thanks for this, @aakhter — solid feature. I ran a deep multi-agent review (23 findings, all minor/nit, zero blockers) and verified the security posture: Heads-up: the committed Review fixes I applied (separate commit, all tested):
Verified: Deferred for your call (none correctness/security): the full 100-item history is broadcast in every session-state SSE event (payload bloat — worth stripping from the light serializer); the badge is a total-count labeled 'unread'; the header button is always-on vs the opt-in convention of its neighbors; and |
Ark0N
pushed a commit
that referenced
this pull request
Jun 14, 2026
Document the three PRs that grew attachments since the last update: COD-37/#119 (registry + magic links) was already covered, but COD-38/#120 (document previews/thumbnails) and COD-39/#121 (history drawer) added four source files and several endpoints that weren't documented. Split a dedicated Attachments row out of Infra, extend the Attachments Key Pattern to cover the converter pipeline + concurrency limiter + history drawer, and refresh the files-route handler count (8 -> 14) and total (~140 -> ~146). Also carries the prior pending app.js line-count (3.7K -> 3.9K) and config-file-count (10 -> 12) bumps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #120 (document attachment previews). Until #120 merges, this PR's diff also shows the COD-38 commit (
49c92e4); the history commit is3731aef. Once #120 lands, rebasing leaves only the history change.Accumulates a per-session attachment history and exposes it through a slide-in drawer with an unread badge, so attachments stay reachable after their cards are dismissed.
Backend
session-attachment-history— history state: dedupe by source/relative path, newest-first, 100-item cap, andexternalPathsanitization (the absolute host path is server-private and never leavestoState()).session.ts—_attachmentHistory+ sanitized getter /upsertAttachmentHistory/restoreAttachmentHistory/getAttachmentHistoryForPersist; restored from saved state in the constructor.file-routes—GET /api/sessions/:id/attachments(list — resolves each entry to live metadata + routes; external entries are re-registered so the guard runs again) andGET /api/sessions/:id/attachments/:attachmentId(metadata poll, guarded by the registry's TOCTOU-saferesolveServableAttachmentPath).server.ts— detected/registered attachments upsert into history and persist; the private (externalPath-bearing) history rides on disk under__attachmentHistory, separate from the sanitized public copy in session state, and is restored on mux-session recovery.types/session.ts—SessionAttachmentHistoryItem+SessionState.attachmentHistory.Frontend
panels-ui— the drawer (lazily built, appended to<body>), the unread badge, list rendering with per-item Preview / Download / Open / "Card" (reshow) actions, and a debounced live refresh of the open drawer on new detections.app.js— history state + per-session badge/cleanup wiring (clear on kill-all, refresh on tab switch, drop on session removal).index.html/styles.css/mobile.css— header button + badge and the drawer (full-screen sheet on phones).Verification
tsc,eslint,prettier --check,check:frontend-syntax,check:public-assetsclean. New history-module unit tests pass; fulltest:cigreen (2866 passed). Badge bump, drawer open/render/reshow/close verified in-browser.