feat: port the details panel, add a text editor, and stream file reads - #1105
Conversation
Details panel #1093: DetailsDialog now renders a file or folder body built from the v2 snapshot row, with copy-to-clipboard on the identifying fields and an explicit empty state for a size the projection has not resolved. Text editor #1092: an `edit` row action for the preview allow-list's text types opens a dialog that loads through the preview path — same allow-list, same byte budget — and saves through the facade write handles. TypeScript frames nothing: the bytes go to beginWrite/pushChunk/commitWrite as-is. Streaming reads #1080: a save and an image preview mint a `/stream/` ticket when this tab has a controlling Service Worker, so the browser pulls ranges instead of the tab buffering the plaintext. The byte budget now only bounds what is genuinely buffered. Closes #1093 Closes #1092 Closes #1080
/simplify: collapse the details bodies onto shared `NodeRows`/`StateRows` primitives so a file and a folder no longer restate the same four rows, drop the now-unused `StateRows.tsx`, and stop exporting the preview byte budget that nothing outside the hook reads. Altitude: `streamTicket` moves out of the download hook into `lib/` so the preview hook no longer depends on a sibling hook, and the text editor now names a non-text load as a refusal instead of rendering an empty dialog. /security-review found nothing to fold in.
|
Warning Review limit reached
Next review available in: 38 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThe file browser now uses dedicated details components, media-service streaming with buffered fallbacks, and a text editor that saves through chunked facade writes. Tests cover metadata, clipboard behavior, streaming paths, preview limits, editor eligibility, save failures, and dismissal protection. ChangesFile browser details
Media-service read paths
Text editing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FileBrowserActions
participant TextEditorDialog
participant Facade
FileBrowserActions->>TextEditorDialog: open editable text file
TextEditorDialog->>Facade: beginWrite
TextEditorDialog->>Facade: pushChunk for each 1 MiB chunk
TextEditorDialog->>Facade: commitWrite
Facade-->>TextEditorDialog: write result
TextEditorDialog-->>FileBrowserActions: close after successful save
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (3)
apps/web/src/components/file-browser/details/DetailsPrimitives.tsx (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace or remove this implementation comment.
The comment states the timeout duration. It does not state why this duration is needed. State the user-facing rationale, or remove the comment.
As per coding guidelines, “Comments should explain why rather than what.”
🤖 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 `@apps/web/src/components/file-browser/details/DetailsPrimitives.tsx` around lines 5 - 6, Update the comment associated with ACKNOWLEDGED_MS to explain the user-facing reason for the acknowledgment timeout, or remove the comment if that rationale is not needed; do not merely restate the 2000 ms duration.Source: Coding guidelines
apps/web/src/components/file-browser/FileBrowserActions.test.tsx (1)
671-687: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering a multi-chunk write.
Every fixture in this suite is far below the 1 MiB
CHUNK_BYTESthreshold, so the loop inTextEditorDialog.savealways runs exactly one iteration. The offset arithmetic and thesliceupper bound are the parts most likely to regress, and no test pins them.Exporting
CHUNK_BYTESand asserting the concatenatedpushChunkpayloads for a draft that spans two chunks would close the gap.🤖 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 `@apps/web/src/components/file-browser/FileBrowserActions.test.tsx` around lines 671 - 687, Add coverage for a two-chunk edit in the TextEditorDialog save flow by exporting and using CHUNK_BYTES, creating a draft larger than one chunk, and asserting the concatenated pushChunk payloads preserve the full content and chunk boundaries. Keep the existing facade write and commit assertions intact.apps/web/src/styles/dialogs.css (1)
191-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep a focus indicator that survives forced colors.
outline: noneremoves the only indicator that forced-colors mode preserves. Theborder-colorsubstitution is an author color, so a high-contrast user gets no visible focus state on the editor field.♿ Proposed fix: replace the outline instead of removing it
.text-editor-field:focus { - outline: none; border-color: var(--color-text-secondary); + outline: var(--border-thickness) solid var(--color-text-secondary); + outline-offset: 2px; }The PR notes the editor save round trip was not runtime verified. Please record the manual verification steps for the editor dialog in
VERIFICATION.md.As per coding guidelines: "When application changes affect UI components, styles, layouts, pages, routes, or other user-facing behavior, attempt Puppeteer MCP verification; if unavailable, document manual verification steps and flag the work in
VERIFICATION.md."🤖 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 `@apps/web/src/styles/dialogs.css` around lines 191 - 194, Update the .text-editor-field:focus rule to preserve a focus indicator in forced-colors mode by replacing outline removal with a suitable outline-based focus style. Add manual editor-dialog save round-trip verification steps to VERIFICATION.md, including opening the dialog, editing, saving, and confirming the persisted result.Sources: Coding guidelines, Path instructions
🤖 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 `@apps/web/src/components/file-browser/TextEditorDialog.tsx`:
- Around line 46-50: Update the write flow in TextEditorDialog’s
beginWrite/commitWrite path to enforce optimistic concurrency using the version
captured in row.contentVersion. Pass that expected version through the frontend
protocol and Engine::begin_write or validate it during commitWrite, rejecting
stale edits instead of overwriting newer content.
---
Nitpick comments:
In `@apps/web/src/components/file-browser/details/DetailsPrimitives.tsx`:
- Around line 5-6: Update the comment associated with ACKNOWLEDGED_MS to explain
the user-facing reason for the acknowledgment timeout, or remove the comment if
that rationale is not needed; do not merely restate the 2000 ms duration.
In `@apps/web/src/components/file-browser/FileBrowserActions.test.tsx`:
- Around line 671-687: Add coverage for a two-chunk edit in the TextEditorDialog
save flow by exporting and using CHUNK_BYTES, creating a draft larger than one
chunk, and asserting the concatenated pushChunk payloads preserve the full
content and chunk boundaries. Keep the existing facade write and commit
assertions intact.
In `@apps/web/src/styles/dialogs.css`:
- Around line 191-194: Update the .text-editor-field:focus rule to preserve a
focus indicator in forced-colors mode by replacing outline removal with a
suitable outline-based focus style. Add manual editor-dialog save round-trip
verification steps to VERIFICATION.md, including opening the dialog, editing,
saving, and confirming the persisted result.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d2440ab7-c5ab-4061-9fab-c989f912d1a3
📒 Files selected for processing (13)
apps/web/src/components/file-browser/DetailsDialog.tsxapps/web/src/components/file-browser/FileBrowserActions.test.tsxapps/web/src/components/file-browser/FileBrowserActions.tsxapps/web/src/components/file-browser/TextEditorDialog.tsxapps/web/src/components/file-browser/details/DetailsPrimitives.tsxapps/web/src/components/file-browser/details/FileDetails.tsxapps/web/src/components/file-browser/details/FolderDetails.tsxapps/web/src/components/file-browser/details/copy-clipboard.tsapps/web/src/components/file-browser/details/details.test.tsxapps/web/src/hooks/useFileDownload.tsapps/web/src/hooks/useFilePreview.tsapps/web/src/lib/streamTicket.tsapps/web/src/styles/dialogs.css
The panel is documented as one file's current version and showed none: toRow dropped SnapshotChildDescriptor.contentVersion, so the value never reached it. Thread it onto ListingRow and render it under content, dim 'unknown' when the snapshot has not projected it, as size already does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WegkkQ3uhNREerTW4MMeY2 Entire-Checkpoint: f925b23aebf2
1 participant
Three issues on one file-actions surface. #1092 and #1080 both rework
useFilePreview.ts, which is why they ship together.Closes #1093
Closes #1092
Closes #1080
What landed
#1093 — the current-version details panel
DetailsDialogused to flatten every node into one label/value list. It now renders a file body or a folder body over the v2 snapshot row:details/DetailsPrimitives.tsx—DetailRow,DetailSection,DimValue,CopyableValue, plus theNodeRows/StateRowsblocks every kind shares.details/FileDetails.tsxadds the content section (formatted size and the exact byte count);details/FolderDetails.tsxomits it, because a folder carries no content.details/copy-clipboard.ts— reports whether the write landed, so the button never confirms a copy the browser refused.unknown, notundefined.Every value is the engine's word verbatim; nothing is derived client-side.
#1092 — the text editor
An
editrow action appears for exactly the typespreviewKindcalls text. The dialog loads throughuseFilePreview, so it inherits the allow-list and the byte budget rather than restating them, and saves through the facade write handles:beginWrite({ node }, size)thenpushChunkthencommitWrite, withabortWritereleasing the reservation on failure. TypeScript frames nothing — the edited bytes go to the engine as bytes.Modal busyblocks dismissal while the save is in flight, matching the busy-dialog rule. The draft is adopted from the load exactly once, so a snapshot landing mid-edit cannot throw away what has been typed.#1080 — streamed preview and download
streamTicket(apps/web/src/lib/streamTicket.ts) mints a/stream/ticket when this tab has a controlling Service Worker, and returnsnullotherwise, so both callers degrade to the buffered facade read on a browser without one.Blob. Nothing of the file is held in the tab.Discrepancies against the issue bodies
Verified against the code, not the issue text.
packages/client/src/media/range.tssafeMimeTypecollapses anything that is notaudio/*,video/*, or non-SVGimage/*toapplication/octet-stream— deliberately, because a ticket URL is same-origin and top-level navigable. A PDF served through the pipe would arrive opaque and the sandboxed<iframe>would not render it. Loosening that check is a security decision that belongs withrange.ts, which this PR does not own. The PDF preview keeps the buffered read and the cap.version-download-guard.tsexclusion is correct, and nothing in v1'sFileDetails/FolderDetailsbeyond the primitives survives the port: every remaining v1 row (fileMetaIpnsName, metadata CID, ECIES-wrapped keys, IPNS sequence numbers) names v1 crypto that v2's snapshot does not carry.copy-clipboard.tsdocument.execCommand('copy')fallback is not ported. The app already requires a secure context to register its Service Worker, so the Clipboard API is always available; the fallback would be dead code.SnapshotChildDescriptor.contentVersionis the one genuine current-version field the panel could show, and does not.ListingRowdrops it, andapps/web/src/vault/listing.tsis outside this PR's file set. Worth a follow-up.Runtime verification
The Puppeteer MCP browser is shared and was being navigated out from under this session mid-run, so the verification was scripted against Playwright's Chromium instead — same browser, deterministic. Driven against a real stack: Kubo, mock IPNS routing and Postgres from
docker/docker-compose.yml, an API on:3010, and theVITE_E2E_HOOKdev bundle on:5174, signed in through the introspection hook.Verified:
name / type / node id / modified / size / bytes / queued; folder body renders the same minussizeandbytes.copy node idwrote the exact hex the row displays (read back off the clipboard) and flipped the button to its acknowledged state. Computed styles hold:.details-rowgrid110px 344px,.details-sectioncolourrgb(0, 102, 68).<img src>is/stream/<uuid>. Probing that ticket withRange: bytes=0-15answered 206 withcontent-type: image/png,x-content-type-options: nosniff,content-security-policy: default-src 'none'; sandbox,cache-control: no-store. Closing the dialog revoked it — the same URL then answers 404.href="/stream/<uuid>"anddownload="cat.png". NoBlob, no object URL.edititem on an image row. On a text row the dialog opens, and the save button stays disabled until there is a draft.Not verified at runtime, and why:
GET /account/quotaand then issues noPOST /content/upload, so the op sits atpending: contentindefinitely (watched for two minutes across several runs). Reads of unpublished content are refused, and the editor correctly reports the engine's owncontent unavailable: content not yet publishedfail-closed. This is a pre-existing local-stack gap —tests/web-e2e/README.mdalready records that no write-path slice can run here — and nothing in this diff touches upload. The save path is covered by unit tests: the edited bytes reachingbeginWrite/pushChunk/commitWrite, the refusal to dismiss mid-save,abortWriteon rejection, and the over-budget refusal before any decrypt.Manual steps to close that gap once uploads publish locally: upload a
.txt, wait forpending: none, thenedit, change the body,save, and re-openeditto read the new bytes back.Worth a separate issue
The streamed read serves content the buffered read refuses.
openContentStream/readStreamanswered 206 with real bytes for a file whose op was stillpending: content, whilefacade.downloadon the same node rejected withcontent not yet published. Two read paths over one node disagree about whether it is readable.Gates
pnpm typecheck,pnpm lint,pnpm lint:tracker-refs,pnpm test— green. 199 web tests, 374 client, 178 api. Working tree clean before and after./simplify— four findings folded in: the file and folder bodies were restating the same four rows (nowNodeRows/StateRows, andStateRows.tsxis gone);MAX_BUFFERED_BYTESwas exported with no consumer;streamTicketlived inside the download hook and was reverse-imported by the preview hook (nowlib/streamTicket.ts, so no hook depends on a sibling hook); the editor pattern-matched a subset ofFilePreviewand rendered an empty dialog for the rest (now a named refusal). One finding skipped:TextEditorDialogrepeats thebeginWritethen chunk-loop thencommitWrite/abortWriteshape fromuseDropUpload.ts. Factoring it out means either editinguseDropUpload.tsor adding a shared write module, both outside this PR's file set, and the upload loop is entangled with cancellation and op-id tracking that an in-place edit does not need./security-review— nothing to fold in. The mime a ticket declares always comes from the fixedpreviewKindallow-list, never sniffed and never off the wire, and that allow-list is a strict subset of whatsafeMimeTypehonours; a hostile file body is answered undernosniffplusdefault-src 'none'; sandbox; the text decode isfatal: true, so non-UTF-8 is refused rather than mangled; and the editor puts no key material anywhere and does no framing of its own./crypto-privacy-reviewnot run: the diff touches nocrates/coreprimitive, no key or seal material, and no adoption-gate read.Deliberately not done
range.ts,FileBrowser.tsx,snapshotStore.ts,AppShell.tsx,StatusIndicator.tsxanduseFolderPicker.tsuntouched.Note on the file set
One file lands outside the literal set named for this PR:
apps/web/src/lib/streamTicket.ts, created to resolve the/simplifyfinding above. It is net-new with a unique name, so it cannot collide with the sibling PR the way a shared edit could.Note
Add text editor, stream file reads, and port the details panel in the file browser
FileDetailsandFolderDetailscomponents with copyable name/node ID fields, dimmed unknown values, andcontentVersionsurfaced from listing rows.Macroscope summarized c5939a2.
Summary by CodeRabbit
New Features
Bug Fixes
Tests