feat: add in-session image sharing - #191
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughChangesSession image sharing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SessionNotesPanel
participant SessionView
participant images_ts
participant ImageChannel
participant useNotesStore
participant SessionImageViewer
participant imageSave_ts
SessionNotesPanel->>SessionView: select image file
SessionView->>images_ts: validate and sign image
SessionView->>ImageChannel: send image payload
ImageChannel->>SessionView: receive image payload
SessionView->>images_ts: verify image payload
SessionView->>useNotesStore: appendImage
SessionView->>SessionImageViewer: open selected image
SessionImageViewer->>imageSave_ts: save image
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/stories/SessionNotesPanel.stories.tsx (1)
33-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a populated-image panel story.
Lines 33-37 set
imagesto an empty list. Storybook never renders the new preview button, filename treatment, or image-open callback. Add aWithImagestory with a validSessionImageand an observableonOpenImagecallback.As per coding guidelines, “Every component and feature component must have a Storybook story.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stories/SessionNotesPanel.stories.tsx` around lines 33 - 37, Add a WithImage Storybook story for the SessionNotesPanel using a valid SessionImage in the images prop, and make onOpenImage observable rather than a no-op so the preview and image-open behavior are exercised. Keep the existing story unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/session/SessionImageViewer.tsx`:
- Around line 122-131: Update the overflow viewport wrapping the image in
SessionImageViewer to be keyboard-focusable, provide an accessible label, and
apply a visible focus-ring style while focused. Keep the existing zoom-dependent
scrolling and image behavior unchanged.
In `@src/features/session/SessionNotesPanel.tsx`:
- Around line 109-113: Gate GIF rendering in SessionNotesPanel.tsx lines 109-113
and SessionImageViewer.tsx lines 123-131 behind the global reduced-motion
setting. When reduced motion is enabled, use a static first-frame or other
non-animated fallback; otherwise preserve the existing animated image behavior.
- Around line 131-141: Update the file input in SessionNotesPanel by keeping its
existing image-selection behavior while removing it from sequential keyboard
focus and the accessibility tree. Use the appropriate non-focusable and
presentation attributes on the input referenced by fileInputRef; leave the
visible button control unchanged.
- Around line 41-49: Update the useLayoutEffect dependency in SessionNotesPanel
so it changes whenever the ordered entries content changes, not only when
entries.length changes; derive or reuse a stable representation of the sorted
entries’ identities/timestamps, while preserving the existing scroll-to-bottom
behavior.
In `@src/features/session/SessionView.tsx`:
- Around line 299-301: Update the open-image state in SessionView to track the
selected image ID rather than retaining the full SessionImage object, then
derive the active image from the current sessionImages collection. Clear or
close the viewer when that ID no longer exists, ensuring retention eviction and
session resets cannot display stale images.
- Around line 967-981: Update the image transfer flow around imageAction.send so
the local image is appended via useNotesStore.getState().appendImage only after
send resolves successfully; ensure rejected sends do not leave a normal sent
image entry in the feed.
In `@src/stories/SessionImageViewer.stories.tsx`:
- Around line 9-31: Update the SessionImage fixture around IMAGE_URL and image
so preview and download use the same valid PNG bytes. Replace the SVG data URL
and mismatched text Blob with a valid PNG fixture, then derive objectUrl,
mimeType, width, and height consistently from that fixture while preserving the
existing SessionImage metadata.
---
Nitpick comments:
In `@src/stories/SessionNotesPanel.stories.tsx`:
- Around line 33-37: Add a WithImage Storybook story for the SessionNotesPanel
using a valid SessionImage in the images prop, and make onOpenImage observable
rather than a no-op so the preview and image-open behavior are exercised. Keep
the existing story unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4260f691-667d-4e32-9597-68756cb6eed5
📒 Files selected for processing (13)
ARCHITECTURE.mdsrc-tauri/src/commands/system.rssrc-tauri/src/lib.rssrc/features/session/SessionImageViewer.tsxsrc/features/session/SessionNotesPanel.tsxsrc/features/session/SessionView.tsxsrc/features/session/imageSave.tssrc/features/session/images.tssrc/features/session/notesStore.tssrc/stories/SessionImageViewer.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/strings.tstests/unit/session-images.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Installer (macOS)
- GitHub Check: Installer (Windows)
- GitHub Check: Analyze (rust)
- GitHub Check: Rust (macOS)
- GitHub Check: Rust (Windows)
- GitHub Check: Rust (Linux)
- GitHub Check: Frontend
🧰 Additional context used
📓 Path-based instructions (6)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Do not create new documentation files unless explicitly requested; update canonical documentation,
CHANGELOG.md, orISSUES.mdwhen justified.
Files:
ARCHITECTURE.md
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use one focused change per commit, with a Conventional Commit subject such as
feat:,fix:,chore:,docs:, orci:; PRs are squash-merged.
Files:
ARCHITECTURE.mdtests/unit/session-images.test.tssrc-tauri/src/commands/system.rssrc/features/session/imageSave.tssrc/features/session/SessionImageViewer.tsxsrc/stories/SessionImageViewer.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/strings.tssrc/features/session/notesStore.tssrc/features/session/SessionNotesPanel.tsxsrc/features/session/images.tssrc-tauri/src/lib.rssrc/features/session/SessionView.tsx
**/*.{ts,tsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,rs}: Never add telemetry; the application is local-only. Never instruct users to paste model files or BIP39 mnemonics into an AI chat service.
Add comments only when the reason is non-obvious; identifiers should carry the meaning and code should read top-to-bottom.
Maintain scope discipline: do not refactor adjacent code while implementing a feature, add abstractions for hypothetical needs, or expand a bug fix beyond the bug.
Files:
tests/unit/session-images.test.tssrc-tauri/src/commands/system.rssrc/features/session/imageSave.tssrc/features/session/SessionImageViewer.tsxsrc/stories/SessionImageViewer.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/strings.tssrc/features/session/notesStore.tssrc/features/session/SessionNotesPanel.tsxsrc/features/session/images.tssrc-tauri/src/lib.rssrc/features/session/SessionView.tsx
tests/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for unit and integration tests; component tests are not currently supported because the harness uses node-env without RTL/jsdom, so component behavior belongs in Storybook and axe-core checks.
Files:
tests/unit/session-images.test.ts
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Rust changes must pass
cargo test,cargo fmt --check, andcargo clippy; dependency and supply-chain changes must passcargo deny check.
Files:
src-tauri/src/commands/system.rssrc-tauri/src/lib.rs
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: All design-token values—colors, spacing, fonts, radii, shadows, motion, and z-indexes—must come fromsrc/design/tokens.ts; do not use raw hex values, arbitrarypx, or inlinecubic-beziervalues.
User-facing toast and notification copy must live insrc/strings.ts; prefer centralized strings for JSX text andaria-labelvalues.
Maintain WCAG AA contrast for every text/background pairing in both themes, do not convey information by color alone, and honor the global reduced-motion kill switch; new motion must be gated by default.
Treat peer wire formats and identity derivation as cross-version contracts; coordinate changes so older builds and existing stored data remain compatible.
Every component and feature component must have a Storybook story.
Files:
src/features/session/imageSave.tssrc/features/session/SessionImageViewer.tsxsrc/stories/SessionImageViewer.stories.tsxsrc/stories/SessionNotesPanel.stories.tsxsrc/strings.tssrc/features/session/notesStore.tssrc/features/session/SessionNotesPanel.tsxsrc/features/session/images.tssrc/features/session/SessionView.tsx
🔇 Additional comments (14)
src/features/session/SessionNotesPanel.tsx (1)
2-40: LGTM!Also applies to: 51-102, 142-150
src/features/session/SessionImageViewer.tsx (1)
1-121: LGTM!Also applies to: 132-138
src/strings.ts (1)
594-614: LGTM!src/stories/SessionImageViewer.stories.tsx (1)
33-63: LGTM!src/stories/SessionNotesPanel.stories.tsx (1)
4-4: LGTM!src/features/session/images.ts (2)
1-204: LGTM!Also applies to: 290-297
238-243: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDenial of Service (CWE-409)
Reachability: External
Reachability path
● Entry src/features/session/SessionView.tsx │ ▼ ● Hop src/features/session/SessionImageViewer.tsx:36 download │ ▼ ● Hop src/features/session/imageSave.ts:21 saveSessionImage │ ▼ ● Sink src/features/session/images.tsValidate decoded image dimensions before acceptance.
verifyIncomingImagetrusts dimensions signed by the sending peer. The peer also controls the image bytes.A compressed image can have a small encoded size but a very large decoded size. The current 5 MiB limit and 32,768-per-axis limit do not bound decoded memory. False metadata also bypasses the dimension control.
Decode the received image before storage. Compare its actual dimensions with the manifest. Enforce a maximum pixel count and an animation limit before
SessionImageViewerrenders the object URL.Also applies to: 284-287
tests/unit/session-images.test.ts (1)
1-137: LGTM!ARCHITECTURE.md (1)
381-381: LGTM!src/features/session/notesStore.ts (1)
20-45: LGTM!Also applies to: 55-73
src/features/session/imageSave.ts (1)
1-36: LGTM!src-tauri/src/commands/system.rs (1)
342-345: LGTM!src-tauri/src/lib.rs (1)
73-74: LGTM!Also applies to: 162-163
src/features/session/SessionView.tsx (1)
106-128: LGTM!Also applies to: 258-260, 896-918, 946-966, 982-982, 1047-1048, 1678-1695, 1973-1987
|
Auditing and addressing the requested changes. I’m fixing the unresolved review findings and hardening image validation, then I’ll run the full CI/Storybook/Rust gate set before marking this ready. |
GitHub Actions
Failure
GitHub Actions
Failure
GitHub Actions
Failure
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
What changed and why
Restores the signed, session-scoped P2P image sharing from #185 after its direct integration was reverted for bypassing the required pull-request merge path. Images remain memory-only, are validated before rendering, open in a zoomable viewer, and download only through a native save dialog. The PR branch is rebuilt from current
mainas one focused commit, retaining all review hardening while removing the old stack, CI-trigger commits, and dependency drift.Fixes #185.
Manual test
npm run tauri devlaunched and the changed surface behaves as described — not run; the P2P flow needs two live app instancesCompatibility surfaces
session-imageaction is ignored by older clients that never register it; existing action formats are unchangedGates
npm run build && npm run lint && npm run test— 1,085 tests passednpm run check-tokens && npm run check-strings && npm run check-contrastnpm run check-migrations && npm run check-storiesnpm run build-storybook && npm run check-a11y— 318 tests passedcd src-tauri && cargo fmt --check && cargo clippy && cargo test— 117 tests passed; two pre-existing warnings remaincd src-tauri && cargo deny check— advisories, bans, licenses, and sources passedCI-only checks will run after this PR is reopened.
Merge style