Skip to content

feat(explorer): add optional host filesystem navigation - #23934

Open
Chihen-Tai wants to merge 22 commits into
stablyai:mainfrom
Chihen-Tai:feat/explorer-host-mode
Open

Chihen-Tai wants to merge 22 commits into
stablyai:mainfrom
Chihen-Tai:feat/explorer-host-mode

Conversation

@Chihen-Tai

@Chihen-Tai Chihen-Tai commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

The file explorer can only show the project folder. This adds a "Host" button that lets you walk up out of the project and browse the rest of the machine, like an SFTP browser. It works for local and SSH projects. One click brings you straight back to the project.

What Changed

Before: the right-sidebar Explorer is rooted at the workspace. From /home/allen/codes you cannot reach /home/allen or its siblings (Documents, other projects). This hurts most on SSH hosts, where the Explorer is the easiest way to look around.

After: a hard-drive toggle in the Explorer toolbar enters Host mode.

  • The body becomes a folder listing that starts at the workspace root. It has a "Parent directory" row, a host label, a refresh button, and clickable breadcrumbs (Up, ~, each path segment).
  • Clicking a folder enters it. Clicking a file opens it. Large folders (tens of thousands of entries) stay responsive: the list is virtualized.
  • Up from / or from a UNC share root does nothing. Up from a Windows drive root (C:\) shows the drive list, as the remote folder picker already does.
  • Search:
    • Names filters only the folder on screen ("Find in this folder").
    • Asking for a workspace Contents search from anywhere (the shortcut, the command palette, "find in folder", or the Contents tab) returns to the project and runs the search there. The UI never shows one folder while searching another.
  • The toolbar button or "Return to workspace root" restores the tree exactly as it was, including expanded folders.
  • Host mode is session-only: leaving the workspace, the workspace's host changing or becoming unavailable, or restarting Orca returns to the project view.

Project mode is unchanged.

Mechanism

  • Separate pane. Host mode is a read-only pane layered over the tree (the tree is made inert), not an extension of the tree. The tree's cache, watchers, git decorations and search stay bound to the workspace, and Host mode adds no watchers or searches of its own.
  • Listing reuses the existing any-directory browse paths:
    • SSH: ssh:browseDir.
    • Local: a new fs:browseHostDir. It calls the same function files.browseServerDir already uses (extracted as browseServerDirectory) and returns names and types only. It never touches the external-path allowlist.
  • Opening a file goes through a new fs:resolveHostBrowseEntry in main. It runs realpath, then stat, and reports whether the canonical path is inside the canonical workspace root. Ownership follows that canonical verdict, so a workspace symlink pointing outside stays read-only.
    • A symlink to a folder is browsed, never granted.
    • Locally, only a regular file outside the workspace is granted, and only that one file. It gets the same session read/write path grant that terminal links and View Log already use.
    • Over SSH, main only classifies the entry and grants nothing.
  • Where the file opens. Files inside the workspace, including ones reached through a symlink, open through the normal writable Explorer path, so no duplicate tab appears. Files outside the workspace open read-only (readOnly is an editor property; main does not know about it). They are not persisted: neither the file nor its tab chrome. Opening a file that is already an open View Log tab only focuses that tab.
  • State. Host-mode state lives in a small context provider below FileExplorerFiles. Toggling re-renders only the Host controls, not the tree. While the Host overlay is shown, the tree's keyboard shortcuts stand down and the covered Project search inputs are inert, so no keystroke can act on the hidden tree or run a hidden workspace search.
  • Paths. Path handling reuses the remote folder picker's helpers, extended for UNC and WSL paths (\\server\share, \\wsl.localhost\Distro, \\?\ prefixes).

Why

Why not grow the tree itself. Widening the tree (for example, extending the sparse-checkout root selector upward) would pull every workspace-bound system along with it:

  • recursive watchers over ~ or /, which on SSH compete for the relay's watch capacity;
  • rg over a whole home directory, including .ssh and token files;
  • git decorations and file operations outside any repository.

A separate flat pane makes those impossible by construction, and it matches how SFTP browsers behave.

Why no parent-folder grants. The obvious shortcut is to call fs:authorizeExternalPath on each parent folder. That grant covers a whole subtree, so it would silently make ~ (or /) readable, writable and deletable for the rest of the session. Host mode never grants a folder.

Trust model, stated plainly

  • Local:
    • The renderer can already call fs:authorizeExternalPath on any path, and files.browseServerDir already lists any folder for a paired device.
    • A settings gate on a names-only listing would add no real boundary, because a compromised renderer can grant itself any path anyway. So there is none; the toolbar toggle is the explicit user action.
    • What this PR does guarantee: browsing never widens grants, and each click grants at most the one regular file clicked, outside the workspace.
  • SSH: the relay has no filesystem allowlist by design (src/relay/context.ts), so no client-side allowlist is added. It would only be false reassurance. (The docs/relay-fs-allowlist-removal.md that comment references is not in the repo.)

Worst case if something is wrong

  • Security: no folder subtree is ever granted by this feature.
  • Data: a clicked file outside the workspace gets the same session read/write grant as a terminal link. Orca opens it read-only, and it only becomes editable through an existing ordinary open path (Quick Open, a terminal link) or a renderer that is already compromised.

Switching cost. In the first version, Host state lived in FileExplorerFiles. Every toggle re-rendered every tree row: an 86–110 ms long task in the dev build on this repository, plus the Host bar pushing the search box down twice. Moving the state into a provider and layering the Host bar inside the overlay fixed both. Dev build, this repository, 75 tree rows, three enters and exits:

Before After
Long tasks over 50 ms 86, 90, 110 ms none
Click to second frame (enter / exit) 113–133 / 55–67 ms 10–45 ms
Layout shifts, Names view 9 (sum 0.039) 0
Spinner flashes on local listings 6 0 (spinners wait 200 ms)

Two cases still shift the layout once per toggle, by design: a sparse checkout hides its scope row (sum 0.021 over six toggles), and the Contents view hides its filter row (0.058). Leaving an empty gap there looked worse than one short shift. Entering Host mode fades the listing in; leaving is instant, because the tree underneath is already rendered.

Linked Issue

No existing issue covers this; happy to open one if you prefer.

Visual Proof

Captured from the dev app against a throwaway fixture home at /tmp/orca-demo/home/allen (codes is the workspace; datasets links to a folder elsewhere and a-link.ts links back into the workspace).

Project mode (unchanged) Host mode, at the workspace Host mode, one level up
project host at workspace host parent
Symlinked folder is browsed, not granted Contents tab returns to the project search
symlink dir contents

A file outside the workspace opens read-only:

outside file read-only

Testing

Manual, macOS, local workspace (dev app driven through CDP, isolated profile and home, workspace added through the /tmp alias so its path is not canonical):

Scenario Result
Project mode No parent row; tree unchanged
Before Host mode Reading the home folder or notes.txt is denied
Up one level, breadcrumbs Siblings (Documents, …) listed; each segment navigates
Click a symlinked folder Its contents are listed; reading that folder is still denied afterwards
Open notes.txt outside the workspace Read-only tab; typing plus Cmd+S leaves the file's bytes and mtime unchanged; only that file becomes readable, its folder and siblings stay denied
Click a symlink into the workspace Reuses the one writable tab at the workspace path, no duplicate
Delete, F2, Cmd+Backspace, typing while the Host list is focused The tree's selected file is untouched; Tab never reaches the hidden tree or the covered Project search inputs; focus() on those inputs fails
Return to workspace root Tree and expanded folders restored
Quit with a Host tab active, relaunch The Host tab is gone, the workspace tab is restored with its content, no dangling tab references

SSH was not exercised manually. The SSH paths (ssh:browseDir listing, provider realpath/stat classification, externalSshTargetId for outside files) are covered by unit tests with a mocked provider.

Automated

  • filesystem-host-browse-handlers.test.ts: real temp folders plus a mocked SSH provider.
  • Host-mode logic, file opening, browsing hook, list, bar, toolbar, the FileExplorer overlay, and the mode hook with a real zustand store.
  • UNC and WSL path cases.
  • Read-only tab merging, session persistence, and a full persist/restore round trip on the real store.

Guards checked by removing them: each guard below was removed once, and the named test failed.

  • A grant on browse or on a folder: main handler tests.
  • The Names filter reaching the workspace file list: host-mode tests.
  • Allowing SSH Host mode on a client without the desktop browse APIs: host-mode tests.
  • Unmounting the tree, or re-rendering it on toggle: FileExplorer overlay tests.
  • The covered Project search inputs not being inert: FileExplorer overlay tests.
  • Dropping readOnly, or persisting Host tabs or their tab chrome: open and session tests.
  • Ignoring canonical workspace ownership, in the renderer or in main: open-plan and handler tests.
  • A Host open ending a View Log live tail: read-only tab tests.
  • A late click result pulling the user back after they navigated away, or opening after the visit ended: browsing-hook tests.
  • A visit surviving a change of workspace or host, or reusing an old listing on re-entry: host-mode hook tests.
  • A failed folder open jumping two levels on "up": browsing-hook tests.
  • Granting files that are already inside the workspace: handler tests.
  • The key guard on the Host list, virtualization, scroll reset, the UNC parent rule, the null-byte check on SSH paths, the disabled toggle when unavailable, and leaving Host mode on a search request: their own tests.

Three of these first passed when they should not have. The first virtualization check left the list virtualized, so it now replaces the list with a plain render and the 5,000-entry test fails. The first tree-remount test never re-rendered, because FileExplorer is memoized with no props, so it now toggles through hook state. The null-byte check was only exercised on the local path, where Node rejects \0 before the guard runs, so an SSH-path test was added. All three now fail when the guard is removed.

Checks run:

  • pnpm tc (node, cli, web).
  • oxlint.
  • check:code-quality:changed.
  • Localization extraction and coverage.
  • Vitest over the touched areas (right sidebar, sidebar, store, lib, runtime, main IPC, preload, shared).
  • build:electron-vite.
  • The full Vitest suite: every failure also fails on a clean main checkout, or passes when run alone (timing-sensitive under full-suite load). None of the failing files are touched by this PR.

verify:localization-runtime-catalog fails the same way on main (sparsePreset.findPath):

git worktree add /tmp/orca-main main && (cd /tmp/orca-main && node config/scripts/generate-runtime-required-english-catalog.mjs); git worktree remove /tmp/orca-main
  • I manually tested these changes locally (macOS local workspace, driven through CDP; SSH not manually tested)
  • Automated tests added/updated

AI Disclosure

Implemented and tested with Claude Code (Claude Opus 5.5). A second Claude session reviewed each step adversarially, and the manual checks above were driven by Claude Code through CDP.

Notes

Known limitations

  • authorizeExternalPath grants a path's subtree. If a granted file is later replaced by a folder, that folder becomes accessible. This existing behavior is shared with terminal links and View Log, and is unchanged here.
  • SSH listing uses ls -1Ap:
    • A file name containing a newline is mis-split; clicking it only fails and grants nothing.
    • Symlinks are not marked in the listing, so every non-folder is classified on click.
  • A symlink inside the workspace that points outside opens writable from the Project tree (existing symlink activation, unchanged here) but read-only from Host mode, so the same file can appear in two tabs.
  • The Host list has Tab focus but no arrow-key navigation, and it is virtualized: a Tab-focused row that scrolls out of view unmounts and focus falls back to the page.
  • Leaving the workspace, or its host changing or dropping out (for example while an SSH owner is briefly unresolved), ends Host mode; entering again starts at the workspace root.
  • Deep paths squeeze the breadcrumb segments. This is the existing remote folder picker component, unchanged here.

Follow-ups

  • Host browsing for paired Orca servers (needs a server-side realpath/stat and file opening outside the worktree).
  • Symlink-aware SSH listing.
  • Paged or streamed directory listings for very large folders (the listing call is shared with the remote folder picker).
  • Arrow-key navigation and focus retention for the virtualized Host list.
  • Going Up should select the child folder you just left (as Finder and MobaXterm do); today the list starts at the top.
  • When the active tab is a Host tab at quit, restore falls back to a terminal tab rather than the most recent editor tab.
  • Write operations in Host mode, which need their own design (grants, undo/redo, trash outside roots).

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

  • This PR is small and focused (one feature, but it touches main, preload, renderer and editor state)
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (typecheck, changed-code lint and the touched test areas pass locally; full lint, full test and build left to CI)

🤖 Generated with Claude Code

Chihen-Tai and others added 6 commits September 30, 2026 04:46
Adds a Host mode to the right-sidebar File Explorer that browses the
workspace host's filesystem like an SFTP browser: parent directory,
breadcrumbs, sibling folders, and a one-click return to the workspace
root. Project mode is unchanged.

Host mode is a read-only flat list layered over the tree, so the tree's
caches, watchers, git decorations and search stay bound to the workspace.
Listing reuses the existing any-directory browse paths (ssh:browseDir,
files.browseServerDir) plus a local fs:browseHostDir that returns names
only and never touches the external-path allowlist. Opening a file
resolves it in main: only a regular file outside the workspace gets a
single-file grant, and it opens read-only and is never persisted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…bservable

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hydration keeps editor tab chrome even without a backing file, so a
Host-mode tab excluded from openFilesByWorktree still came back as an
empty tab after restart. Filter those tabs where unified tabs are
persisted, and cover the full persist/restore round trip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Host-mode state lived in FileExplorerFiles, so every toggle re-rendered
the whole tree (~110 ms long task in dev on a large folder) and the Host
bar was inserted into the flow, shifting the query strip twice.

Move the state into a provider whose children bail out, render the Host
bar inside the tree overlay, cover the query row in place, delay visible
spinners by 200 ms, and dim the current folder during slow navigation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…for workspace searches

The Host overlay covered the Project Names/Contents inputs without making
them inert, so Tab or a search focus request could land in them and run a
workspace search behind the Host view. Make them inert, and return to
Project mode whenever a workspace Contents search is requested (shortcut,
command, Contents tab) so the search is always visible. Split the context
value into its own module so Fast Refresh keeps working.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 22:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds optional host filesystem browsing to the file explorer.

The PR appears safe to merge; no new actionable issue or outstanding previous finding was identified.

Summary

The PR adds a session-only Host filesystem pane to the desktop Explorer. It lists local and SSH directories separately from the workspace tree, classifies clicked files before opening them, and keeps external-file tabs read-only and out of session persistence.

  • Host visits are keyed to the workspace and SSH connection ID.
  • The project tree remains mounted and inert while the Host pane is visible.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Explorer -->|Enter Host mode| HostPane
  HostPane -->|List directory| BrowseAPI
  HostPane -->|Click entry| Resolver
  Resolver -->|Directory| HostPane
  Resolver -->|Workspace file| ProjectTab
  Resolver -->|External file| ReadOnlyTab
  HostPane -->|Return| Explorer
Loading

Reviews (6) · Last reviewed commit: "refactor(explorer): trim Host mode code ..."

Comment thread src/renderer/src/components/right-sidebar/file-explorer-host-mode.ts Outdated
Comment thread src/renderer/src/components/right-sidebar/file-explorer-host-mode.ts Outdated
Comment thread src/renderer/src/components/right-sidebar/FileExplorerHostList.tsx
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

The file explorer now supports host filesystem browsing through local and SSH sources. The change adds directory listing, entry resolution, navigation, filtering, and file-opening behavior, with Host mode controls in the explorer. Host-browse files are excluded from persisted sessions. Remote browsing also adds UNC path parsing and navigation support.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 95990

Changing hosts while browsing can leave entries from the previous host visible and open a path on the wrong host. Bind browsing sessions to host identity; otherwise the change is mergeable with this bounded issue acknowledged.

Security architecture risk: 🟡 Moderate · up to 95990

Canonical path checks and read-only, session-only previews provide useful safeguards. However, machine identity is not consistently included in browsing and tab state, creating a conditional risk of opening a path against the wrong machine during an ownership change. Downstream authorization coverage remains incomplete.

Retained concerns

  • Medium · security · inferred: Host browser state and editor tab reuse are not consistently scoped to execution authority. If a non-null local or SSH owner changes while the same worktree, path, and component remain active, the old listing and outstanding operations retain their generations, but subsequent activation uses the new source. Same-path editor tabs are matched by worktree and runtime environment rather than SSH target; a reused tab can receive new connection metadata while retaining its existing dirty or writable state. This creates conditional wrong-host access and tab-ownership confusion through the new Host caller. Production remount behavior and an end-to-end unintended mutation remain unverified.
Security review details

Security Blast Radius

  • inferred — The intended access scope is the local user's host filesystem or the selected connected SSH account, rather than only the workspace. The identified identity concern is bounded to authorities already available to the application and same-worktree/path transitions; no privilege elevation, cross-tenant access, or access to an unconnected host was established.

Security Findings and Attack Paths

  • inferred — A conditional wrong-host path exists when an active visit survives an authority change: names retained from one host are resolved using another host, and same-path tab reuse can update SSH metadata without separating tab identity. A malicious host could influence displayed entry names, but exploitation also requires the relevant ownership transition and user activation. Production remount behavior and a completed disclosure or mutation path were not verified.

Trust Boundaries and Controls

  • observed — Canonical containment separates workspace opens from external previews. Read-only preview metadata is a renderer contract, not a distinct filesystem capability: local read and write handlers both use path authorization, while SSH handlers delegate to the selected provider. The inspected immediate parent already exposed general external-path authorization; backend mutation authority was not established as a new PR vulnerability.
  • observed — Editor mutation context checks external SSH target consistency and requires an SSH connection generation. This is counterevidence against arbitrary cross-host writes, although it does not establish isolation of retained browsing state or cross-authority tab reuse.

Resilience and Maintainability Implications

  • observed — Persistence excludes Host-browse files and their unified tabs, and read-only hydration ignores dirty drafts. These controls prevent ordinary preview restoration from reintroducing writable draft state, but depend on preserving correct tab identity and markers.

Hardening Proposals

  • proposed — Bind Host visits, asynchronous generations, and external tab identity to execution authority, including the SSH target. On authority change or teardown, invalidate pending operations and require a fresh listing. Preserve dirty-tab compatibility only when the incoming open names the same authority.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.51% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 39 files. (6 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: optional host filesystem navigation in the Explorer.
Description check ✅ Passed The description is detailed and covers the user impact, implementation, rationale, security model, testing, visual proof, limitations, and checklist. The required Linked Issue section is incomplete be…
Full details: Docstring Coverage

Explanation

Docstring coverage is 29.51% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 39 files. (6 skipped: 6 unsupported.)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Store captured provenance when a Host tab becomes writable. · open-file-apply.ts:161-163

src/renderer/src/store/slices/editor/actions/open-file-apply.ts:161-163
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Store captured provenance when a Host tab becomes writable.

For a local Host tab, the writable reopen captures operationProvenance, but refreshExternalSshProvenance is false because externalSshTargetId is absent. The existing-tab update therefore keeps the old undefined value and drops the capture. Later save and rename paths recapture ownership instead of using the snapshot taken when the tab became writable.

Preserve existing provenance, but store the captured value when the existing tab has none.

🐛 Suggested fix
               operationProvenance: refreshExternalSshProvenance
                 ? operationProvenance
-                : f.operationProvenance,
+                : (f.operationProvenance ?? operationProvenance),

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 3e7fe2da-e6fe-4f1e-9b74-29bc663a3c69

📥 Commits

Reviewing files that changed from the base of the PR and between 2807332 and d1cad27.

📒 Files selected for processing (43)
  • src/main/ipc/filesystem.ts
  • src/main/ipc/filesystem/filesystem-host-browse-handlers.test.ts
  • src/main/ipc/filesystem/filesystem-host-browse-handlers.ts
  • src/main/runtime/runtime-server-environment-commands.ts
  • src/preload/api/filesystem-api.ts
  • src/preload/api/fs-bridge.ts
  • src/renderer/src/components/right-sidebar/FileExplorer.host-mode.test.tsx
  • src/renderer/src/components/right-sidebar/FileExplorer.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerHostBar.test.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerHostBar.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerHostList.test.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerHostList.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerHostQueryRow.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerToolbar.test.tsx
  • src/renderer/src/components/right-sidebar/FileExplorerToolbar.tsx
  • src/renderer/src/components/right-sidebar/file-explorer-host-mode-context-value.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-mode-context.tsx
  • src/renderer/src/components/right-sidebar/file-explorer-host-mode.test.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-mode.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-open.test.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-open.ts
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.test.tsx
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.ts
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-mode.test.tsx
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-mode.ts
  • src/renderer/src/components/sidebar/RemoteFileBrowserBreadcrumbs.tsx
  • src/renderer/src/components/sidebar/remote-file-browser-drive-paths.test.ts
  • src/renderer/src/components/sidebar/remote-file-browser-drive-paths.ts
  • src/renderer/src/components/sidebar/remote-file-browser-helpers.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/lib/workspace-session-editor-drafts.test.ts
  • src/renderer/src/lib/workspace-session-unified-tabs.ts
  • src/renderer/src/lib/workspace-session.ts
  • src/renderer/src/store/slices/editor-read-only-tabs.test.ts
  • src/renderer/src/store/slices/editor-session-host-browse-restore.test.ts
  • src/renderer/src/store/slices/editor/actions/open-file-apply.ts
  • src/renderer/src/store/slices/editor/types/open-file.ts
  • src/shared/filesystem-entry-types.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/renderer/src/components/sidebar/remote-file-browser-drive-paths.ts Outdated
…on desktop APIs

- A workspace symlink that points outside now opens read-only: local/SSH
  ownership follows main's canonical verdict; only paired servers fall back
  to the literal path.
- The web client stubs ssh.browseDir, so SSH Host mode now requires the
  desktop browse and resolve APIs, like local.
- Up from a UNC share root without a trailing separator is a no-op.
- A read-only tab reopened writable keeps the provenance captured at that
  moment; restored tabs still resolve their owner lazily.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Host mode hides the Project tree but leaves its keyboard shortcuts live. The Host toggle keeps focus inside the Explorer shell (data-orca-explorer-shell), and useFileExplorerKeys listens on window in the capture phase while only ignoring targets marked data-ignore-file-explorer-keys — which the Host list/bar/filter have, but the toolbar does not. So immediately after entering Host mode, Delete/Enter/arrow keys act on the hidden tree's selection (local Delete goes to Trash with no confirmation, Enter starts a rename). The inert boundary blocks focus, not these ref-driven shortcuts. Details inline.

Reviewed changes

  • Host pane and state: new read-only overlay (FileExplorerHostOverlay/HostBar/HostList/HostQueryRow), a Host-mode context provider, a toolbar hard-drive toggle, and inert boundaries that leave the Project tree mounted but covered.
  • Main-process browse/resolve: new fs:browseHostDir (names/types only) and fs:resolveHostBrowseEntry (realpath + stat; grants exactly the clicked regular file locally, SSH only classifies); browseServerDirectory extracted from RuntimeServerEnvironmentCommands and reused.
  • Open/planning logic: file-explorer-host-mode.ts, file-explorer-host-open.ts, use-file-explorer-host-browser.ts, use-file-explorer-host-mode.ts route local/SSH/runtime sources; workspace files open writable, outside files open read-only with hostBrowse: true.
  • Session-only tabs: hostBrowse excluded from buildEditorSessionData and buildPersistedUnifiedTabSessionData; applyOpenFileToState never promotes it onto an existing tab and clears it on a normal writable open.
  • UNC/WSL/long-path paths: uncRootOf, the unc branch in splitBrowsePath, parentOfUncPath, parentPath, joinPath, plus reworked RemoteFileBrowserBreadcrumbs.
  • i18n: fileExplorer.host.* across en/es/fr/ja/ko/zh.

ℹ️ Host-tab exclusion from the incremental session patch is untested

The exclusion itself holds — every Host open goes through openFile → openWorkspaceEditorItem, which always reallocates groupsByWorktree/unifiedTabsByWorktree, so buildWorkspaceSessionPatch always rebuilds the unified tabs and runs the sessionOnlyEditorIds filter. But no test exercises that path with a live Host tab: vanished full-payload test is the only one pinning both filters, the "rewrites the tab chrome" test only checks the reopen-writable direction, and workspace-session-editor-drafts.test.ts never populates unified tabs. The production open also uses preview: true while the restore fixture uses preview: false, so preview replacement of a Host file is untested.

Technical details
# Host-tab session exclusion relies on an untested coupling

## Affected sites
- `src/renderer/src/lib/workspace-session-unified-tabs.ts` — filter is correct but only reached via the `groupsByWorktree`/`unifiedTabsByWorktree` reference change, not `openFiles`.
- `src/renderer/src/store/slices/editor-session-host-browse-restore.test.ts` — builds full payloads, never `buildWorkspaceSessionPatch`, and opens the Host file with `preview: false`.
- `src/renderer/src/components/right-sidebar/../lib/workspace-session-editor-drafts.test.ts` — asserts only `openFilesByWorktree`/`activeFileIdByWorktree` with `unifiedTabsByWorktree: {}`.

## Required outcome
- A test that opens a Host file, builds `buildWorkspaceSessionPatch(state, changedFields)` from the live delta, and asserts `patch.unifiedTabs` and `patch.openFilesByWorktree` exclude the Host entity/file.

## Suggested approach (optional)
Mirror the host file's production open options (`preview: true`) so preview replacement is covered too.

ℹ️ Nitpicks

  • src/preload/api/filesystem-api.ts says resolveHostBrowseEntry "grants a local read only when the target resolves to a regular file". The grant is authorizeExternalPath, i.e. full path authorization (the file is also writable/deletable via fs:writeFile/fs:deletePath); only the editor tab is read-only. Consider tightening the wording.
  • The fileExplorer.host.contentsUnavailable copy ("Return to the workspace root to use it") is slightly off: clicking the Contents tab actually leaves Host mode and runs the workspace search, so "return to the workspace root" could read as a manual step.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread src/renderer/src/components/right-sidebar/FileExplorerToolbar.tsx

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The follow-up commit is a strict improvement and introduces no new issues. The two inline findings from the prior review (toolbar keyboard fallback into the hidden tree; Host re-activation silently re-listing the root) are not touched by this commit and remain open.

Reviewed changes (delta since d1cad27)

  • Canonical ownership now decides Host opens for local/SSH: planHostFileOpen uses main's workspaceRelativePath for non-runtime sources and the literal containment check only for paired runtimes. A workspace symlink that leaves the workspace (canonical verdict null) now stays read-only instead of opening through the writable workspace path — this closes the inconsistency called out in the prior review.
  • SSH Host mode requires the desktop APIs: getHostBrowseAvailability now gates the ssh owner on both browseHostDir and resolveHostBrowseEntry (the web client stubs ssh.browseDir), while runtime browsing stays available without them. use-file-explorer-host-mode.ts passes the combined check.
  • UNC root Up fix: parentOfUncPath returns its input when the share root has no segments, so parentPath('\\server\share') / parentPath('\\?\UNC\server\share') now equal the input and Up is correctly disabled at the root instead of re-listing the same folder.
  • Provenance on Host-tab promotion: applyOpenFileToState now captures and stores owner provenance when a read-only tab turns writable, without disturbing hydrated writable tabs that intentionally resolve their owner lazily.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…ost, revalidate on return

- Render Host folders through the shared VirtualizedList and drop the
  2,000-row cap: 50k-entry folders enter and filter without long tasks.
- Explorer tree shortcuts stand down while the Host overlay is shown, so
  Delete/F2 with focus on the toolbar or body cannot act on the hidden
  tree's selection.
- Reactivating Host mode without re-entering keeps the visited folder and
  revalidates it (reissuing an interrupted navigation) instead of snapping
  back to the workspace root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Chihen-Tai

Copy link
Copy Markdown
Contributor Author

@coderabbitai Re the outside-diff provenance note: fixed in b90c711, narrowed to the read-only → writable transition. Restored tabs without provenance keep resolving their owner lazily (a blanket ?? would freeze a snapshot on any reopen and make a later owner change fail saves). Tests "captures owner provenance when a Host tab turns writable" and "keeps restored writable tabs resolving their owner lazily when reopened".

Docstring coverage: this repo's AGENTS.md asks for "Concise/Brief Non-obvious Comments ONLY", so functions carry short "Why" comments instead of docstrings.

@Chihen-Tai

Copy link
Copy Markdown
Contributor Author

@greptileai review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes (delta since b90c711)

  • Host listings are virtualized: FileExplorerHostList now renders through VirtualizedList (mounting only the viewport plus overscan) and drops the HOST_LIST_RENDER_LIMIT cap and the fileExplorer.host.truncated string from all six locales; filtering runs through useDeferredValue.
  • Tree shortcuts stand down under Host mode: a new isExplorerTreeCoveredByHostMode gate at the top of the useFileExplorerKeys handler returns early whenever the Host overlay is in the explorer shell, so Delete/rename/arrow shortcuts can no longer act on the hidden tree selection regardless of where focus sits. This closes the prior review's important toolbar finding; the overlay div now carries data-file-explorer-host-overlay.
  • Reactivation revalidates instead of resetting: useFileExplorerHostBrowser's activation effect keeps the last visited folder (or an interrupted pendingPath) and silently re-lists it in the background; only an explicit enter() resets to the workspace root.
  • \\?\C: root is terminal: parentPath('\\?\C:', 'win32') now equals its input, so Up is disabled at an extended-length drive root.

All three prior Pullfrog threads are addressed by these commits.

ℹ️ Stale known-limitation in the PR description

The description still lists "The list renders at most 2,000 entries per folder; the Names filter narrows it" under Known limitations, but this commit removes that cap by virtualizing the list (and deletes the fileExplorer.host.truncated copy). Worth dropping or rewording so reviewers don't look for behavior that no longer exists.

Technical details
# Stale PR-description limitation
## Required outcome
- Remove or rewrite the "renders at most 2,000 entries per folder" Known-limitations bullet to reflect viewport-only windowing.
## Suggested approach (optional)
- Mention that very large folders now mount only the visible window.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread src/renderer/src/components/right-sidebar/FileExplorerHostList.tsx
The virtualized Host list reuses one scroller, so a deep scroll carried
over into the next folder or filter result. Reset it when the listed path
or filter changes; revalidating the same folder keeps the user's place.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
ast-grep outline src/renderer/src/store/slices/editor/actions/open-file-apply.ts
ast-grep outline src/renderer/src/store/slices/editor-read-only-tabs.test.ts
rg -n -C 12 'operationProvenance|readOnly|hostBrowse' src/renderer/src/store/slices/editor/actions/open-file-apply.ts
rg -n -A 65 -B 5 'captures owner provenance when a Host tab turns writable|keeps restored writable tabs resolving their owner lazily when reopened' src/renderer/src/store/slices/editor-read-only-tabs.test.ts
fd -H '^AGENTS\.md$' --exec rg -n -C 5 'Concise|Brief|Non-obvious|docstring|Why' {}

Length of output: 14235


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '115,170p' src/renderer/src/store/slices/editor/actions/open-file-apply.ts
sed -n '263,308p' src/renderer/src/store/slices/editor-read-only-tabs.test.ts
rg -n -A 4 'Concise/Brief Non-obvious Comments ONLY' AGENTS.md

Length of output: 5034


@Chihen-Tai, the narrowed fix addresses the provenance concern. Missing provenance is retained on the read-only → writable transition, while restored writable tabs remain without provenance on reopen. The existing external-SSH refresh behavior remains unchanged.

Both named tests contain the expected assertions. I inspected the tests but did not run them.

Thanks for the guidance on comments. AGENTS.md requires concise, non-obvious “Why” comments. My docstring coverage request was unnecessary; I withdraw it.


✏️ Learnings added
Learnt from: Chihen-Tai
URL: https://github.com/stablyai/orca/pull/23934

Timestamp: 2026-09-29T22:39:15.624Z
Learning: In stablyai/orca, src/renderer/src/store/slices/editor/actions/open-file-apply.ts must preserve lazy owner resolution for restored writable tabs without operationProvenance. Reopening such a tab must not attach captured provenance through a blanket fallback. Missing provenance is retained when an existing read-only tab becomes writable; the separate external-SSH provenance refresh remains applicable.

Learnt from: Chihen-Tai
URL: https://github.com/stablyai/orca/pull/23934

Timestamp: 2026-09-29T22:39:15.624Z
Learning: In stablyai/orca, AGENTS.md requires "Concise/Brief Non-obvious Comments ONLY": explain why, not how, and use one line when possible. Do not request docstrings solely for documentation coverage when concise non-obvious "Why" comments satisfy this guidance.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes (delta since the prior pullfrog review at 1f63af93)

  • Host list scroll reset: FileExplorerHostList gained a useLayoutEffect keyed on [scrollElement, resolvedPath, deferredQuery] that sets the scroller's scrollTop to 0, so entering a different folder or changing the Names filter starts at the top while a same-folder revalidation keeps the user's place.
  • Regression test: a new FileExplorerHostList case covers a new folder, a changed filter, and a same-folder revalidation. I confirmed it fails when the effect is removed (scroll stays at 500), so the assertion is not theatre.

The effect's deps are the right ones: resolvedPath is a value string, so an entry-list refresh or a silent reactivation re-list of the same folder leaves scroll untouched, and scrollElement is a stable setState from useState, so no spurious reset on ordinary renders.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Chihen-Tai and others added 5 commits September 30, 2026 12:35
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ing the workspace

- Drop paired-server Host browsing: it could not open files outside the
  worktree, had no realpath, and was disabled over SSH. Host mode now reports
  one generic 'not available for this workspace' state.
- Leaving the workspace ends Host mode; each entry starts a fresh session at
  the workspace root, replacing the retain-and-revalidate machinery.
- Simplify the host label to local and SSH.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n leave

Also drop the unused source-null clear: active already requires a source.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A briefly unresolved owner (reconnect, hydration) used to resume the same
visit and flash its stale folder; ending the visit keeps it simple. Tested.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.ts Outdated
Chihen-Tai and others added 2 commits September 30, 2026 19:20
…sits on host change

- Register fs:browseHostDir directly on browseServerDirectory (drop a pass-through
  wrapper); the no-grant test now goes through the registered IPC handler.
- Correct two comments that described the file grant as read-only, update a stale
  scroll comment, merge a doubled doc comment, drop an alias, share the
  desktop-API check, and make an internal constant module-private.
- Scope a Host visit to its host as well as its workspace, so a workspace
  repointed to another SSH target never shows or opens the old host's listing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes (delta since the prior pullfrog review at 27454a3)

  • Host mode trimmed to local + SSH: dropped paired-server (runtime) browsing — it could not open files outside the worktree and had no realpath, and was already disabled over SSH. getHostBrowseSource now returns HostBrowseSource | null (null for runtime/unresolved/non-desktop clients), planHostFileOpen lost its source argument and unsupported variant, and one generic fileExplorer.host.unavailable copy replaced the three reason strings across all six locales.
  • Fresh session per visit: useFileExplorerHostBrowser is now keyed by a visitKey (an entry counter) instead of source + worktree, and the retain-and-revalidate machinery plus browser.reset() are gone. useFileExplorerHostMode holds visit = { worktreeId, entry }; leaving the workspace ends Host mode and each entry starts a fresh browse at the workspace root.
  • End on host loss: 95990d2 restores !source to the render-phase visit clear, so a briefly unresolved owner (reconnect/hydration) ends Host mode instead of resuming a stale folder when the host returns. A test locks both the end and the non-resume.
  • Tests: e72c331 locks fresh-per-entry sessions and the filter reset on leave; 9d48c37 types the recorded visit keys without an assertion.

I ran the four touched host-mode suites (use-file-explorer-host-mode.test.tsx, file-explorer-host-mode.test.ts, use-file-explorer-host-browser.test.tsx, FileExplorerHostList.test.tsx) — 33/33 pass. The three prior Pullfrog threads are addressed and resolved; the reactivation thread is superseded by 62c980d (author replied there).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 27f8d2d0-d88c-4cec-9b46-5b6dc5aebe9e

📥 Commits

Reviewing files that changed from the base of the PR and between 27454a3 and 95990d2.

📒 Files selected for processing (15)
  • src/renderer/src/components/right-sidebar/FileExplorerHostList.test.tsx
  • src/renderer/src/components/right-sidebar/file-explorer-host-mode.test.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-mode.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-open.test.ts
  • src/renderer/src/components/right-sidebar/file-explorer-host-open.ts
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.test.tsx
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.ts
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-mode.test.tsx
  • src/renderer/src/components/right-sidebar/use-file-explorer-host-mode.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/zh.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

- Toolbar takes a single Host availability flag instead of a mirrored state object.
- Drop redundant filter reset on entry and the duplicate active-visit comparison.
- Merge overlapping Host tests, keeping every assertion (with messages).
- Cover a slow click resolving after the Host visit ends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes (delta since the prior pullfrog review at 95990d2)

  • Host mode ends when the workspace's host changes: useFileExplorerHostMode records a hostKey (local or ssh:<connectionId>) per visit and clears the visit during render when it no longer matches the current source, so repointing a workspace at another host never leaves a listing from the old machine clickable. active is now simply visit !== null; the render-phase clear owns the mismatch check, and a non-null hostKey already implies a non-null source. A new test locks both the end and the non-resume when the host returns.
  • Browse handler simplified: the exported browseHostDirectory wrapper is gone; registerFilesystemHostBrowseHandlers calls browseServerDirectory directly. Its test now drives the real fs:browseHostDir IPC handler through a mocked ipcMain, and additionally asserts a null-byte SSH path never reaches the relay.
  • "Unavailable" reshaped: FileExplorerToolbar's unavailableLabel: string | null became available: boolean with the label chosen inside the toolbar; useFileExplorerHostMode exposes available instead of a pre-built toolbar object.
  • Redundant filter reset dropped: enter no longer clears the Names filter — exit and the render-phase clear already do, and the filter can only be non-empty while Host mode is active.
  • Test consolidation: the read-only-tab and session-restore suites merge overlapping cases behind an openWritable helper, and the host-browser slow-click case becomes one table-driven test covering both "navigated away" and "visit ended". Coverage is not weakened — the assertions are exact, not truthiness checks.

I re-read the authoritative full diff and the incremental range-diff, traced the hostKey/render-clear interaction (a stale visit cannot paint because the same-component setState-during-render is re-rendered before commit), and confirmed no dangling references to the removed browseHostDirectory or the old unavailableLabel/toolbar shapes. Ran the six touched suites (use-file-explorer-host-mode, use-file-explorer-host-browser, FileExplorer.host-mode, filesystem-host-browse-handlers, editor-read-only-tabs, editor-session-host-browse-restore): 44/44 pass. Security posture is unchanged from prior reviews — browsing never widens grants, and each outside click grants at most the one regular file.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Resolve FileDocument rename conflict and replace the removed
useDelayedStatus hook with a local loading delay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes (delta since the prior pullfrog review at 976e0e2)

  • Merged origin/main and adapted to the removed status hook: the branch's use-file-explorer-host-browser no longer imports useDelayedStatus/@/lib/delayed-status (main deleted both in 5c62cb5320); the show delay is now an inline useState+useEffect timer keyed on [loading, sessionKey].
  • Adopted main's MarkdownDocument → FileDocument rename in src/shared/filesystem-entry-types.ts (with a MarkdownDocument = FileDocument compat alias); no branch-authored change beyond the merge resolution.
  • No other branch-authored delta: the range-diff 976e0e2..0230ee9 is exactly these two hunks, and no useDelayedStatus / @/lib/delayed-status references remain in src. Ran use-file-explorer-host-browser.test.tsx — 12/12 pass, including the fake-timer spinner case.
Technical details
# Inline spinner timer drops the minimum-visible hold

## Affected sites
- `src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.ts:135` — `showLoading = loading && delayedLoadingKey === sessionKey`; the timer at :127-134 fills `delayedLoadingKey` after `HOST_LOADING_SHOW_DELAY_MS` (200ms) and clears it whenever `loading`/`sessionKey` change.
- Removed `src/renderer/src/hooks/use-delayed-status.ts` / `src/renderer/src/lib/delayed-status.ts` — the old hook also enforced `STATUS_MIN_VISIBLE_MS = 400`, holding a shown status up so it could not flash right at the show delay.

## Required outcome
- Decide whether the Host spinner should keep the 400ms minimum-visible hold: if yes, the inline timer must not hide before that floor; if the immediate hide is intended, no change is needed.

## Suggested approach (optional)
- Track a `shownAt` timestamp alongside `delayedLoadingKey` and gate the reset on `Date.now() - shownAt >= 400`, mirroring the old `show()`/`minVisibleMs` behavior, or keep a trimmed `createDelayedStatus` helper for this consumer.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread src/renderer/src/components/right-sidebar/use-file-explorer-host-browser.ts Outdated
Restores the 400ms minimum-visible hold the removed useDelayedStatus
hook provided, so a listing that lands just after the show delay no
longer flashes the spinner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes (delta since the prior pullfrog review at 0230ee9)

  • Host-mode spinner regained its minimum-visible hold: use-file-explorer-host-browser.ts now tracks the shown key plus a shown-at timestamp; after the 200ms show delay a spinner stays on screen for at least 400ms, so an SSH listing that lands just after the delay no longer flashes. A new fake-timer test locks the hold and fails without it — this closes the prior review's inline finding.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — minor suggestions below.

Reviewed changes (delta since the prior pullfrog review at b2bfb20; the net branch-authored change is the merge's adaptation to upstream 51fe6f3fba / #24489)

  • Dropped the authorizeExternalPath grant from Host entry resolution: main deleted that API, so resolveHostBrowseEntry no longer calls it and fs:resolveHostBrowseEntry now only classifies (realpath + stat).
  • External local Host files moved onto main's user-named access: openHostFile still stores relativePath === filePath, so editorTabFileAccess resolves {kind:'user-file'} (read in place, no root, nothing remembered) while the tab stays readOnly + hostBrowse; SSH external files keep externalSshTargetId.
  • Tests retargeted: the handler test now asserts the clicked file, its parent, and nested children are never allowlisted and resolveAuthorizedPath rejects; the host-mode test dropped the authorizeExternalPath mock.
  • Docs retargeted: HostBrowseEntryResolution and the preload resolveHostBrowseEntry signature now say "classification only; no grant".
  • Ratchet updated: file-explorer-host-open.ts added to USER_NAMED_TAB_OPENERS (correct, since it can open a tab read as user-named).
  • Adopted main's csvPreviewOnly field on OpenFile (CSV editor, not branch behavior).

The read path holds: useEditorPanelFileContentLoader passes editorTabFileAccess(...) whenever the tab is present in openFiles, so a freshly-opened external Host file reads as user-named even with no grant; readOnly keeps it non-editable and editorTabDocumentFolderAccess still returns undefined for read-only tabs. Net posture is narrower than the old single-file subtree grant. Ran the touched suites — 41/41 and 34/34 pass across the handler, host-open, local-file-access, read-only-tabs, host-mode, host-browser, FileExplorer overlay, session-restore, and drafts tests.

ℹ️ Nitpicks

  • src/renderer/src/store/slices/editor/types/open-file.ts:149 — the hostBrowse comment still explains non-persistence as restore cannot replay its path grant, but external Host files no longer receive any path grant (they read through user-named access). Reword to the read-only session semantics.
  • PR description — the Mechanism / Trust model / Worst case sections still describe the removed grant ("gets the same session read/write path grant that terminal links and View Log already use"); worth updating to the classification-only model.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Chihen-Tai and others added 2 commits October 8, 2026 22:01
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

2 participants