Skip to content
Merged
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
12 changes: 4 additions & 8 deletions apps/macos/src/main/about.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { BrowserWindow, app, ipcMain, shell } from "electron";
import path from "node:path";

import { RENDERER_BASE_PROD, getDevRendererBase } from "./app-config";

/**
* Branded About window — replaces Electron's default `aboutPanel`
* (which only shows the bundle name) with the same information surface
Expand Down Expand Up @@ -60,14 +62,8 @@ export const getVersionInfo = (): AppVersionInfo => ({
const ABOUT_PATH = "/about";

const aboutWindowUrl = (): string => {
if (app.isPackaged) return `app://vellum.ai/assistant${ABOUT_PATH}`;
// Dev: build atop the same env-var the main window honors. Strip any
// trailing slash so a `VELLUM_DEV_URL=http://localhost:5173/assistant/`
// override doesn't produce `/assistant//about`.
const devBase = (
process.env.VELLUM_DEV_URL ?? "http://localhost:5173/assistant"
).replace(/\/+$/, "");
return `${devBase}${ABOUT_PATH}`;
const base = app.isPackaged ? RENDERER_BASE_PROD : getDevRendererBase();
return `${base}${ABOUT_PATH}`;
};

// Module-scope handle so reopening the menu item focuses the existing
Expand Down
39 changes: 39 additions & 0 deletions apps/macos/src/main/app-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Shared application identity constants for the main process.
*
* `app-protocol` and `app-host` define the custom scheme the packaged
* renderer is served from. They're referenced from at least three
* places today (`index.ts` for `protocol.registerSchemesAsPrivileged`
* + the `protocol.handle` registration, `main-window.ts` for the
* BrowserWindow load URL and the same-origin navigation guard,
* `about.ts` for the About window's prod URL), so they live here as a
* single source of truth. Drift between callers would have shown up
* as a broken renderer load rather than a build error, which is
* exactly the kind of thing a small shared module prevents.
*
* The renderer-base URLs are derived: `RENDERER_BASE_PROD` is the
* packaged path the `app://` protocol handler resolves to; the dev
* path is honored from `VELLUM_DEV_URL` (vel's edge proxy, or the
* local Vite default at port 5173). Both end at the `/assistant`
* suffix that `apps/web/vite.config.ts`'s `base` setting requires.
*/

export const APP_PROTOCOL = "app";
export const APP_HOST = "vellum.ai";

const DEV_SERVER_FALLBACK_URL = "http://localhost:5173/assistant";

/**
* Renderer-base URL for the packaged app. Auxiliary windows append
* their own subpath (`/about`, future `/conversations/<id>`, etc.).
*/
export const RENDERER_BASE_PROD = `${APP_PROTOCOL}://${APP_HOST}/assistant`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Serve the /assistant prefix before packaging

In packaged builds this base causes the loaded HTML to request assets under /assistant/assets/... because apps/web/vite.config.ts sets base: "/assistant/", but the app:// handler in apps/macos/src/main/index.ts joins the full URL pathname directly under rendererRoot and only serves files that exist there. Since the web build emits files under rendererRoot/assets/... (not rendererRoot/assistant/assets/...), the main JS/CSS requests 404 and the packaged Electron app opens a blank/broken renderer unless the protocol handler strips the /assistant mount prefix or the files are copied under that prefix.

Useful? React with 👍 / 👎.


/**
* Renderer-base URL in dev. Honors `VELLUM_DEV_URL` so the launcher
* can point at whichever Vite-or-equivalent is up (standalone Vite
* at :5173, or vel's edge proxy at :3000). Strips any trailing slash
* so callers can append `/<subpath>` without producing `//`.
*/
export const getDevRendererBase = (): string =>
(process.env.VELLUM_DEV_URL ?? DEV_SERVER_FALLBACK_URL).replace(/\/+$/, "");
78 changes: 78 additions & 0 deletions apps/macos/src/main/app-protocol.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,81 @@ describe("resolveRelativePath — startsWith guard", () => {
});
});
});

// `apps/web/vite.config.ts` sets `base: "/assistant/"`, so the
// built HTML emits asset URLs like `/assistant/assets/index.js`.
// The renderer files on disk live directly under `rendererRoot`,
// NOT under a `/assistant/` subdirectory. The protocol handler
// passes `mountPrefix: "/assistant"` so these tests pin the
// stripping behavior.
describe("resolveAppProtocolPath — mount-prefix stripping", () => {
test("maps `/assistant` to the root itself", () => {
expect(
resolveAppProtocolPath(ROOT, "app://vellum.ai/assistant", "/assistant"),
).toEqual({ kind: "ok", resolved: ROOT });
});

test("maps `/assistant/<rest>` to `rendererRoot/<rest>`", () => {
expect(
resolveAppProtocolPath(
ROOT,
"app://vellum.ai/assistant/assets/index.js",
"/assistant",
),
).toEqual({ kind: "ok", resolved: path.join(ROOT, "assets/index.js") });
});

test("a nested deep path under the mount maps cleanly", () => {
expect(
resolveAppProtocolPath(
ROOT,
"app://vellum.ai/assistant/assets/css/app.css",
"/assistant",
),
).toEqual({
kind: "ok",
resolved: path.join(ROOT, "assets/css/app.css"),
});
});

test("a path that doesn't start with the mount is passed through (and 404s as usual)", () => {
// `/favicon.ico` lives at the bare protocol root. Stays under
// `rendererRoot/favicon.ico` (where Vite emits favicon).
expect(
resolveAppProtocolPath(
ROOT,
"app://vellum.ai/favicon.ico",
"/assistant",
),
).toEqual({ kind: "ok", resolved: path.join(ROOT, "favicon.ico") });
});

test("a sibling that shares the mount prefix as a string is NOT stripped", () => {
// `/assistantfoo` shares `/assistant` as a string prefix but
// isn't under the mount. The strip only fires for exact match or
// `/assistant/` prefix.
expect(
resolveAppProtocolPath(
ROOT,
"app://vellum.ai/assistantfoo/bar",
"/assistant",
),
).toEqual({
kind: "ok",
resolved: path.join(ROOT, "assistantfoo/bar"),
});
});

test("path-traversal guard still fires for stripped paths", () => {
// `/assistant/../etc/passwd` — URL normalization collapses to
// `/etc/passwd`, which doesn't start with the mount, so no strip.
// The result lands at `rendererRoot/etc/passwd` (in-root, ok).
expect(
resolveAppProtocolPath(
ROOT,
"app://vellum.ai/assistant/../etc/passwd",
"/assistant",
),
).toEqual({ kind: "ok", resolved: path.join(ROOT, "etc/passwd") });
});
});
20 changes: 19 additions & 1 deletion apps/macos/src/main/app-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,29 @@ export const resolveRelativePath = (
export const resolveAppProtocolPath = (
rendererRoot: string,
requestUrl: string,
mountPrefix?: string,
): ResolveResult => {
const url = new URL(requestUrl);
// `apps/web/vite.config.ts` sets `base: "/assistant/"`, so the
// built HTML references assets under `/assistant/assets/...`. The
// renderer files on disk live directly under `rendererRoot` — they
// are NOT nested in a `/assistant/` subdirectory. Stripping the
// mount prefix here maps `/assistant/<rest>` requests to
// `rendererRoot/<rest>`. A `mountPrefix` of `"/assistant"` matches
// both the bare `/assistant` (mapped to the root) and
// `/assistant/<rest>` paths; other top-level requests pass
// through untouched (and 404 or land in `forbidden`).
let pathname = url.pathname;
if (mountPrefix) {
if (pathname === mountPrefix) {
pathname = "/";
} else if (pathname.startsWith(`${mountPrefix}/`)) {
pathname = pathname.slice(mountPrefix.length);
}
}
let relativePath: string;
try {
relativePath = decodeURIComponent(url.pathname).replace(/^\/+/, "");
relativePath = decodeURIComponent(pathname).replace(/^\/+/, "");
} catch {
// Malformed percent-encoding (e.g. `%ZZ`) throws `URIError`.
// Convert to `forbidden` so the protocol handler returns a clean
Expand Down
40 changes: 25 additions & 15 deletions apps/macos/src/main/dock.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
import { describe, expect, test } from "bun:test";
import { describe, expect, mock, test } from "bun:test";

import { computePolicy, formatBadge } from "./dock";
// `./main-window` (which `./dock` imports `current` /
// `onMainWindowVisibilityChange` from) transitively pulls in
// `./window-state`, which depends on the `electron-store` module —
// stub both so the pure-function tests below don't need a real
// store. The mocks are no-ops; the `computePolicy` matrix tests
// only exercise the pure path.
mock.module("./main-window", () => ({
current: () => null,
onMainWindowVisibilityChange: () => undefined,
}));

const { computePolicy, formatBadge } = await import("./dock");

describe("formatBadge", () => {
test("returns empty string for zero", () => {
Expand Down Expand Up @@ -44,26 +55,25 @@ describe("formatBadge", () => {
});

describe("computePolicy", () => {
test("regular while any window is visible (signed out, gate off)", () => {
expect(computePolicy(1, false, false)).toBe("regular");
expect(computePolicy(3, false, false)).toBe("regular");
test("regular while the main window is visible (signed out, gate off)", () => {
expect(computePolicy(true, false, false)).toBe("regular");
});

test("regular while signed in even with no visible windows", () => {
expect(computePolicy(0, true, false)).toBe("regular");
expect(computePolicy(0, true, true)).toBe("regular");
test("regular while signed in even when the main window is hidden", () => {
expect(computePolicy(false, true, false)).toBe("regular");
expect(computePolicy(false, true, true)).toBe("regular");
});

test("regular when signed out + no windows AND accessory gate off", () => {
expect(computePolicy(0, false, false)).toBe("regular");
test("regular when signed out + main hidden AND accessory gate off", () => {
expect(computePolicy(false, false, false)).toBe("regular");
});

test("accessory only when signed out + no windows + gate on", () => {
expect(computePolicy(0, false, true)).toBe("accessory");
test("accessory only when signed out + main hidden + gate on", () => {
expect(computePolicy(false, false, true)).toBe("accessory");
});

test("visible windows override every other signal", () => {
expect(computePolicy(2, false, true)).toBe("regular");
expect(computePolicy(1, true, true)).toBe("regular");
test("main-window visibility overrides every other signal", () => {
expect(computePolicy(true, false, true)).toBe("regular");
expect(computePolicy(true, true, true)).toBe("regular");
});
});
84 changes: 57 additions & 27 deletions apps/macos/src/main/dock.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { app, BrowserWindow, ipcMain } from "electron";
import { app, ipcMain } from "electron";

import {
current as currentMainWindow,
onMainWindowVisibilityChange,
} from "./main-window";

/**
* Dock integration: unread-count badge + visibility state machine.
Expand All @@ -10,19 +15,22 @@ import { app, BrowserWindow, ipcMain } from "electron";
*
* The state machine has two inputs:
*
* 1. **Visible window count**, observed via the `browser-window-created`
* / per-window `closed` events. No renderer involvement.
* 1. **Main-window visibility**, subscribed via
* `onMainWindowVisibilityChange` on `./main-window`. Auxiliary
* windows (About, future thread pop-outs) deliberately do NOT
* drive dock policy — opening About while signed out would
* otherwise flicker the dock icon to `regular` and back.
* 2. **Signed-in flag**, published by the renderer over the
* `vellum:dock:setSignedIn` IPC channel. Renderer is the source of
* truth today; this side of the bridge becomes a no-op once the
* main-process auth state is the canonical signal.
*
* Policy:
*
* - Any visible window OR signed in → `regular` (Dock icon visible).
* - Main visible OR signed in → `regular` (Dock icon visible).
* We keep the icon visible while signed in so the user can re-open
* the window from the Dock after closing the last one.
* - No visible window AND signed out → `accessory` (Dock icon hidden,
* - Main hidden AND signed out → `accessory` (Dock icon hidden,
* menu-bar-only).
*
* Transitions are debounced ~100ms so a fast close-then-open (e.g.
Expand All @@ -45,11 +53,13 @@ export const formatBadge = (count: number): string => {

const POLICY_DEBOUNCE_MS = 100;

// Gate the `accessory` (menu-bar-only) transition until a menu-bar
// (tray) entry point exists. Going accessory before then would hide
// the Dock icon with no replacement, leaving the user no way to re-open
// the window. Flip to `true` in the same change that lands the tray.
const ALLOW_ACCESSORY_MODE = false;
// When the user is signed out and no windows are visible, drop the
// Dock icon and run menu-bar-only — same accessory-mode UX Swift Vellum
// exposes. Safe to flip on now that `installTray` provides an
// always-available entry point back to the app (the Dock icon used to
// be the only one). Kept as a module-level constant rather than a
// setting so the build-time choice is reviewable.
const ALLOW_ACCESSORY_MODE = true;
Comment thread
ashleeradka marked this conversation as resolved.

interface DockState {
signedIn: boolean;
Expand All @@ -65,20 +75,30 @@ const state: DockState = {

let refreshTimer: NodeJS.Timeout | null = null;

const visibleWindowCount = (): number =>
BrowserWindow.getAllWindows().filter((win) => !win.isDestroyed()).length;
// True iff the main window is the visible, focusable surface. The dock
// policy is conceptually about the main window — not about "any window
// being alive" — so we check by reference via `main-window.current()`
// rather than scanning every BrowserWindow. Auxiliary windows
// (About, future thread pop-outs, command palette) should NOT keep
// the dock icon visible when the user is signed out — that would be
// a UX bug: opening About would briefly show the dock icon and
// closing it would hide it again.
const isMainWindowVisible = (): boolean => {
const win = currentMainWindow();
return !!win && !win.isDestroyed() && win.isVisible();
};

// Pure function of (visible window count, signed-in flag, accessory-mode
// gate). Factored out of the caller so tests can exercise the matrix
// without standing up a full Electron `BrowserWindow` registry — the
// caller passes `visibleWindowCount()` + `state.signedIn` +
// Pure function of (main-window visibility, signed-in flag,
// accessory-mode gate). Factored out so tests can exercise the
// 2×2×2 matrix without standing up an Electron BrowserWindow.
// Caller passes `isMainWindowVisible()` + `state.signedIn` +
// `ALLOW_ACCESSORY_MODE` at the seam.
export const computePolicy = (
visibleWindows: number,
mainVisible: boolean,
signedIn: boolean,
allowAccessoryMode: boolean,
): DockState["policy"] => {
if (visibleWindows > 0) return "regular";
if (mainVisible) return "regular";
if (signedIn) return "regular";
return allowAccessoryMode ? "accessory" : "regular";
};
Expand All @@ -88,12 +108,23 @@ export const computePolicy = (
// keeps the two surfaces in sync (await sequencing is the documented
// pattern). The accessory transition is synchronous on the Electron
// side — `hide()` returns void — so no await there.
//
// Re-check `state.policy` after the awaited `dock.show()`. Between
// the await and resume, another `applyPolicy("accessory")` may have
// run synchronously and flipped both the state and the activation
// policy. Without the re-check, the resuming `regular` call would
// stomp the newer `accessory` setActivationPolicy, leaving the
// activation policy and dock visibility out of sync.
const applyPolicy = async (next: DockState["policy"]): Promise<void> => {
if (next === state.policy) return;
state.policy = next;
if (!app.dock) return;
if (next === "regular") {
await app.dock.show();
// A concurrent `applyPolicy("accessory")` may have run while we
// were awaiting — bail out so we don't override its
// `setActivationPolicy("accessory")`.
if (state.policy !== "regular") return;
app.setActivationPolicy("regular");
} else {
app.dock.hide();
Expand All @@ -106,7 +137,7 @@ const scheduleRefresh = (): void => {
refreshTimer = setTimeout(() => {
refreshTimer = null;
void applyPolicy(
computePolicy(visibleWindowCount(), state.signedIn, ALLOW_ACCESSORY_MODE),
computePolicy(isMainWindowVisible(), state.signedIn, ALLOW_ACCESSORY_MODE),
);
}, POLICY_DEBOUNCE_MS);
};
Expand Down Expand Up @@ -153,13 +184,12 @@ export const installDock = (): void => {
scheduleRefresh();
});

// Observe the visible-window count so closing the last window can
// transition us into accessory mode (once `ALLOW_ACCESSORY_MODE` is
// flipped), and opening the first one transitions us back to regular.
app.on("browser-window-created", (_event, win) => {
scheduleRefresh();
win.once("closed", scheduleRefresh);
});
// Subscribe to main-window visibility transitions (created, shown,
// hidden, closed). Auxiliary windows (About, future thread pop-outs,
// command palette) don't fire this hook, so they correctly don't
// affect dock policy — a signed-out user opening About would
// otherwise briefly flicker the dock icon to `regular` and back.
onMainWindowVisibilityChange(scheduleRefresh);

// macOS convention: clear the Dock badge before the process exits so
// a relaunch doesn't briefly show a stale count from the OS's cache.
Expand All @@ -177,7 +207,7 @@ export const installDock = (): void => {
// following `setActivationPolicy` call inside `applyPolicy`; the
// caller has nothing to await on.
void applyPolicy(
computePolicy(visibleWindowCount(), state.signedIn, ALLOW_ACCESSORY_MODE),
computePolicy(isMainWindowVisible(), state.signedIn, ALLOW_ACCESSORY_MODE),
);
applyBadge();
};
Loading