You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
AI code review — automated review for reference; please use your judgment.
Nice extraction — the shared hooks remove real duplication between layouts and the enabled() gating keeps legacy/new handling mutually exclusive. A few things to tighten:
packages/app/src/pages/layout/open-project.ts:20-22 — The registration call swallows all errors (.catch(() => undefined)) and then unconditionally proceeds to open() and navigate. Why it matters: a failed project.open (bad path, permission error) leaves the user staring at a project view backed by nothing, with zero feedback — worse than the old fire-and-forget because the await signals intent to care about the result. Suggestion: return/propagate failure, skip navigation, and surface a toast.
packages/app/src/pages/layout/use-deep-links.ts:24-38 — Neither .then chain has a .catch. If openProject rejects (e.g. options.open throws synchronously inside ensureProject), navigation is silently dropped and the rejection becomes unhandledrejection noise. Suggestion: add .catch with at least a log, or make handleDeepLinks async with try/catch.
packages/app/src/pages/layout/use-deep-links.ts:23-39 — Multiple links in one event now complete in arbitrary async order, so the final route depends on resolution order (a slow open-project link can land after a new-session link and override its navigation). Pre-existing ambiguity made more likely by deferring all navigation behind promises. Suggestion: process links sequentially (await in a loop) or document last-resolved-wins.
packages/app/src/pages/layout/open-project.ts:17 — The known-check maps directories through options.projectDirectory in the legacy wiring but not in layout-new.tsx, which passes the raw directory. If the new layout's project list keys differ from raw worktree paths (the reason legacy needs projectRoot), known will always be false there and every open pays an extra API round-trip. Suggestion: confirm both lists share the same keying, or apply one canonical mapping inside the hook.
packages/app/src/pages/layout/open-project.ts:16-25 — Two overlapping triggers (deep link plus manual open) for the same unknown directory both run the registration call. Harmless today, cheap to fix: memoize the in-flight promise per directory.
Nit: use-deep-links returns { handleDeepLinks } but neither call site uses it; either drop it from the API or keep it deliberately for tests.
Closing this PR and using the clean replacement here instead: #44137. @Hona@Brendonovich probably worth looking into Enough1122, I found 199 similar comments in a recent sample over about 14 hours.
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
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.
Issue for this PR
Closes #43472
Related to #40094. That issue also covers stale project records after a repository is moved.
Type of change
What does this PR do?
This adds
useOpenProject. The hook checks the project list and asks the server to recognize an unknown directory before it opens the project.It also adds
useDeepLinks. This hook owns the pending links, desktop event listener, route creation, and project-open sequence.Both layouts now use the same hooks. The legacy layout keeps its session handoff and worktree normalization through hook options.
How did you verify your code works?
bun typecheckinpackages/appbun test src/pages/layout/helpers.test.tsinpackages/app(27 tests pass)git diff --checkScreenshots / recordings
Not applicable. This changes project recognition and navigation behavior without changing the UI.
Checklist
If you do not follow this template your PR will be automatically rejected.