Skip to content

feat(files): drag explorer files into Claude Code and Codex terminals - #23

Merged
kshivang merged 13 commits into
mainfrom
feat/file-drag-to-terminal
Oct 6, 2026
Merged

kshivang merged 13 commits into
mainfrom
feat/file-drag-to-terminal

Conversation

@nileshvasita

@nileshvasita nileshvasita commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Merge order

Merge #20 first, then this PR. This branch includes project-switcher head beb3fc4; combined fixes are validated together. Version 1.7.1 follows #20 version 1.7.0.

What changes

The FILES explorer already supports selecting and copying paths, but cannot drag those files into an existing terminal prompt. Add a native drag source to each tree row so users can reference files while working in Claude Code, Codex, or a shell.

  • Dragging a selected row exports the full selection in predictable order, including selected items in collapsed folders. Dragging an unselected row exports only that item.
  • Export copy-only javaFileListFlavor with actual file references. Compacted directories resolve to their displayed end directory. No file contents are read and no Enter is sent by the source.
  • Reject the entire selection with an explanation if paths contain control characters, are not absolute, or selected entries can no longer be resolved in the tree.
  • Reuse the existing terminal receiver, which inserts paths into the pane receiving the drop and handles path quoting. No terminal plugin changes are required for the native transfer contract.

Closes #22.

Validation

  • CI=true ./gradlew test buildPluginJar --console=plain -Pkotlin.daemon.jvmargs=-Xmx2g against released API 1.0.87: 258 combined tests passed across 31 suites, zero failures/errors/skips, including eight drag tests and the project-switcher regressions.
  • Tests cover full selection ordering, compacted folders, native transfer types, spaces/quotes/Unicode, copy-only actions, and whole-payload rejection.
  • git diff --check passed.
  • Source contract checked against terminal-tab 2.5.110 and BossTerm 1.2.173. Live BOSS interaction has not been tested; this PR remains draft pending manual checks.

Manual checks before marking ready

  • Drag a single file and multiple files from FILES into Claude Code, Codex, and a plain shell with existing prompt text; verify references are inserted without submitting.
  • Drop into an unfocused split pane and verify only that pane receives the paths.
  • Verify spaces, apostrophes, Unicode, collapsed selections, and compacted folders.
  • Verify click, range/toggle selection, double-click, context menus (including right-click with slight mouse movement), drag cancellation, and drop-target feedback.
  • Verify rejected control-character paths show a dialog, receive focus cleanly on macOS, and insert nothing.
  • Record desktop OS and shell tested. The existing receiver uses POSIX quoting; Windows shell compatibility is not established by these unit tests.

Browsing Downloads/Documents independently of the active project remains a follow-up from #22.

kshivang and others added 5 commits August 26, 2026 12:10
The header's first line showed the open project's name as static text. It is
now the control that changes the project: a dropdown listing recent projects,
current one first with a checkmark and its parent directory underneath (home
collapsed to ~), plus "Open Project..." below a divider.

Selecting a row calls ProjectDataProvider.selectProject, the same host call the
top bar's own project menu makes, so the switch belongs to the WINDOW rather
than to this panel: the top bar name, the sidebar panels and every
project-scoped tab follow it. Re-picking the open project is a no-op instead of
re-running the host's project-selection flow to land where we already are.

Two defects on the picker path, both of which made a folder chosen from
"Open Project..." appear to do nothing:

- The picker ran from inside the menu's own composition. Under HARDWARE
  rendering the menu is a heavyweight always-on-top AWT window (BossWindow
  provides LocalHeavyweightOverlays = true), and the host's picker parents its
  native FileDialog to KeyboardFocusManager.activeWindow. Dismissing the menu
  and opening the picker in one event therefore handed the modal dialog an owner
  that the same frame disposes. The pick request now crosses back into the
  panel's own composition and runs a frame later, once the menu window is gone.

- The macOS picker returns dialog.directory WITH a trailing separator when
  nothing inside the folder was selected. PathUtils.name of that is "", so such
  a pick reached the host as a project literally named "Unknown", on a path that
  did not match the recents entry for the same folder. New
  PathUtils.trimTrailingSeparator runs first; the dropdown's dedupe key shares
  the same helper, so one folder cannot occupy two rows.

The picker path is logged at INFO throughout. It is driven by a native dialog
inside the host, so without those lines there is no way to tell a cancelled
dialog from a missing provider from a callback the host ignored.

Tests: 19 new (dropdown ordering and matching incl. Windows separators; picker
result handling). slf4j-simple joins the test runtime because BossLogger binds
slf4j in its static init, so any test reaching a log call died without it.
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review: drag explorer files into terminals

This change is small and easy to follow. Keeping the path logic in FileTreeDrag apart from Compose lets it be unit-tested. The copy-only native file list with no side effects on completion is the right contract for dropping into a terminal. Rejecting the whole payload instead of sending part of it is a good call.

BOSS conventions

  • ✅ Only Compose Multiplatform APIs are used (foundation.draganddrop.dragAndDropSource, ui.draganddrop.*) plus AWT DataFlavor/Transferable. That is correct for Desktop, and nothing is Android-specific.
  • ✅ No new provider access. contextMenuProvider is still null-checked as before.
  • ✅ plugin.json is untouched. Both new Kotlin files end with a newline.
  • ℹ️ build.gradle.kts still says 1.6.1. That's fine if the release bot bumps the version (the last commit is Bump version to 1.6.1 [skip ci]). If not, this is a user-visible feature and should get a minor bump there.

Possible issues

  1. Shared errorMessage state (CodebaseContent.kt:325, :371). The drag error writes to the same errorMessage that the Create/Rename/Delete dialogs use for their inline validation text, and dismissing it sets that shared value back to null. Today this works because the dialogs are modal. Still, it couples unrelated flows, and one boolean plus one shared string is more state than needed. A dedicated var dragError by remember { mutableStateOf<String?>(null) }, with dragError?.let { ... }, would be simpler and self-contained.

  2. Right-button or other-button drags. dragAndDropSource { } uses the default drag detector, which I don't believe filters by mouse button. A right-press on an unselected row already calls onSelectOnly in the Initial pass (around line 753). If the user then moves the mouse slightly before the menu opens, a native drag may start instead of, or as well as, the context menu. Please add "right-click with a small mouse movement" to the manual checklist. If it reproduces, gate on the primary button.

  3. Stale selection counts as "no longer in the tree." In paths(), nodesByPath[it] ?: return null rejects the whole drag if any selected path is missing from the loaded tree. That happens after an external delete or rename, or if a collapsed folder's children were unloaded or reloaded. Check whether the ViewModel prunes selectedPaths when the tree reloads. If it doesn't, users will hit this dialog in normal use. Dropping missing entries, or pruning selection on reload, may be friendlier than a modal dialog at the start of a drag. At minimum, please confirm the ViewModel's reload behavior.

  4. Modal dialog opened from the drag-start callback. onRejected sets state inside the transferData lambda, which runs during pointer-input handling. Returning null cancels the drag, so this should be fine. Still, it's worth checking manually that the dialog grabs focus cleanly on macOS, where a pending mouse-drag session can sometimes keep focus.

Nits

  • CodebaseContent.kt:628: changing the em dash to a hyphen in an existing comment is unrelated noise. Fine to keep, but it's outside the PR's scope.
  • FileTreeDrag.transferable returns Transferable? and folds two checks (control characters, relative paths) into one null. The message already names both causes. If you want a precise message, return a sealed result or check the two separately. Optional.
  • Formatting of if (paths.isEmpty() || paths.any { path -> ... }) return null: the multi-line lambda inside the if condition is a little hard to read. A local val invalid = paths.any { ... } would help.
  • Optionally, add a KDoc note that tree must be the same snapshot as rows.

Performance
Fine. Drag start does one O(rows) filter, plus a traversal of the loaded tree only when selected items are hidden. It only runs when a drag starts, not on each recomposition.

Security
The control-character rejection (isISOControl covers C0, DEL and C1, including \u009b CSI) is a sensible safeguard, because the receiver writes into a PTY. Quoting is left to the terminal receiver. That is the right boundary, but it means shell-metacharacter safety ($(...), ;, &) depends entirely on the terminal plugin's POSIX quoting. The $(echo nope);&.txt test only checks that the name survives the transfer, not that it is quoted safely. As the PR says, Windows shells (cmd/PowerShell) are not covered by POSIX quoting, so it's worth tracking that as a known limitation or a follow-up issue.

Tests
The coverage is good: ordering, collapsed selection, compacted chains, flavors, the defensive copy, copy-only actions, and each control-character class. Suggestions:

  • Add a case where the source is selected but the selection also contains the chain end path of a compacted directory, to confirm distinct() dedupes top and end correctly.
  • There is no UI or gesture test for the dragAndDropSource / combinedClickable interaction. That's understandable, and the manual checklist covers it.

Overall this looks good. Items 1 and 3 are the ones I'd most like addressed or confirmed before marking it ready, and item 2 should go on the manual checklist.

@nileshvasita

Copy link
Copy Markdown
Contributor Author

Addressed review in 5e0eb84:

  • Replaced shared operation error state plus a boolean with dedicated nullable dragError. Dismissal now affects only the drag dialog.
  • Added compact chain top/end deduplication regression and documented the tree/rows snapshot contract.
  • Verified Compose 1.10 DragAndDropSource.skiko.kt explicitly uses PointerMatcher.mouse(PointerButton.Primary). A custom button filter is unnecessary. Added right-click with slight movement and macOS dialog focus to the manual checklist.
  • Confirmed watcher refresh, hidden-file toggles, and deletes prune selection. Rename/explicit refresh have an existing gap; the watcher can skip pruning on an unchanged tree. Tracked separately in fix(files): prune stale selection after rename and explicit refresh #24. This PR retains whole-payload rejection with reselect guidance rather than silently omitting requested files.
  • Existing POSIX terminal quoting and unverified Windows shells remain documented limitations.

CI=true ./gradlew test buildPluginJar --console=plain passed: 220 tests, no failures or skips. Follow-up agent review found no blockers. The built plugin was copied locally for manual testing with the previous JAR backed up; BOSS was not launched. Keeping the PR draft until live interaction checks are complete.

@claude

claude Bot commented Oct 1, 2026

Copy link
Copy Markdown

Review: drag explorer files into terminals

This is a clean, focused PR. The drag logic sits in its own small FileTreeDrag object and is tested well. The README and PR description say clearly what has been checked and what hasn't. A few notes below, most important first.

✅ What's good

  • Logic is separate from Compose. paths() and transferable() are plain functions, so the ordering, compact-chain and rejection rules can be unit tested without a UI. The 8 tests cover the edge cases that matter: hidden selections, compact chains, duplicate top/end, special characters, and C0/C1 control characters.
  • Safe defaults. The drag is copy-only, offers only javaFileListFlavor, never reads file contents, and has no onTransferCompleted side effect. getTransferData returns a fresh ArrayList each time, so a drop target can't mutate the payload.
  • All or nothing. If any path is bad, the whole drag is rejected rather than silently dropping files. That's the right call for a terminal target.
  • Conventions: only Compose Multiplatform / AWT APIs are used (androidx.compose.foundation.draganddrop is the multiplatform artifact), no new provider access, and both new Kotlin files end with a newline.

⚠️ Possible issues

  1. Right-button drags may start a native drag (CodebaseContent.kt:764). The default dragAndDropSource { } gesture detector does not filter by mouse button as far as I know. A right-press with a little movement can therefore start a drag after the Initial-pass onSelectOnly. The host's context-menu modifier sits outside baseModifier and could lose the gesture. Your manual checklist already covers this. If it turns out to be a problem, the overload that takes a custom drag detector (start only on isPrimaryPressed) would fix it in the plugin, with no host change.

  2. Rejection may be too strict after a tree refresh (FileTreeDrag.kt:40). nodesByPath[it] ?: return null rejects the whole drag if any hidden selected path isn't in the loaded tree. The tree is lazy, so a reload or re-collapse can unload a folder's children while selection keys survive. CodebaseViewModel only prunes selection on delete/rename. The user then gets a "select again" dialog for a selection that still looks valid. Two options:

    • Fall back to File(path) for unresolved paths that still exist on disk. They're already absolute selection keys.
    • Prune the selection whenever the tree reloads.

    If strict is intentional, consider saying in the message which item couldn't be resolved.

  3. Folders and their contents can both be exported. If a collapsed folder and a file inside it are both selected, both references go out. That's arguably correct, since it's what was selected, but it can surprise someone dropping into an agent prompt. Not blocking; maybe mention it in the README.

  4. Hidden items are ordered by plain string comparison (remaining.sorted()), not tree order. That's documented ("path order"). Be aware it differs from what the user would see if they expanded the folder: directories aren't sorted first, and the comparison is case-sensitive.

🧹 Minor / style

  • CodebaseContent.kt:624 changes an em dash to a hyphen in an unrelated comment. It's harmless, but it's noise in the diff.
  • FileTreeDrag.paths(..., tree: FileNode? = null): a missing tree makes every drag that includes hidden items fail (see the second assert in the first test). The only production caller always passes tree, so consider making it required so a future caller can't forget it.
  • The dragError dialog copies the layout of the other BossDialogs. That's fine for now. If more simple message dialogs appear, a small MessageDialog(title, message, onDismiss) helper would cut the repetition.

🔢 Versioning
build.gradle.kts is not bumped (still 1.6.1). Merging to main triggers the release/store publish workflow. A user-visible feature like this should probably go out as 1.7.0 (or at least 1.6.2), so the store release doesn't clash with or reuse the existing version. Remember to change it only in build.gradle.kts; plugin.json is synced at build time.

⚡ Performance
No concerns. Work happens once per drag start: a linear filter over rows, plus one iterative tree traversal (no recursion-depth risk), and only when a selected item is hidden. Recreating the lambda for every row on recomposition is cheap.

🔒 Security
Rejecting ISO control characters (including \u009b CSI and \u0085) means the transfer itself can't inject terminal escape or newline sequences. Quoting shell metacharacters ($(...), ;, &) is left to the receiving terminal, which is the right split. The Windows quoting caveat in the README is appreciated.

🧪 Tests
The unit coverage of the pure logic is good. Nothing exercises the gesture wiring itself (selection preserved on press, the drag threshold, right-click). That's expected for Compose desktop, and your manual checklist covers it.

Overall this looks good to merge once the manual checks pass and the version is bumped. Item 1 is the one I'd watch most closely during manual testing.

@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review: drag explorer files into terminals

Overall this is a well-scoped change. The drag logic lives in its own pure object (FileTreeDrag), which made it easy to unit test. It only exports javaFileListFlavor, offers Copy as the only action, never reads file contents and never sends Enter. If any path is unsafe, it rejects the whole selection instead of sending part of it. That matches the intent of #22. Comments below, most important first.

Potential bugs / behavior to verify

  1. Right-button and modifier drags may start a native drag. dragAndDropSource { … } with the default detector starts on any pointer drag that passes the touch slop. On desktop that isn't limited to the primary button. Right-click with a slight mouse movement could start a drag and swallow the context menu. You already list this in the manual checks. If it reproduces, a custom detectDragGestures that ignores events where !event.buttons.isPrimaryPressed would fix it. A Cmd/Shift-click with a little jitter could also start a drag instead of toggling or range-selecting, which is worth a quick check.

  2. Dragging an unselected row leaves the old selection highlighted. The row you drag isn't highlighted and the old selection still is. Finder and IntelliJ either select the dragged item or at least show which rows the drag carries. This isn't a blocker, but it is inconsistent with the right-click behavior a few lines above, which collapses the selection to the clicked row "so the highlight always matches what the context menu will act on". Consider calling onSelectOnly(node.path) when a drag starts from an unselected row. The README would need updating to match.

  3. The control-character filter misses some characters that look like line breaks or reorder text. Char.isISOControl() covers C0, DEL and C1, which handles the cases that matter most. It doesn't catch U+2028/U+2029 (line and paragraph separators) or bidi overrides such as U+202E. A bidi override can make the inserted path look different from what will actually run. Since the stated goal is "terminal destinations can treat control characters as input", you could also reject Character.getType(c) in FORMAT, LINE_SEPARATOR and PARAGRAPH_SEPARATOR. This is optional hardening; the receiver quotes paths anyway.

  4. Stale snapshot inside the lambda. onDragTransferData captures selectedPaths, rows and tree from the latest composition. That's fine, because the modifier is rebuilt on every recomposition and rows is remember(tree, expandedPaths). The KDoc requirement that "[rows] must be derived from the same tree snapshot as [tree]" holds at the call site. A short comment at the call site would keep it that way if rows is ever hoisted or memoized differently.

Code quality

  • paths() is clear. The traversal only runs when part of the selection is hidden, and distinct() correctly merges a compact chain top and end into one reference. Rejecting the drag when a hidden selected path can't be resolved is a sensible conservative choice.
  • Returning a fresh ArrayList from getTransferData is good defensive practice, and there's a test for it.
  • The new error dialog doesn't follow the existing dialogs' typography. The others use explicit fontSize = 13.sp or 11.sp and the same title, body and button layout. Extracting a small InfoDialog(title, message, onDismiss), or reusing the confirmation dialog's structure, would keep things consistent. It would also be reusable if more one-off messages show up later.
  • Starting the drag returns null and shows a modal dialog in the middle of the gesture. That's reasonable, but please confirm on macOS that the dialog doesn't get stuck behind an AWT drag session. You already list this in the manual checks.

BOSS plugin conventions

  • ✅ Only Compose Multiplatform and AWT desktop APIs (androidx.compose.ui.draganddrop and java.awt.datatransfer), nothing Android-specific.
  • ✅ No new PluginContext provider access. contextMenuProvider is still null-checked as before.
  • ✅ The version is only changed in build.gradle.kts, and plugin.json is untouched.
  • ✅ Both new Kotlin files end with a newline.
  • ❓ Version jump from 1.6.1 to 1.7.1 skips 1.7.0. Is that intentional, for example to account for the auto-bump commit (🔖 Bump version … [skip ci]) in the release workflow? If not, 1.7.0 reads more naturally for a new feature.

Performance

  • Nothing significant. Each row builds a transfer-data lambda on recomposition, but it only runs when a drag starts. The full-tree traversal also only runs at drag start, and only when part of the selection is collapsed.

Security

  • Good: the source sends no keystrokes, reads no file contents and offers Copy only. Relative paths and control characters are rejected for the whole payload, and quoting is left to the receiving terminal. Only item 3 above is worth considering.
  • As the PR notes, the receiver's POSIX quoting isn't proven on Windows shells. That belongs to the terminal plugin, but it's worth tracking as a follow-up issue.

Test coverage

The eight new tests are thorough. They cover ordering, hidden selections, compacted folders, deduplication, flavors, payload immutability, copy-only actions and rejection paths. Possible additions:

  • A selected compacted folder that is hidden inside a collapsed parent, to check that the hidden path also resolves to the chain end.
  • A Windows-style absolute path case, or a note that isAbsolute depends on the platform the tests run on.
  • Rejection of U+2028 and U+202E, if you adopt item 3.

UI wiring (the dragAndDropSource placement relative to combinedClickable and the context menu) can't be covered by these unit tests, so the manual checklist matters before this leaves draft.

Nice work overall. Items 1 and 2 are the ones I'd most like checked during the manual pass.

@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review: drag explorer files into terminals

The drag logic is small, separate from the UI and well tested. The pure FileTreeDrag.paths / transferable split works well, and so do refusing the whole payload and offering Copy only. Notes below, most important first.

1. Scope: this PR includes #20
The diff also carries the whole project-switcher change from #20: ProjectSwitcher*, ProjectSelection, ProjectPathPolling, the MCP select_project normalization, the header rewrite and the polling mutex. About 75% of the +1273 lines are #20. Please merge #20 first and rebase, so this PR's diff is only the drag feature (FileTreeDrag.kt, the CodebaseContent.kt hook, its tests and the README). Then each change can be reviewed and reverted on its own. The version bump 1.6.1 → 1.7.1 fits that order (1.7.0 for #20); it is correctly made only in build.gradle.kts.

2. A right-button drag may start a native drag (please verify)
Modifier.dragAndDropSource { _ -> … } (CodebaseContent.kt:765) detects drags with the standard drag-gesture detector. As far as I know, that detector does not filter by mouse button. So right-click followed by a few pixels of movement could start a file drag, and that can race with contextMenuProvider.applyContextMenu. Your manual checklist already covers this case, which is good. If it reproduces, there is a cheap fix. The pointerInput on the Initial pass just above already sees every press, so it can record whether the last press was secondary, and onDragTransferData can return null in that case (this cancels the drag without opening the dialog).

3. Stale selection rejects the whole drag
paths() returns null when a selected path is no longer in the loaded tree, for example after an external delete or rename followed by a refresh. The user then gets "Select the items again". Refusing the whole payload is right for unsafe paths. Here it may be friendlier to prune stale entries from selectedPaths when the tree reloads, so a normal drag after a refresh just works. If the view model already prunes on reload, ignore this.

4. Minor / optional

  • Feedback on unselected rows: dragging an unselected row exports only that row, but the highlight still shows the old selection during the drag. Finder and IntelliJ select the dragged row. This is a UX choice, not a bug, but the README could say so explicitly.
  • Format characters: isISOControl() covers C0, DEL and C1, which is the right guard against terminal injection. Bidi overrides (U+202E) and zero-width characters get through. They cannot run anything, but they can make the inserted path look different from the real one. Optional: also reject Character.getType(c) == Character.FORMAT.
  • Hidden-node lookup: it walks the whole loaded tree once per drag, and only when something selected is hidden. That's fine. Just noting it is O(loaded nodes) and runs on the UI thread at drag start. It would only matter for very large expanded-then-collapsed trees.
  • Dialog style: the inline dragError dialog in CodebaseContent.kt repeats the Surface/Column/OK layout. If an error or info dialog helper already exists in this file, consider reusing it.

Conventions checklist

  • ✅ Compose Multiplatform / AWT desktop APIs only (androidx.compose.ui.draganddrop, java.awt.datatransfer); no Android APIs.
  • ✅ Null-safe providers: onSelectProject becomes null when projectDataProvider is missing, recentProjects falls back to an empty flow, and picker and selection failures are caught and logged.
  • ✅ Version changed only in build.gradle.kts.
  • ✅ No changed Kotlin file is missing its trailing newline.
  • ✅ The slf4j-simple test-runtime dependency is a sensible fix for the BossLogger static-init failure in tests.

Tests
Unit coverage is good: ordering, compacted chains, deduplication, special characters, defensive copying of the list, Copy-only, rejection, and the full C0/C1 set. Nothing tests the gesture wiring itself: right-click, a click without movement, and drag-versus-combinedClickable. That is acceptable for a draft as long as the manual checklist is done; the right-button case in point 2 is the one I'd prioritise.

Nice work overall. Once it's rebased onto #20 and the right-drag behaviour is confirmed, this looks close to ready.

@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review: drag explorer files into terminals

This review covers the drag-and-drop changes (FileTreeDrag.kt, the FileTreeItem and CodebaseContent wiring, the README, and FileTreeDragTest). The project-switcher commits from #20 come along on this branch; I've left those for #20's own review.

Overall this is a careful, well-scoped change. The payload is native javaFileListFlavor only, offers Copy only, never reads file contents or sends Enter, and rejects the whole selection instead of sending part of it. That's the right shape for a terminal drop target, and the tests cover the pieces that matter: ordering, compacted chains, special characters, copy-only, control characters, and the defensive copy.

Conventions ✅

  • Version changed only in build.gradle.kts (1.6.1 → 1.7.1, following feat(ui): switch projects from the panel header #20's 1.7.0). plugin.json is untouched.
  • Every changed Kotlin file ends with a newline. The diff has no "No newline at end of file" markers.
  • Only Compose Multiplatform and AWT APIs (androidx.compose.foundation.draganddrop, java.awt.datatransfer). Nothing Android-specific.
  • Null providers: the drag path doesn't touch any provider. onDragTransferData defaults to { null }, which turns dragging off cleanly.

Things worth looking at

1. Which mouse button can start a drag (CodebaseContent.kt, .dragAndDropSource { … })
In CMP, dragAndDropSource's default start detector is built on detectDragGestures, which as far as I know doesn't check which button is pressed. So right-click plus a little mouse movement could start a native drag that competes with the host's context-menu gesture. Your manual checklist already covers this, which is good. If it does misbehave, one fix is a custom start detector that only starts on the primary button. Another is to return null from the lambda when the press wasn't primary, which needs the last pointer event buttons tracked in the existing pointerInput.

2. A modal dialog opens while the drag gesture is still in progress
dragError is set inside the transfer-data callback, which runs when the drag starts while the button is still held down. A BossDialog that opens mid-gesture can grab focus or the pointer in odd ways, especially on macOS, where the release event may go to the dialog. Your checklist covers this too. If it feels rough, a lighter signal would fit a gesture that the code is cancelling anyway: an inline banner or toast above the tree, or deferring the dialog until the pointer is released.

3. Hidden selections are ordered as strings, not in tree order
remaining.sorted() compares path strings, so foo-bar.txt sorts before foo/x.txt because - (0x2D) < / (0x2F), while the tree shows foo/ first. The README says "path order", so it's documented and predictable. If you want hidden items in tree order, you could collect them during the depth-first traversal you already do in paths() (pre-order, children in display order) and avoid the separate sort. Not blocking.

4. A stale hidden selection cancels the whole drag
If a collapsed folder reloads (refresh, rename, or a file watcher event) and a selected descendant disappears, nodesByPath[it] ?: return null rejects everything. That matches the "never a partial payload" rule and the message tells the user what to do. Just confirm that's the UX you want, versus dropping entries that no longer exist. Those can't be dragged meaningfully anyway, and the selection probably ought to be pruned when they vanish.

5. Small points

  • The transferable() check is a little hard to read with the multi-line lambda inside the if. Pulling out private fun isDraggable(path: String) = path.none(Char::isISOControl) && File(path).isAbsolute would make it read more easily.
  • onDragTransferData captures selectedPaths, rows, and tree from the current composition. That's correct as long as the dragAndDropSource node picks up the newest lambda on recomposition, which it does in CMP 1.10. I mention it only because a press, then a selection change, then a drag in the same frame would use the previous snapshot. That's harmless.
  • The tree traversal is O(n) only when a hidden selection exists, and it runs once per drag start. Fine.

Security

Rejecting ISO control characters (including C1 like \u009b CSI and \u0085) and relative paths is a good defence against terminal injection through file names. Shell metacharacters such as $(…), ;, and quotes are passed through as File objects, and the terminal plugin's receiver handles the quoting. Your test with $(echo nope);&.txt shows that hand-off works, but it also means correct quoting depends entirely on the terminal plugin. As you noted, the receiver uses POSIX quoting, so Windows needs verifying before this is called cross-platform.

Tests

The unit coverage is good. One gap: no test covers the Compose wiring itself, i.e. that a drag on a selected row keeps the multi-selection rather than combinedClickable collapsing it on press. A UI test is probably overkill here, so the manual checklist is the right place for it.

Summary

No blocking problems in the drag code. The main risks are gesture interactions (secondary-button drags, the dialog opening mid-drag), and your manual checklist already targets them. Once #20 lands and those checks pass, this looks good to merge. 👍

@kshivang
kshivang marked this pull request as ready for review October 6, 2026 07:50
@kshivang
kshivang merged commit 621db0b into main Oct 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(files): drag files from the explorer into Claude Code and Codex terminals

2 participants