fix: address review findings (#4704)

This commit is contained in:
Jinjing 2026-06-05 10:39:18 -07:00 committed by GitHub
parent fa779ac55f
commit 39e633c136
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 101 additions and 9 deletions

View File

@ -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()
}

View File

@ -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) {

View File

@ -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,

View File

@ -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}$/)
})
})

View File

@ -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)
}
}

View File

@ -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,