Skip to content

Commit 3264ff7

Browse files
fix(desktop): stop listing browsers that are not installed
Detection keyed off the browser's user-data directory, which is not evidence the browser exists. Installers for native messaging hosts create an empty one for every Chromium fork they know about, so a machine with only Chrome and Helium listed Edge, Brave, Vivaldi, Opera and Arc as importable sources — each holding nothing but a `NativeMessagingHosts` folder. It now keys off the cookie database, which is the thing an import actually needs. Existence is checked without opening the file, which matters for Safari: TCC permits `stat` on the jar inside its container but refuses a read, so Safari is still found and the user gets the Full Disk Access prompt instead of Safari vanishing from the list. A source that is not on the machine is now left out of the menu rather than shown as a dead row. Every other unavailable reason stays visible, because each names something the user can do — quit the browser, grant access. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ea2efcb commit 3264ff7

4 files changed

Lines changed: 64 additions & 17 deletions

File tree

apps/desktop/src/preview/BrowserImport/BrowserImport.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ const withImporter = Effect.fnUntraced(function* () {
4040
yield* fileSystem.makeDirectory(`${helium.userDataDirectory(paths)}/Default`, {
4141
recursive: true,
4242
});
43+
// The cookie database is what marks a source as installed, so a fixture
44+
// without one is reported as absent before any other check runs.
45+
yield* fileSystem.writeFileString(`${helium.userDataDirectory(paths)}/Default/Cookies`, "db");
4346

4447
const importer = yield* BrowserImport.BrowserImport.pipe(
4548
Effect.provide(

apps/desktop/src/preview/BrowserImport/Sources.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,24 @@ describe("isSourceRunning", () => {
5454
});
5555

5656
describe("isSourceInstalled", () => {
57-
it.effect("follows the presence of the user-data directory", () =>
57+
it.effect("ignores a user-data directory that holds no cookie database", () =>
5858
run(
5959
Effect.gen(function* () {
6060
const fileSystem = yield* FileSystem.FileSystem;
6161
const paths = yield* withSourceHome();
62+
const root = helium.userDataDirectory(paths);
63+
64+
// Installers for native messaging hosts create an empty user-data
65+
// directory for every Chromium fork they know about, so treating the
66+
// directory as evidence lists browsers the user does not have.
67+
yield* fileSystem.makeDirectory(`${root}/NativeMessagingHosts`, { recursive: true });
68+
assert.isFalse(yield* isSourceInstalled(helium, paths));
69+
70+
yield* fileSystem.makeDirectory(`${root}/Default`, { recursive: true });
71+
yield* fileSystem.writeFileString(`${root}/Default/Cookies`, "db");
6272
assert.isTrue(yield* isSourceInstalled(helium, paths));
6373

64-
yield* fileSystem.remove(helium.userDataDirectory(paths), { recursive: true });
74+
yield* fileSystem.remove(root, { recursive: true });
6575
assert.isFalse(yield* isSourceInstalled(helium, paths));
6676
}),
6777
),

apps/desktop/src/preview/BrowserImport/Sources.ts

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,26 @@ export const listSourceProfiles = Effect.fn("BrowserImportSources.listSourceProf
101101
return profiles.length === 0 ? DEFAULT_PROFILES : profiles;
102102
});
103103

104+
/**
105+
* Whether a directory entry exists, without following it or opening it.
106+
*
107+
* `stat` resolves symlinks and the locks below deliberately dangle, so
108+
* `readLink` is the probe that answers for the entry itself.
109+
*/
110+
const entryExists = Effect.fnUntraced(function* (path: string) {
111+
const fileSystem = yield* FileSystem.FileSystem;
112+
return yield* fileSystem.stat(path).pipe(
113+
Effect.catchCause(() => fileSystem.readLink(path)),
114+
Effect.as(true),
115+
Effect.orElseSucceed(() => false),
116+
);
117+
});
118+
104119
/** Whether the browser is running, which leaves its cookie DB mid-write. */
105120
export const isSourceRunning = Effect.fn("BrowserImportSources.isSourceRunning")(function* (
106121
definition: BrowserImportSourceDefinition,
107122
paths: SourcePaths,
108123
) {
109-
const fileSystem = yield* FileSystem.FileSystem;
110124
const lock = paths.path.join(definition.userDataDirectory(paths), "SingletonLock");
111125
// Chromium writes a `SingletonLock` symlink for as long as an instance holds
112126
// the profile. Its presence is a far cheaper and more targeted signal than
@@ -116,19 +130,31 @@ export const isSourceRunning = Effect.fn("BrowserImportSources.isSourceRunning")
116130
// `stat` and `exists` follow links — so they report every running browser as
117131
// closed, which would let an import read a live, mid-write database.
118132
// `readLink` is the one probe that answers for the entry itself.
119-
return yield* fileSystem.stat(lock).pipe(
120-
Effect.catchCause(() => fileSystem.readLink(lock)),
121-
Effect.as(true),
122-
Effect.orElseSucceed(() => false),
123-
);
133+
return yield* entryExists(lock);
124134
});
125135

136+
/**
137+
* Whether the source has cookies to import.
138+
*
139+
* Keyed off the cookie database rather than the user-data directory, because
140+
* that directory is not evidence the browser exists: installers for native
141+
* messaging hosts create an empty one for every Chromium fork they know about,
142+
* so a machine with only Chrome reports Edge, Brave, Vivaldi, Opera and Arc as
143+
* present. The database is the thing an import actually needs, so its absence
144+
* is the honest answer either way.
145+
*
146+
* Existence is checked without opening the file, which matters for Safari: TCC
147+
* permits `stat` on the jar inside its container but refuses a read, so this
148+
* still sees it and the user gets the Full Disk Access prompt rather than
149+
* having Safari disappear.
150+
*/
126151
export const isSourceInstalled = Effect.fn("BrowserImportSources.isSourceInstalled")(function* (
127152
definition: BrowserImportSourceDefinition,
128153
paths: SourcePaths,
129154
) {
130-
const fileSystem = yield* FileSystem.FileSystem;
131-
return yield* fileSystem
132-
.exists(definition.userDataDirectory(paths))
133-
.pipe(Effect.orElseSucceed(() => false));
155+
const profiles = yield* listSourceProfiles(definition, paths);
156+
const found = yield* Effect.forEach(profiles, (profile) =>
157+
entryExists(cookieDatabasePath(definition, paths, profile.directory)),
158+
);
159+
return found.some(Boolean);
134160
});

apps/web/src/components/settings/IntegrationsSettings.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,14 @@ function BrowserProfilesSetting({ disabled }: { readonly disabled: boolean }) {
595595
});
596596
};
597597

598+
// A browser that is not on this machine is left out rather than listed as a
599+
// dead row: there is nothing to act on, and the menu is a list of things you
600+
// can import from. Every other unavailable reason stays, since each names a
601+
// step the user can take.
602+
const importableSources = (sources ?? []).filter(
603+
(source) => source.unavailable !== "notInstalled",
604+
);
605+
598606
const loadSources = () => {
599607
if (!previewBridge) return;
600608
// Cleared first: availability changes while the app runs (quitting a
@@ -678,15 +686,15 @@ function BrowserProfilesSetting({ disabled }: { readonly disabled: boolean }) {
678686
<MenuGroupLabel>Import from</MenuGroupLabel>
679687
{sources === null ? (
680688
<MenuItem disabled>Looking for browsers…</MenuItem>
681-
) : sources.length === 0 ? (
689+
) : importableSources.length === 0 ? (
682690
<MenuItem disabled>No supported browsers found</MenuItem>
683691
) : (
684-
sources.flatMap((source) =>
692+
importableSources.flatMap((source) =>
685693
source.unavailable
686694
? [
687-
// Kept visible with its reason rather than hidden:
688-
// "Helium is running, quit it" beats "Helium isn't
689-
// listed".
695+
// Kept visible with its reason, because each remaining
696+
// reason is something the user can act on: "Helium is
697+
// running, quit it" beats "Helium isn't listed".
690698
<MenuItem key={source.id} disabled>
691699
<span className="flex flex-col items-start">
692700
<span>{source.name}</span>

0 commit comments

Comments
 (0)