feat(#106): Add directory UI + filesystem browse endpoint#119
Merged
Abernaughty merged 4 commits intomainfrom Apr 4, 2026
Merged
feat(#106): Add directory UI + filesystem browse endpoint#119Abernaughty merged 4 commits intomainfrom
Abernaughty merged 4 commits intomainfrom
Conversation
Phase A of #106 — Workspace selector "Add directory" with text input and server-side directory browser. Backend: - GET /filesystem/browse endpoint — lists subdirectories at a path with project detection (package.json, pyproject.toml, .git, etc.) and has_children metadata for lazy tree expansion - BrowseDirectoryEntry + BrowseDirectoryResponse Pydantic models - Defaults to user home dir, filters hidden dirs, sorts projects first Dashboard: - WorkspaceSelector "Add directory" button in dropdown - Text input mode: paste/type path, Enter to confirm, Escape to cancel - Browse mode: breadcrumb navigation + directory listing with project badges, parent navigation, "Select this directory" confirmation - Client-side duplicate check before API call - Outside-click dismissal resets add-directory state - Auto-focus on input appearance Proxy: - /api/filesystem/browse SvelteKit server route Types: - BrowseDirectoryEntry + BrowseDirectoryResponse TS interfaces
Closed
29 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an "Add directory" flow to the workspace selector with inline input, server-backed directory browsing and breadcrumb navigation, keyboard/focus handlers, outside-click dismissal, SvelteKit proxy route, new TypeScript/Pydantic types, and a FastAPI endpoint that enumerates child directories and detects projects. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Dashboard UI
participant Proxy as SvelteKit Proxy
participant Backend as FastAPI
participant FS as File System
User->>UI: Open workspace dropdown → choose "Add directory"
UI->>UI: Show input & browse UI, focus input
User->>UI: Click "Browse" or enter a path
UI->>Proxy: GET /api/filesystem/browse?path=...&show_hidden=...
Proxy->>Backend: Proxy GET /filesystem/browse?path=...&show_hidden=...
Backend->>FS: Resolve path, list immediate child directories
Backend->>FS: Inspect children for project markers and subdirs
FS-->>Backend: Return entries & metadata
Backend-->>Proxy: BrowseDirectoryResponse (entries, parent_path, current_path)
Proxy-->>UI: Return entries → render breadcrumb + list
User->>UI: Select entry or press Enter to confirm
UI->>workspacesStore: addDirectory(newDirPath)
workspacesStore-->>UI: success → close dropdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/src/lib/components/WorkspaceSelector.svelte`:
- Around line 167-168: The catch block currently assigns the raw transport error
message to browseError, which leads the template to render backend errors
verbatim; instead change the catch handling in the try/catch around the browse
fetch (the code assigning browseError in the catch) to set a neutral unavailable
state (e.g., set browseError to null and/or set a boolean like browseUnavailable
= true) and ensure the template uses this flag to show the neutral “browser
unavailable / type a path instead” empty-state UI and the existing manual path
fallback rather than displaying the raw error; update any other catch sites that
assign browseError (notably the occurrences around lines 302-304) to follow the
same pattern.
- Around line 145-150: Reset any stale browse selection when starting or failing
a browse: in openBrowseMode() and where fetchBrowse('')/fetchBrowse calls begin,
set browsePath = null (or '') along with browseError = null and addDirError =
null so previous successful selection is cleared; in the fetchBrowse error/catch
handling ensure browsePath is also cleared. Also gate the "Select this
directory" action (the handler that reads browsePath to add a directory) to only
proceed when browseError is falsy and browseLoading is false (and browsePath is
non-empty), so it cannot add the previous folder after a 403/network failure.
Update references to browsePath, browseError, browseLoading, openBrowseMode, and
fetchBrowse accordingly.
In `@dev-suite/src/api/main.py`:
- Around line 421-440: The per-child probe currently catches PermissionError but
still appends the directory, causing inaccessible folders to be shown and
leaving FileNotFoundError/NotADirectoryError uncaught; update the try/except
around the child inspection (the block that computes child_contents, is_project
and has_children for each child) to catch PermissionError, FileNotFoundError and
NotADirectoryError and on those exceptions skip the child (use continue) instead
of setting is_project/has_children and appending a BrowseDirectoryEntry for that
child, so only fully inspected entries are added to entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61bf7dd4-1051-4ec9-99ba-2e4182dd49af
📒 Files selected for processing (5)
dashboard/src/lib/components/WorkspaceSelector.sveltedashboard/src/lib/types/api.tsdashboard/src/routes/api/filesystem/browse/+server.tsdev-suite/src/api/main.pydev-suite/src/api/models.py
…ble, skip unreadable dirs Fixes all 3 CodeRabbit Major/Minor items from PR #119 review: 1. Clear stale browse selection on each fetchBrowse() call — prevents "Select this directory" from adding the previous folder after a 403/network failure. Also gate select button on browseError/Loading. 2. Replace raw transport error with neutral browseUnavailable state — shows "Browser unavailable — use Type path below" instead of verbatim fetch errors. Graceful degradation per dashboard guidelines. 3. Skip unreadable child dirs instead of listing them with unknown metadata. Catch FileNotFoundError/NotADirectoryError alongside PermissionError for race conditions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
dev-suite/src/api/main.py (2)
421-432: Minor cleanup: redundantis_dir()check and double iteration.Line 423's
if child.is_dir()guard is always true (already filtered at line 417). Thechild.iterdir()is called twice—once forchild_contentsand once forhas_children. Consider consolidating:♻️ Proposed refactor to remove redundancy
try: - # Detect project markers - child_contents = set(c.name for c in child.iterdir()) if child.is_dir() else set() - is_project = bool(child_contents & _PROJECT_MARKERS) - has_children = any( - c.is_dir() and (show_hidden or not c.name.startswith(".")) - for c in child.iterdir() - if c.is_dir() - ) + # Single pass: detect project markers and visible subdirectories + child_names: set[str] = set() + has_children = False + for c in child.iterdir(): + child_names.add(c.name) + if not has_children and c.is_dir() and (show_hidden or not c.name.startswith(".")): + has_children = True + is_project = bool(child_names & _PROJECT_MARKERS) except (PermissionError, FileNotFoundError, NotADirectoryError):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-suite/src/api/main.py` around lines 421 - 432, The block calculating child_contents and has_children redundantly calls child.iterdir() twice and checks child.is_dir() unnecessarily; refactor inside the loop that processes directory entries so you iterate child.iterdir() once, build child_contents (set of entry names) and compute has_children (any entry.is_dir() and (show_hidden or not entry.name.startswith("."))) in that single pass, then set is_project = bool(child_contents & _PROJECT_MARKERS); update references to use these variables and remove the redundant child.is_dir() guard.
391-396: Consider path restrictions for production deployments.The endpoint allows browsing any readable directory on the server filesystem. While
require_authprovides access control, in production environments you may want to restrict browsing to a configurable allowlist of root directories (e.g., user home,/projects) to limit exposure if the API secret is compromised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-suite/src/api/main.py` around lines 391 - 396, The browse_filesystem endpoint (function browse_filesystem) currently lets authenticated users list any readable server path; to harden production, add a configurable allowlist (e.g., ALLOWED_BROWSE_PATHS or app config value) and validate the incoming path against it before performing filesystem operations: normalize/resolve the requested path (use pathlib.Path.resolve()) and ensure it is contained within one of the allowed root directories, returning a 403/ApiResponse error if not allowed; keep require_auth as-is but enforce this allowlist check early in browse_filesystem and make the allowlist loadable from environment/config for deployment-specific roots.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev-suite/src/api/main.py`:
- Around line 440-441: The outer exception handler that currently does "except
PermissionError: _error(f\"Permission denied: {target}\", 403)" must also handle
TOCTOU outcomes where the target is removed or replaced between is_dir() and
iterdir(); update that handler to also catch FileNotFoundError and
NotADirectoryError (or add separate except clauses) and call _error with an
appropriate status (e.g., 404 for FileNotFoundError/NotADirectoryError) so the
function handling directory iteration (referencing target and the iterdir()
call) returns a proper client error instead of a 500.
---
Nitpick comments:
In `@dev-suite/src/api/main.py`:
- Around line 421-432: The block calculating child_contents and has_children
redundantly calls child.iterdir() twice and checks child.is_dir() unnecessarily;
refactor inside the loop that processes directory entries so you iterate
child.iterdir() once, build child_contents (set of entry names) and compute
has_children (any entry.is_dir() and (show_hidden or not
entry.name.startswith("."))) in that single pass, then set is_project =
bool(child_contents & _PROJECT_MARKERS); update references to use these
variables and remove the redundant child.is_dir() guard.
- Around line 391-396: The browse_filesystem endpoint (function
browse_filesystem) currently lets authenticated users list any readable server
path; to harden production, add a configurable allowlist (e.g.,
ALLOWED_BROWSE_PATHS or app config value) and validate the incoming path against
it before performing filesystem operations: normalize/resolve the requested path
(use pathlib.Path.resolve()) and ensure it is contained within one of the
allowed root directories, returning a 403/ApiResponse error if not allowed; keep
require_auth as-is but enforce this allowlist check early in browse_filesystem
and make the allowlist loadable from environment/config for deployment-specific
roots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18e8404f-41bc-45f3-9d79-9a8825c28a42
📒 Files selected for processing (2)
dashboard/src/lib/components/WorkspaceSelector.sveltedev-suite/src/api/main.py
🚧 Files skipped from review as they are similar to previous changes (1)
- dashboard/src/lib/components/WorkspaceSelector.svelte
- Outer exception: catch FileNotFoundError/NotADirectoryError alongside PermissionError to handle TOCTOU race where target dir is removed between is_dir() check and iterdir() call (returns 404 not 500) - Single-pass iterdir: consolidate double child.iterdir() into one loop that collects child names and checks for visible subdirectories - Fix import sort order (I001) from main merge
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dev-suite/src/api/main.py (1)
24-57:⚠️ Potential issue | 🟡 MinorFix import sorting to pass CI.
The pipeline failed with ruff I001: "Import block is un-sorted or un-formatted." Run
ruff check --fixor manually reorder the imports according to the project's ruff configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-suite/src/api/main.py` around lines 24 - 57, The import block is unsorted per ruff; reorder and/or auto-fix the imports so they follow the project's ruff import ordering (standard library first: __future__, asyncio, logging, os, pathlib.Path, contextlib.asynccontextmanager; third-party next: dotenv.load_dotenv, fastapi.Depends/FastAPI/HTTPException/Query/Request, fastapi.middleware.cors.CORSMiddleware, sse_starlette.EventSourceResponse; local package imports last: .auth.require_auth, .events.SSEEvent/event_bus, and all symbols from .models). You can run `ruff check --fix` or manually reorder the import lines to match that grouping and alphabetical order within groups.
♻️ Duplicate comments (1)
dev-suite/src/api/main.py (1)
440-441:⚠️ Potential issue | 🟠 MajorHandle TOCTOU race in outer exception block.
The target directory could be deleted or replaced between the
is_dir()check (line 407) anditerdir()(line 416). Currently onlyPermissionErroris caught, soFileNotFoundErrororNotADirectoryErrorwould propagate as a 500.🛡️ Proposed fix
except PermissionError: _error(f"Permission denied: {target}", 403) + except (FileNotFoundError, NotADirectoryError): + _error(f"Not a directory: {target}", 400)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-suite/src/api/main.py` around lines 440 - 441, The except block that currently only catches PermissionError should also handle TOCTOU-related errors raised between the is_dir() check and the iterdir() call: catch FileNotFoundError and NotADirectoryError in the same error-handling section (or expand the try to include both is_dir() and iterdir()) and call _error(...) with appropriate status codes (e.g., FileNotFoundError -> _error(f"Not found: {target}", 404); NotADirectoryError -> _error(f"Not a directory: {target}", 400)) so these race conditions no longer raise a 500; adjust the except clause around the code that uses is_dir() and iterdir() accordingly.
🧹 Nitpick comments (1)
dev-suite/src/api/main.py (1)
423-428: Remove redundantis_dir()checks.Line 423:
child.is_dir()is always true here since line 417 already filtered non-directories.Lines 425-428: The
if c.is_dir()filter is redundant withc.is_dir() and ...in the condition.♻️ Suggested simplification
try: # Detect project markers - child_contents = set(c.name for c in child.iterdir()) if child.is_dir() else set() + child_contents = set(c.name for c in child.iterdir()) is_project = bool(child_contents & _PROJECT_MARKERS) has_children = any( c.is_dir() and (show_hidden or not c.name.startswith(".")) for c in child.iterdir() - if c.is_dir() )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-suite/src/api/main.py` around lines 423 - 428, The code performs redundant is_dir() checks for "child" and within the has_children generator: remove the unnecessary child.is_dir() in the child_contents computation (since the outer loop already ensures directories) and simplify the has_children generator by dropping the if c.is_dir() filter while keeping the c.is_dir() check in the condition or vice versa—i.e., choose one place to test directoryness; update the expressions computing child_contents, is_project, and has_children (references: child, child_contents, is_project, has_children, _PROJECT_MARKERS) so each directory is only checked once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@dev-suite/src/api/main.py`:
- Around line 24-57: The import block is unsorted per ruff; reorder and/or
auto-fix the imports so they follow the project's ruff import ordering (standard
library first: __future__, asyncio, logging, os, pathlib.Path,
contextlib.asynccontextmanager; third-party next: dotenv.load_dotenv,
fastapi.Depends/FastAPI/HTTPException/Query/Request,
fastapi.middleware.cors.CORSMiddleware, sse_starlette.EventSourceResponse; local
package imports last: .auth.require_auth, .events.SSEEvent/event_bus, and all
symbols from .models). You can run `ruff check --fix` or manually reorder the
import lines to match that grouping and alphabetical order within groups.
---
Duplicate comments:
In `@dev-suite/src/api/main.py`:
- Around line 440-441: The except block that currently only catches
PermissionError should also handle TOCTOU-related errors raised between the
is_dir() check and the iterdir() call: catch FileNotFoundError and
NotADirectoryError in the same error-handling section (or expand the try to
include both is_dir() and iterdir()) and call _error(...) with appropriate
status codes (e.g., FileNotFoundError -> _error(f"Not found: {target}", 404);
NotADirectoryError -> _error(f"Not a directory: {target}", 400)) so these race
conditions no longer raise a 500; adjust the except clause around the code that
uses is_dir() and iterdir() accordingly.
---
Nitpick comments:
In `@dev-suite/src/api/main.py`:
- Around line 423-428: The code performs redundant is_dir() checks for "child"
and within the has_children generator: remove the unnecessary child.is_dir() in
the child_contents computation (since the outer loop already ensures
directories) and simplify the has_children generator by dropping the if
c.is_dir() filter while keeping the c.is_dir() check in the condition or vice
versa—i.e., choose one place to test directoryness; update the expressions
computing child_contents, is_project, and has_children (references: child,
child_contents, is_project, has_children, _PROJECT_MARKERS) so each directory is
only checked once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2921528a-9ada-43ba-baae-8b5dc6de32a7
📒 Files selected for processing (2)
dev-suite/src/api/main.pydev-suite/src/api/models.py
✅ Files skipped from review due to trivial changes (1)
- dev-suite/src/api/models.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase A of #106 — adds "Add directory" UI to the workspace selector with both a text input and a server-side directory browser.
Closes the Phase A checklist items from #106:
Deferred:
Changes
Backend (
dev-suite/src/api/)GET /filesystem/browse— new endpoint that lists subdirectories at a given path:?path=/some/dir(defaults to user home) and?show_hidden=false{ current_path, parent_path, entries[] }where each entry hasname,path,has_children,is_project.git,package.json,pyproject.toml,Cargo.toml,go.mod,pom.xml,build.gradle,MakefileNew models:
BrowseDirectoryEntry,BrowseDirectoryResponseDashboard (
dashboard/)WorkspaceSelector.svelte — enhanced with:
+styled, appears at bottom of dropdown after dividerNew proxy route:
/api/filesystem/browse/+server.tsNew types:
BrowseDirectoryEntry,BrowseDirectoryResponseinapi.tsTesting
Manual testing required:
Screenshots
UI screenshots available after local testing
Issue
Partial fix for #106 (Phase A only — Phase B = Planner agent)
Summary by CodeRabbit