Fix file tree not loading when opened immediately after cloning a repo#9857
Fix file tree not loading when opened immediately after cloning a repo#9857Chukwuebuka-2003 wants to merge 9 commits intowarpdotdev:masterfrom
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Chukwuebuka-2003 on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR tries to avoid the file tree's standalone lazy-loading path when the displayed directory is a freshly cloned git repository, and instead kicks off the existing repository detection/indexing flow.
Concerns
- The new
.gitexistence check is treated as definitive, but detection can still fail for stale or invalid.gitentries. In that case the code skips lazy-loading and leaves the file tree empty with no fallback. - Security pass: no direct security issues found in the changed lines.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
Chukwuebuka-2003
force-pushed
the
fix/file-tree-load-after-clone
branch
from
f7479a4 to
531895b
Compare
done |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes the local file tree lazy-load path to trigger git repository detection when the displayed path appears to be a repository root.
Concerns
- The changed code does not compile because it calls
StandardizedPath::as_local_path(), which is not defined in the repository API.
Verdict
Found: 1 critical, 0 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // and a valid HEAD file inside it. This avoids treating stale/invalid .git folders | ||
| // as repos (which would cause detection to fail and leave the tree empty). | ||
| let is_valid_git_repo = path | ||
| .as_local_path() |
There was a problem hiding this comment.
🚨 [CRITICAL] This does not compile because StandardizedPath has to_local_path(), not as_local_path().
| .as_local_path() | |
| .to_local_path() |
When a user opens the file tree immediately after cloning a repository, there's a race condition where the terminal hasn't yet triggered git detection, so the file tree falls back to lazy-loading instead of triggering proper repo indexing. This fix checks if a path is a valid git repository by verifying both the .git directory exists AND .git/HEAD is a valid file. This avoids treating stale/invalid .git folders as repos. If valid, we trigger proper git detection instead of lazy-loading, ensuring the full file tree gets built even when opened immediately after a fresh clone. Fixes warpdotdev#9846
Chukwuebuka-2003
force-pushed
the
fix/file-tree-load-after-clone
branch
from
531895b to
25fca58
Compare
fixed |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR tries to route freshly opened git roots through repo detection instead of registering them as standalone lazy-loaded directories.
Concerns
- The new detection call drops a must-use future, which will fail the repository's warning-deny validation unless it is explicitly spawned or dropped.
- Git-looking directories no longer have a lazy-load fallback when full repo indexing fails, so a repo that exceeds the file limit or otherwise fails indexing can remain empty and repeatedly retry detection.
Verdict
Found: 1 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| }) | ||
| .unwrap_or(false); | ||
|
|
||
| if is_valid_git_repo { |
There was a problem hiding this comment.
UpdatingRepositoryFailed calls back into this method, sees the same valid .git, and retries detection instead of falling back to lazy-loading, leaving the tree empty for over-limit or failed repos.
There was a problem hiding this comment.
Overview
This PR tries to route freshly opened git roots through repo detection instead of registering them as standalone lazy-loaded directories.
Concerns
- The new detection call drops a must-use future, which will fail the repository's warning-deny validation unless it is explicitly spawned or dropped.
- Git-looking directories no longer have a lazy-load fallback when full repo indexing fails, so a repo that exceeds the file limit or otherwise fails indexing can remain empty and repeatedly retry detection.
Verdict
Found: 1 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| DetectedRepositories::handle(ctx).update(ctx, |repos, ctx| { | ||
| std::mem::drop(repos.detect_possible_git_repo( | ||
| &path.to_string(), | ||
| RepoDetectionSource::TerminalNavigation, | ||
| ctx, | ||
| )); | ||
| }); |
There was a problem hiding this comment.
🚨 [CRITICAL] This drops the Future returned by detect_possible_git_repo; futures are #[must_use], so cargo clippy -- -D warnings will reject this hunk.
| DetectedRepositories::handle(ctx).update(ctx, |repos, ctx| { | |
| std::mem::drop(repos.detect_possible_git_repo( | |
| &path.to_string(), | |
| RepoDetectionSource::TerminalNavigation, | |
| ctx, | |
| )); | |
| }); | |
| DetectedRepositories::handle(ctx).update(ctx, |repos, ctx| { | |
| std::mem::drop(repos.detect_possible_git_repo( | |
| &path.to_string(), | |
| RepoDetectionSource::TerminalNavigation, | |
| ctx, | |
| )); | |
| }); |
| }) | ||
| .unwrap_or(false); | ||
|
|
||
| if is_valid_git_repo { |
There was a problem hiding this comment.
UpdatingRepositoryFailed calls back into this method, sees the same valid .git, and retries detection instead of falling back to lazy-loading, leaving the tree empty for over-limit or failed repos.
Chukwuebuka-2003
force-pushed
the
fix/file-tree-load-after-clone
branch
from
325a15a to
9fa253d
Compare
resolved |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes the file tree lazy-load path so an active local root with a .git directory triggers repository detection instead of immediately registering a shallow lazy-loaded tree.
Concerns
- The new synchronous precheck only recognizes repositories with a
.gitdirectory, while the existing repository detector also supports.gitfiles used by worktrees and submodules. Those supported repository layouts still take the lazy-load fallback and retain the race this PR is intended to fix. - Security pass: no security issues were identified in the changed lines.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let is_valid_git_repo = path | ||
| .to_local_path() | ||
| .map(|p| { | ||
| let git_dir = p.join(".git"); | ||
| git_dir.exists() && git_dir.join("HEAD").is_file() | ||
| }) |
There was a problem hiding this comment.
.git directories, so worktrees/submodules with a .git file still take the lazy-load path and keep the same race this PR is fixing; reuse the existing git detection logic or include gitfile handling before falling back.
There was a problem hiding this comment.
Overview
This PR changes the file tree lazy-load path so an active local root with a .git directory triggers repository detection instead of immediately registering a shallow lazy-loaded tree.
Concerns
- The new synchronous precheck only recognizes repositories with a
.gitdirectory, while the existing repository detector also supports.gitfiles used by worktrees and submodules. Those supported repository layouts still take the lazy-load fallback and retain the race this PR is intended to fix. - Security pass: no security issues were identified in the changed lines.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let is_valid_git_repo = path | ||
| .to_local_path() | ||
| .map(|p| { | ||
| let git_dir = p.join(".git"); | ||
| git_dir.exists() && git_dir.join("HEAD").is_file() | ||
| }) |
There was a problem hiding this comment.
.git directories, so worktrees/submodules with a .git file still take the lazy-load path and keep the same race this PR is fixing; reuse the existing git detection logic or include gitfile handling before falling back.
Chukwuebuka-2003
force-pushed
the
fix/file-tree-load-after-clone
branch
from
9fa253d to
97ebba0
Compare
resolved |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes the local file-tree lazy-load path so opening a freshly cloned repository can trigger git repository detection instead of registering a shallow lazy-loaded tree.
Concerns
- The new
.gitprecheck does not actually support gitfile-based repositories such as worktrees and submodules, despite the comment claiming it does; those paths still fall back to lazy loading and keep the race for that repo shape.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let is_valid_git_repo = path | ||
| .to_local_path() | ||
| .map(|p| { | ||
| let git_dir = p.join(".git"); | ||
| let git_file = p.join(".git"); | ||
| // Check for .git directory OR .git file (worktree/submodule) | ||
| let is_git_entry = git_dir.is_dir() || git_file.is_file(); | ||
| // Verify valid HEAD inside .git (regardless of whether dir or file) | ||
| is_git_entry && git_dir.join("HEAD").is_file() | ||
| }) | ||
| .unwrap_or(false); |
There was a problem hiding this comment.
.git files: git_entry.join("HEAD") only works when .git is a directory, so worktrees/submodules still fall back to lazy loading instead of repo detection.
| let is_valid_git_repo = path | |
| .to_local_path() | |
| .map(|p| { | |
| let git_dir = p.join(".git"); | |
| let git_file = p.join(".git"); | |
| // Check for .git directory OR .git file (worktree/submodule) | |
| let is_git_entry = git_dir.is_dir() || git_file.is_file(); | |
| // Verify valid HEAD inside .git (regardless of whether dir or file) | |
| is_git_entry && git_dir.join("HEAD").is_file() | |
| }) | |
| .unwrap_or(false); | |
| let is_valid_git_repo = path | |
| .to_local_path() | |
| .map(|p| { | |
| let git_entry = p.join(".git"); | |
| if git_entry.is_dir() { | |
| git_entry.join("HEAD").is_file() | |
| } else if git_entry.is_file() { | |
| std::fs::read_to_string(&git_entry) | |
| .ok() | |
| .and_then(|contents| { | |
| contents | |
| .trim() | |
| .strip_prefix("gitdir:") | |
| .map(|gitdir| PathBuf::from(gitdir.trim())) | |
| }) | |
| .map(|gitdir| { | |
| let gitdir = if gitdir.is_absolute() { | |
| gitdir | |
| } else { | |
| p.join(gitdir) | |
| }; | |
| gitdir.join("HEAD").is_file() | |
| }) | |
| .unwrap_or(false) | |
| } else { | |
| false | |
| } | |
| }) | |
| .unwrap_or(false); |
When a user opens the file tree immediately after cloning a repository, there's a race condition where the terminal hasn't yet triggered git detection, so the file tree falls back to lazy-loading instead of triggering proper repo indexing. This fix checks if a path has a .git entry (directory or file for worktrees/submodules) and triggers detection. The async detection handles validation properly, including gitfile parsing for worktrees. If the repo has previously failed to index (e.g., exceeded file limit), we fallback to lazy-loading to avoid repeatedly retrying failed detection. Fixes warpdotdev#9846
Chukwuebuka-2003
force-pushed
the
fix/file-tree-load-after-clone
branch
from
97ebba0 to
8c6b8b6
Compare
review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
oz-for-oss
Bot
dismissed stale reviews from themself
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR changes the file tree lazy-load path to trigger existing git repository detection when the displayed directory has a valid .git entry, and adds an integration regression test for opening the file tree immediately after cloning.
Concerns
- The new regression test currently only asserts first-level entries that would also be present in the old lazy-loaded tree, so it does not prove the full repo indexing path was used.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| new_step_with_default_assertions("Open file tree panel") | ||
| .with_action(|app, _, _| open_file_tree_panel(app)), | ||
| ) | ||
| // Verify the file tree loaded the repo - if it stayed in lazy-load mode, |
There was a problem hiding this comment.
💡 [SUGGESTION] This test still passes if the implementation falls back to lazy-loading, because README.md and src are both first-level entries; expand src and assert src/main.rs is visible, or assert the root is not lazy-loaded, so the regression is actually covered.
Expands src/ and verifies main.rs is visible. First-level entries like README.md and src appear in both lazy-loaded and fully-indexed trees, so they don't prove the fix works. Only nested content like src/main.rs proves proper repo indexing occurred.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes the Project Explorer lazy-load path to detect immediate .git entries and trigger repo metadata indexing, and adds an integration test registration.
Concerns
- The git-detection branch can leave the file tree empty when detection returns
Nonebefore the repo metadata model emits a failure, because it skips lazy-loading and drops the returned future. - The new integration test does not distinguish full repo indexing from the existing lazy-loaded directory expansion path.
- Security review found no changed-line security findings.
Verdict
Found: 0 critical, 1 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // Trigger git repo detection - this will properly index the repo and emit | ||
| // events that update the file tree once complete. | ||
| DetectedRepositories::handle(ctx).update(ctx, |repos, ctx| { | ||
| std::mem::drop(repos.detect_possible_git_repo( |
There was a problem hiding this comment.
detect_possible_git_repo returns None without emitting DetectedGitRepo (for example find_git_repo stops at $HOME before checking ~/.git, or watcher registration fails), this branch has already skipped index_lazy_loaded_path, so the tree remains empty and retries on every refresh; await the returned future or trigger lazy-loading on None.
| new_step_with_default_assertions("Open file tree panel") | ||
| .with_action(|app, _, _| open_file_tree_panel(app)), | ||
| ) | ||
| // Verify the file tree loaded the full repo, not just lazy-loaded first-level entries. |
There was a problem hiding this comment.
💡 [SUGGESTION] This assertion can pass with the old lazy-loading path because expanding src calls load_directory for lazy-loaded roots too, so it does not prove repo indexing ran; assert the path is not lazy-loaded / is indexed, or verify nested content before manually expanding.
This ensures the file tree is never empty if detection returns None or fails. Previously we only did lazy-loading for non-git dirs, but now we always register lazy-loaded path first, then trigger detection on top - detection will replace with full content if successful.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @moirahuang. Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR updates the file tree view to kick off git repository detection when a newly displayed local root already contains a valid .git entry, so the lazy-loaded placeholder can be upgraded to a full repo-backed tree. It also registers an integration regression test for the first-open path.
Concerns
- The new
.gitfile precheck reads the entire gitfile synchronously on the view path; cap this or defer parsing to async detection to avoid UI stalls on malformed local folders. - The integration test can pass via existing lazy directory expansion and after terminal bootstrap has already had a chance to detect git, so it does not tightly prove the regression path.
Security
- A malicious or malformed local folder can place a very large
.gitfile at the displayed root, causing an unbounded synchronous read when the file tree opens.
Verdict
Found: 0 critical, 0 important, 2 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| git_entry.join("HEAD").is_file() | ||
| } else if git_entry.is_file() { | ||
| // .git file (worktree/submodule) - read gitdir path and verify HEAD there | ||
| std::fs::read_to_string(&git_entry) |
There was a problem hiding this comment.
💡 [SUGGESTION] [SECURITY] This synchronously reads the entire .git file on the view path; cap it to the small gitfile format or let detect_possible_git_repo parse it asynchronously so a malformed local folder cannot stall opening the file tree.
| new_step_with_default_assertions("Open file tree panel") | ||
| .with_action(|app, _, _| open_file_tree_panel(app)), | ||
| ) | ||
| // Verify the file tree loaded the full repo, not just lazy-loaded first-level entries. |
There was a problem hiding this comment.
💡 [SUGGESTION] This can pass through the old lazy-loading path because expanding src calls load_directory and reveals main.rs; assert the repo is no longer lazy-loaded or trigger the view before terminal bootstrap detection to cover the intended race.
Security: limit synchronous .git file reads to prevent malformed local folders from stalling the file tree open. gitfile format is small (~100B), so 1KB is plenty.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR tries to fix the file tree race after cloning by detecting local .git entries from register_and_refresh_lazy_loaded_directory and then kicking off normal repository detection, plus adding an integration regression test.
Concerns
- The new
.gitfile parsing path does not compile because it treats theVec<u8>returned bystd::fs::readlike a string and then tries to convert a borrowed/truncated slice intoString. - The
.gitfile read is still unbounded before the 1KB cap is applied, so malformed local folders can force unnecessary memory allocation and UI-thread latency. - The added integration test does not reliably distinguish the intended full-repo indexing path from the old lazy-load path.
Security
- The
.gitfile parsing should bound reads before allocation or move the detection to existing async git detection logic.
Verdict
Found: 1 critical, 1 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| std::fs::read(&git_entry) | ||
| .ok() | ||
| .and_then(|contents| { | ||
| let contents = contents.trim_end(); // trim trailing nulls |
There was a problem hiding this comment.
🚨 [CRITICAL] std::fs::read returns Vec<u8>, so trim_end() and the later TryInto<String> conversion do not compile; decode with String::from_utf8 or read as a bounded string first.
| // .git file (worktree/submodule) - read gitdir path and verify HEAD there | ||
| // Cap read to 1KB - gitfile format is small (~100 bytes), so this prevents | ||
| // malicious or malformed folders from stalling the UI. | ||
| std::fs::read(&git_entry) |
There was a problem hiding this comment.
.git file on the UI path before applying the 1KB cap; use a bounded reader such as File::open(...).take(1024) or delegate to the async detector so malformed folders cannot cause unbounded allocation or latency.
| new_step_with_default_assertions("Open file tree panel") | ||
| .with_action(|app, _, _| open_file_tree_panel(app)), | ||
| ) | ||
| // With the fix, detection triggers immediately on .git dirs, so the repo |
There was a problem hiding this comment.
💡 [SUGGESTION] This test waits for terminal bootstrap and then expands src, so the old lazy-loading path can still reveal main.rs; assert the repo is not lazy-loaded or use a lower-level test that opens the file tree before terminal git detection runs.
Use std::io::Read with a bounded buffer instead of unbounded std::fs::read. This prevents malicious/malformed .git files from causing large memory allocations or UI stalls on the view path.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @moirahuang. Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR makes the file tree trigger repo detection when an active local root contains a valid .git entry, while keeping the existing lazy-load fallback so the tree remains visible until full indexing completes. It also registers an integration test for the fresh-clone opening path.
Concerns
- The added integration test does not currently distinguish full repo indexing from the existing lazy-load-on-expand path, so it may not fail against the old behavior.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // which only works if full indexing happened (lazy-loading loads shallow tree). | ||
| // Note: This tests the detection path; old lazy-loading would need expand. | ||
| .with_step( | ||
| new_step_with_default_assertions("Expand src to verify nested content loads") |
There was a problem hiding this comment.
💡 [SUGGESTION] This still passes with the old lazy-load behavior because expanding src calls load_directory; assert the model is no longer lazy-loaded or that src/main.rs is already present before expansion to make this a true regression test.
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR teaches the file tree to detect a local .git entry when a standalone directory is lazy-loaded, then triggers repository detection so full indexing can replace the shallow tree. It also registers an integration test for opening the file tree immediately after creating a git repo.
Concerns
- If full repository indexing fails after detection starts, the claimed lazy fallback can be lost because the model-side lazy state is removed while the view still believes the path is registered.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| // Additionally, if this appears to be a git repo and hasn't failed before, | ||
| // trigger proper detection to get full repo indexing instead of shallow tree. | ||
| if has_git_entry && !previously_failed { |
There was a problem hiding this comment.
index_directory removes the model-side lazy-loaded entry and marks the repo Failed while registered_lazy_loaded_paths still contains path; the next refresh skips index_lazy_loaded_path and replaces the visible fallback with an empty root. Preserve the lazy entry until full indexing succeeds, or clear/retry the lazy registration on UpdatingRepositoryFailed.
|
Hi @Chukwuebuka-2003 I appreciate you opening a PR! Unfortunately, I think this isn't quite the right solution. Because we already have codebase indexing logic, I think we shouldn't be forcibly reading the |
2 participants
When a user opens the file tree immediately after cloning a repository, there's a race condition where the terminal hasn't yet triggered git detection, so the file tree falls back to lazy-loading instead of triggering proper repo indexing.
This fix checks if a path contains a .git directory and triggers proper git detection instead of lazy-loading, ensuring the full file tree gets built even when opened immediately after a fresh clone.
Fixes #9846
Description
When a freshly cloned repo is opened in Warp and the user immediately opens the file tree sidebar, the file tree fails to load because:
This fix adds a check in register_and_refresh_lazy_loaded_directory to detect if a path contains a .git directory. If it does, we trigger proper git detection via DetectedRepositories::detect_possible_git_repo instead of lazy-loading, ensuring the full file tree gets built.
Linked Issue
Screenshots / Videos
Not applicable - this is a bug fix in non-UI code
Testing
The existing file tree tests should cover this functionality. The fix uses existing infrastructure (DetectedRepositories::detect_possible_git_repo) that is already well-tested.
Agent Mode
CHANGELOG-BUG-FIX: Fixed file tree sidebar not loading when opened immediately after cloning a repository