Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions apps/mobile/src/features/projects/AddProjectScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import { cn } from "../../lib/cn";
import { useProjects, useServerConfigs } from "../../state/entities";
import { filesystemEnvironment } from "../../state/filesystem";
import { projectEnvironment } from "../../state/projects";
import { useEnvironmentQuery } from "../../state/query";
import { useEnvironmentQuery, useWarmEnvironmentQueryRevalidation } from "../../state/query";
import { sourceControlEnvironment } from "../../state/sourceControl";
import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText";
import { EnvironmentMachineSymbol } from "../../components/EnvironmentMachineSymbol";
Expand Down Expand Up @@ -761,14 +761,17 @@ function FolderBrowser(props: {
() => (browsePath.directoryPath.length > 0 ? { partialPath: browsePath.directoryPath } : null),
[browsePath.directoryPath],
);
const browseState = useEnvironmentQuery(
const browseAtom =
browseInput === null
? null
: filesystemEnvironment.browse({
environmentId: props.environment.environmentId,
input: browseInput,
}),
);
});
const browseState = useEnvironmentQuery(browseAtom);
// The browser unmounts with its screen while the browse atoms stay warm, so
// re-entering the add-project flow must revalidate against the filesystem.
useWarmEnvironmentQueryRevalidation(browseAtom);

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.

馃煚 High projects/AddProjectScreen.tsx:773

Reopening the picker does not retry a cached browse failure, so a transient filesystem or connection error remains displayed until the atom's five-minute idle TTL expires. useWarmEnvironmentQueryRevalidation only refreshes non-waiting Success results and marks the atom handled before that check; refresh any non-waiting settled result, including Failure, so reopen revalidates cached errors.

Also found in 2 other location(s)

apps/mobile/src/state/query.ts:47

Cached failures are never revalidated. Failure is also a settled AsyncResult (the shared runtime defines SettledAsyncResult as Success | Failure), but this condition only refreshes successful values and the ref is set before it. Thus, if a browse RPC fails, closing and reopening the picker within its five-minute idle TTL remounts the cached error, marks that atom as handled, and sends no retry request; the picker remains unusable until eviction or a manual refresh. Refresh any non-waiting settled result (or do not mark failures handled) so transient filesystem/connection errors retry on reopen.

apps/web/src/state/query.ts:48

Cached failures are never revalidated. Failure is also a settled AsyncResult (the shared runtime defines SettledAsyncResult as Success | Failure), but this condition only refreshes successful values and the ref is set before it. Thus, if a browse RPC fails, closing and reopening the picker within its five-minute idle TTL remounts the cached error, marks that atom as handled, and sends no retry request; the picker remains unusable until eviction or a manual refresh. Refresh any non-waiting settled result (or do not mark failures handled) so transient filesystem/connection errors retry on reopen.

馃 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/projects/AddProjectScreen.tsx around line 773:

Reopening the picker does not retry a cached browse failure, so a transient filesystem or connection error remains displayed until the atom's five-minute idle TTL expires. `useWarmEnvironmentQueryRevalidation` only refreshes non-waiting `Success` results and marks the atom handled before that check; refresh any non-waiting settled result, including `Failure`, so reopen revalidates cached errors.

Also found in 2 other location(s):
- apps/mobile/src/state/query.ts:47 -- Cached failures are never revalidated. `Failure` is also a settled `AsyncResult` (the shared runtime defines `SettledAsyncResult` as `Success | Failure`), but this condition only refreshes successful values and the ref is set before it. Thus, if a browse RPC fails, closing and reopening the picker within its five-minute idle TTL remounts the cached error, marks that atom as handled, and sends no retry request; the picker remains unusable until eviction or a manual refresh. Refresh any non-waiting settled result (or do not mark failures handled) so transient filesystem/connection errors retry on reopen.
- apps/web/src/state/query.ts:48 -- Cached failures are never revalidated. `Failure` is also a settled `AsyncResult` (the shared runtime defines `SettledAsyncResult` as `Success | Failure`), but this condition only refreshes successful values and the ref is set before it. Thus, if a browse RPC fails, closing and reopening the picker within its five-minute idle TTL remounts the cached error, marks that atom as handled, and sends no retry request; the picker remains unusable until eviction or a manual refresh. Refresh any non-waiting settled result (or do not mark failures handled) so transient filesystem/connection errors retry on reopen.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 560723da2. The hook now refreshes any settled result instead of only successes, so a cached browse failure retries on reopen rather than sticking for the idle TTL. Successes younger than a short freshness window are skipped so navigation prefetches are not fetched twice (see the sibling CodeRabbit thread).

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.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

// A pinned repository folder does not exist yet, so filtering the listing by
// it would empty the folder picker. Anything the user typed still filters.
const pinnedDirectoryName = props.pinnedDirectoryName ?? "";
Expand Down
39 changes: 39 additions & 0 deletions apps/mobile/src/state/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useAtomRefresh, useAtomValue } from "@effect/atom-react";
import * as Cause from "effect/Cause";
import * as Option from "effect/Option";
import { AsyncResult, Atom } from "effect/unstable/reactivity";
import { useEffect, useRef } from "react";

const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(false)).pipe(
Atom.withLabel("mobile-environment-query:empty"),
Expand All @@ -21,6 +22,44 @@ function formatError(cause: Cause.Cause<unknown>): string {
: "The environment request failed.";
}

// A success the picker's own navigation prefetched moments before the atom
// mounts is fresh; only values that sat in the warm cache need revalidation.
const WARM_QUERY_FRESH_MS = 500;

/**
* Revalidates an environment query that mounts onto a warm cached atom.
*
* Query atoms outlive their subscribers through an idle TTL, and the swr
* wrapper only re-checks staleness when the atom node is rebuilt. A view that
* remounts onto a warm node therefore renders the cached value and never
* refetches, which freezes reads whose ground truth changes outside the app,
* such as the filesystem browse listing. Refreshes once per atom when it
* mounts holding a settled result: successes older than a short freshness
* window (so navigation prefetches are not fetched twice) and failures always
* (so a transient error does not stick for the whole TTL). Cold atoms are
* left to their own initial fetch.
*/
export function useWarmEnvironmentQueryRevalidation<A, E>(
atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
): void {
const result = useAtomValue(atom ?? EMPTY_ASYNC_RESULT_ATOM);
const refresh = useAtomRefresh(atom ?? EMPTY_ASYNC_RESULT_ATOM);
const revalidatedAtom = useRef<Atom.Atom<AsyncResult.AsyncResult<A, E>> | null>(null);
useEffect(() => {
if (atom === null || revalidatedAtom.current === atom) {
return;
}
revalidatedAtom.current = atom;
if (result.waiting || result._tag === "Initial") {
return;
}
if (result._tag === "Success" && Date.now() - result.timestamp < WARM_QUERY_FRESH_MS) {
return;
}
refresh();
}, [atom, refresh, result]);
}

export function useEnvironmentQuery<A, E>(
atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
): EnvironmentQueryView<A> {
Expand Down
17 changes: 10 additions & 7 deletions apps/web/src/components/CommandPalette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ import { readLocalApi } from "../localApi";
import { desktopLocalBackendId } from "../connection/desktopLocal";
import { filesystemEnvironment } from "../state/filesystem";
import { projectEnvironment } from "../state/projects";
import { useEnvironmentQuery } from "../state/query";
import { useEnvironmentQuery, useWarmEnvironmentQueryRevalidation } from "../state/query";
import { sourceControlEnvironment } from "../state/sourceControl";
import { useAtomCommand } from "../state/use-atom-command";
import { useAtomQueryRunner } from "../state/use-atom-query-runner";
Expand Down Expand Up @@ -1006,20 +1006,23 @@ function OpenCommandPaletteDialog(props: {
);
const relativePathNeedsActiveProject =
isExplicitRelativeProjectPath(query.trim()) && currentProjectCwdForBrowse === null;
const browseQuery = useEnvironmentQuery(
const browseAtom =
isBrowsing &&
browsePath.directoryPath.length > 0 &&
browseEnvironmentId !== null &&
!relativePathNeedsActiveProject
browsePath.directoryPath.length > 0 &&
browseEnvironmentId !== null &&
!relativePathNeedsActiveProject
? filesystemEnvironment.browse({
environmentId: browseEnvironmentId,
input: {
partialPath: browsePath.directoryPath,
...(currentProjectCwdForBrowse ? { cwd: currentProjectCwdForBrowse } : {}),
},
})
: null,
);
: null;
const browseQuery = useEnvironmentQuery(browseAtom);
// The palette dialog unmounts on close while the browse atoms stay warm, so
// reopening the folder picker must revalidate against the real filesystem.
useWarmEnvironmentQueryRevalidation(browseAtom);
const browseResult = browseQuery.data;
const isBrowsePending = browseQuery.isPending;
const browseEntries = browseResult?.entries ?? EMPTY_BROWSE_ENTRIES;
Expand Down
39 changes: 39 additions & 0 deletions apps/web/src/state/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useAtomRefresh, useAtomValue } from "@effect/atom-react";
import * as Cause from "effect/Cause";
import * as Option from "effect/Option";
import { AsyncResult, Atom } from "effect/unstable/reactivity";
import { useEffect, useRef } from "react";

const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(false)).pipe(
Atom.withLabel("web-environment-query:empty"),
Expand All @@ -22,6 +23,44 @@ export function formatEnvironmentQueryError(cause: Cause.Cause<unknown>): string
: "The environment request failed.";
}

// A success the picker's own navigation prefetched moments before the atom
// mounts is fresh; only values that sat in the warm cache need revalidation.
const WARM_QUERY_FRESH_MS = 500;

/**
* Revalidates an environment query that mounts onto a warm cached atom.
*
* Query atoms outlive their subscribers through an idle TTL, and the swr
* wrapper only re-checks staleness when the atom node is rebuilt. A view that
* remounts onto a warm node therefore renders the cached value and never
* refetches, which freezes reads whose ground truth changes outside the app,
* such as the filesystem browse listing. Refreshes once per atom when it
* mounts holding a settled result: successes older than a short freshness
* window (so navigation prefetches are not fetched twice) and failures always
* (so a transient error does not stick for the whole TTL). Cold atoms are
* left to their own initial fetch.
*/
export function useWarmEnvironmentQueryRevalidation<A, E>(
atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
): void {
const result = useAtomValue(atom ?? EMPTY_ASYNC_RESULT_ATOM);
const refresh = useAtomRefresh(atom ?? EMPTY_ASYNC_RESULT_ATOM);
const revalidatedAtom = useRef<Atom.Atom<AsyncResult.AsyncResult<A, E>> | null>(null);
useEffect(() => {
if (atom === null || revalidatedAtom.current === atom) {
return;
}
revalidatedAtom.current = atom;
if (result.waiting || result._tag === "Initial") {
return;
}
if (result._tag === "Success" && Date.now() - result.timestamp < WARM_QUERY_FRESH_MS) {
return;
}
refresh();
}, [atom, refresh, result]);
}

export function useEnvironmentQuery<A, E>(
atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
): EnvironmentQueryView<A> {
Expand Down
Loading