From 39e633c136243443f191982a3e81fe50d4e7cd32 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 5 Jun 2026 10:39:18 -0700 Subject: [PATCH] fix: address review findings (#4704) --- config/scripts/run-electron-vite-dev.mjs | 22 +++++-- src/main/index.ts | 2 +- .../fake-electron-vite-dev-cli.mjs | 1 + .../startup/dev-instance-identity.test.ts | 15 +++++ src/main/startup/dev-instance-identity.ts | 10 +++- .../startup/run-electron-vite-dev.test.ts | 60 ++++++++++++++++++- 6 files changed, 101 insertions(+), 9 deletions(-) diff --git a/config/scripts/run-electron-vite-dev.mjs b/config/scripts/run-electron-vite-dev.mjs index a818f3e64..cb5ebc6ec 100644 --- a/config/scripts/run-electron-vite-dev.mjs +++ b/config/scripts/run-electron-vite-dev.mjs @@ -28,15 +28,27 @@ delete process.env.ELECTRON_RUN_AS_NODE const require = createRequire(import.meta.url) const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..') const STABLE_NAME_FLAG = '--stable-name' +const BRANCH_APP_NAME_FLAG = '--branch-app-name' const rawForwardedArgs = process.argv.slice(2) -// Why: keep an escape hatch for tools that key off Electron's stock app name. -// The flag is runner-only and must not leak into Chromium/electron-vite. -const useStableElectronName = +const stableNameRequested = process.env.ORCA_DEV_STABLE_NAME === '1' || rawForwardedArgs.includes(STABLE_NAME_FLAG) -const forwardedRaw = rawForwardedArgs.filter((arg) => arg !== STABLE_NAME_FLAG) +const branchAppNameRequested = + process.env.ORCA_DEV_BRANCH_APP_NAME === '1' || rawForwardedArgs.includes(BRANCH_APP_NAME_FLAG) +// Why: branch-named macOS dev bundles make Keychain treat each worktree as a +// new app reading "Orca Safe Storage"; keep that unstable identity opt-in. +const useStableElectronName = + stableNameRequested || (process.platform === 'darwin' && !branchAppNameRequested) +const useBranchMacAppName = + process.platform === 'darwin' && branchAppNameRequested && !useStableElectronName +const forwardedRaw = rawForwardedArgs.filter( + (arg) => arg !== STABLE_NAME_FLAG && arg !== BRANCH_APP_NAME_FLAG +) if (useStableElectronName) { process.env.ORCA_DEV_STABLE_NAME = '1' } +if (useBranchMacAppName) { + process.env.ORCA_DEV_BRANCH_APP_NAME = '1' +} function readGitValue(args) { try { @@ -314,7 +326,7 @@ if (process.env.ORCA_SKIP_DEV_CLI_PREPARE !== '1') { } seedDevInstanceIdentityEnv() -if (!useStableElectronName && process.env.ORCA_SKIP_DEV_ELECTRON_APP_PREPARE !== '1') { +if (useBranchMacAppName && process.env.ORCA_SKIP_DEV_ELECTRON_APP_PREPARE !== '1') { prepareMacDevElectronApp() } diff --git a/src/main/index.ts b/src/main/index.ts index f556f51c6..572263378 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -497,7 +497,7 @@ function openMainWindow(): BrowserWindow { expectedTeardown: getExpectedTeardownScope(webContentsId) }), deferLoad: true, - title: devInstanceIdentity.name, + title: devInstanceIdentity.displayName, getKeybindings: () => keybindings?.getOverrides(), onBeforeReload: ({ ignoreCache, webContentsId }) => { if (mainWindow?.webContents.id === webContentsId) { diff --git a/src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs b/src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs index a9f44c04f..8d968872e 100644 --- a/src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs +++ b/src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs @@ -72,6 +72,7 @@ if (envFile) { badgeLabel: process.env.ORCA_DEV_DOCK_BADGE_LABEL ?? null, dockTitle: process.env.ORCA_DEV_DOCK_TITLE ?? null, stableName: process.env.ORCA_DEV_STABLE_NAME ?? null, + branchAppName: process.env.ORCA_DEV_BRANCH_APP_NAME ?? null, electronExecPath: process.env.ELECTRON_EXEC_PATH ?? null }, null, diff --git a/src/main/startup/dev-instance-identity.test.ts b/src/main/startup/dev-instance-identity.test.ts index 2021dfed2..644827f9b 100644 --- a/src/main/startup/dev-instance-identity.test.ts +++ b/src/main/startup/dev-instance-identity.test.ts @@ -54,4 +54,19 @@ describe('dev-instance-identity', () => { expect(identity.name).toBe('Orca: feature/other') expect(identity.dockBadgeLabel).toBeNull() }) + + it('keeps the app name stable while preserving dev metadata when requested', () => { + const identity = getDevInstanceIdentity(true, { + ORCA_DEV_STABLE_NAME: '1', + ORCA_DEV_REPO_ROOT: '/repo/worktrees/payment-ui', + ORCA_DEV_WORKTREE_NAME: 'payment-ui', + ORCA_DEV_BRANCH: 'feature/billing-shell' + }) + + expect(identity.name).toBe('Orca') + expect(identity.displayName).toBe('Orca: feature/billing-shell') + expect(identity.devLabel).toBe('payment-ui @ feature/billing-shell') + expect(identity.devBranch).toBe('feature/billing-shell') + expect(identity.appUserModelId).toMatch(/^com\.stablyai\.orca\.dev\.[a-f0-9]{10}$/) + }) }) diff --git a/src/main/startup/dev-instance-identity.ts b/src/main/startup/dev-instance-identity.ts index 17ce1ca64..89890d400 100644 --- a/src/main/startup/dev-instance-identity.ts +++ b/src/main/startup/dev-instance-identity.ts @@ -7,6 +7,7 @@ const BASE_APP_USER_MODEL_ID = 'com.stablyai.orca' const MAX_LABEL_LENGTH = 80 export type DevInstanceIdentity = AppIdentity & { + displayName: string appUserModelId: string } @@ -56,6 +57,7 @@ export function getDevInstanceIdentity( devWorktreeName: null, devRepoRoot: null, dockBadgeLabel: null, + displayName: BASE_APP_NAME, appUserModelId: BASE_APP_USER_MODEL_ID } } @@ -66,17 +68,21 @@ export function getDevInstanceIdentity( cleanEnvValue(env.ORCA_DEV_WORKTREE_NAME) ?? cleanEnvValue(path.basename(repoRoot ?? process.cwd())) const devLabel = cleanEnvValue(env.ORCA_DEV_INSTANCE_LABEL) ?? formatLabel(branch, worktreeName) - const dockTitle = + const displayName = cleanEnvValue(env.ORCA_DEV_DOCK_TITLE) ?? `${BASE_APP_NAME}: ${branch ?? devLabel ?? 'dev'}` + // Why: macOS safeStorage authorization is tied to the app identity. Default + // dev runs keep app.setName() stable while window titles remain distinct. + const name = env.ORCA_DEV_STABLE_NAME === '1' ? BASE_APP_NAME : displayName return { - name: dockTitle, + name, isDev: true, devLabel, devBranch: branch, devWorktreeName: worktreeName, devRepoRoot: repoRoot, dockBadgeLabel: null, + displayName, appUserModelId: createDevAppUserModelId(repoRoot ?? devLabel) } } diff --git a/src/main/startup/run-electron-vite-dev.test.ts b/src/main/startup/run-electron-vite-dev.test.ts index f9e93c9c4..051d49808 100644 --- a/src/main/startup/run-electron-vite-dev.test.ts +++ b/src/main/startup/run-electron-vite-dev.test.ts @@ -225,6 +225,7 @@ describe('run-electron-vite-dev', () => { badgeLabel: string | null dockTitle: string stableName: string | null + branchAppName: string | null electronExecPath: string | null } expect(envSnapshot.args).toContain('--remote-debugging-port=9444') @@ -234,7 +235,8 @@ describe('run-electron-vite-dev', () => { expect(envSnapshot.repoRoot).toBe(resolve('.')) expect(envSnapshot.badgeLabel).toBeNull() expect(envSnapshot.dockTitle).toBe('Orca: feature/billing-shell') - expect(envSnapshot.stableName).toBeNull() + expect(envSnapshot.stableName).toBe(process.platform === 'darwin' ? '1' : null) + expect(envSnapshot.branchAppName).toBeNull() expect(envSnapshot.electronExecPath).toBeNull() await stopWrapperAndTrackedPids(wrapper, trackedPids) @@ -291,6 +293,60 @@ describe('run-electron-vite-dev', () => { await stopWrapperAndTrackedPids(wrapper, trackedPids) }) + it('consumes the branch-app-name flag before forwarding args to electron-vite', async () => { + const tempDir = mkdtempSync(join(tmpdir(), 'orca-dev-wrapper-')) + const pidFile = join(tempDir, 'grandchild.pid') + const envFile = join(tempDir, 'env.json') + const wrapperPath = resolve('config/scripts/run-electron-vite-dev.mjs') + const fakeCliPath = resolve('src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs') + + const wrapper = spawn( + process.execPath, + [wrapperPath, '--branch-app-name', '--remote-debugging-port=9446'], + { + cwd: resolve('.'), + env: devWrapperTestEnv({ + ORCA_ELECTRON_VITE_CLI: fakeCliPath, + ORCA_SKIP_DEV_CLI_PREPARE: '1', + ORCA_SKIP_DEV_ELECTRON_APP_PREPARE: '1', + ORCA_SKIP_DEV_WEB_PREPARE: '1', + ORCA_DEV_WRAPPER_TEST_PID_FILE: pidFile, + ORCA_DEV_WRAPPER_TEST_ENV_FILE: envFile, + ORCA_DEV_BRANCH: 'feature/branch-app-name', + ORCA_DEV_WORKTREE_NAME: 'branch-ui' + }), + stdio: 'ignore' + } + ) + + expect(wrapper.pid).toBeTypeOf('number') + processesToCleanUp.add(wrapper.pid!) + + await waitFor(() => { + try { + return readFileSync(envFile, 'utf8').trim().length > 0 + } catch { + return false + } + }) + + const trackedPids = trackPidFile(pidFile) + + const envSnapshot = JSON.parse(readFileSync(envFile, 'utf8')) as { + args: string[] + stableName: string | null + branchAppName: string | null + electronExecPath: string | null + } + expect(envSnapshot.args).not.toContain('--branch-app-name') + expect(envSnapshot.args).toContain('--remote-debugging-port=9446') + expect(envSnapshot.stableName).toBeNull() + expect(envSnapshot.branchAppName).toBe(process.platform === 'darwin' ? '1' : null) + expect(envSnapshot.electronExecPath).toBeNull() + + await stopWrapperAndTrackedPids(wrapper, trackedPids) + }) + it.skipIf(process.platform !== 'darwin')( 'rebuilds the copied Electron app when Chromium resources are missing', async () => { @@ -299,6 +355,7 @@ describe('run-electron-vite-dev', () => { const fakeCliPath = resolve('src/main/startup/__fixtures__/fake-electron-vite-dev-cli.mjs') const baseEnv = devWrapperTestEnv({ ORCA_ELECTRON_VITE_CLI: fakeCliPath, + ORCA_DEV_BRANCH_APP_NAME: '1', ORCA_SKIP_DEV_CLI_PREPARE: '1', ORCA_SKIP_DEV_WEB_PREPARE: '1', ORCA_DEV_BRANCH: 'feature/rebuild-electron-app', @@ -382,6 +439,7 @@ describe('run-electron-vite-dev', () => { cwd: resolve('.'), env: devWrapperTestEnv({ ORCA_ELECTRON_VITE_CLI: fakeCliPath, + ORCA_DEV_BRANCH_APP_NAME: '1', ORCA_SKIP_DEV_CLI_PREPARE: '1', ORCA_SKIP_DEV_WEB_PREPARE: '1', ORCA_DEV_WRAPPER_TEST_PID_FILE: pidFile,