diff --git a/config/scripts/locale-ko-key-overrides.json b/config/scripts/locale-ko-key-overrides.json index 071a249e01c..953eb8a21fc 100644 --- a/config/scripts/locale-ko-key-overrides.json +++ b/config/scripts/locale-ko-key-overrides.json @@ -1559,12 +1559,6 @@ "auto.components.onboarding.ThemeStep.94b9dc561d": { "ko": "설정 -> Terminal" }, - "auto.components.pet.PetOverlay.4712d196c6": { - "ko": "@keyframes pet-{{value0}} { from { background-position: {{value1}}px {{value2}}px; } to { background-position: {{value3}}px {{value4}}px; } }" - }, - "auto.components.pet.PetOverlay.de932b0e8f": { - "ko": "@keyframes pet-bob { 0%,100% { transform: translateY(0); } 50% { transform: translateY(-4px); } }" - }, "auto.components.repo.NestedRepoChecklist.ea54c7bf8f": { "ko": "/" }, diff --git a/package.json b/package.json index 52cae65e873..8c0ff488d53 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.4.122-rc.3", + "version": "1.4.122-rc.4", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", diff --git a/src/main/ghostty/index.ts b/src/main/ghostty/index.ts index ac8917154a1..8557193fe9f 100644 --- a/src/main/ghostty/index.ts +++ b/src/main/ghostty/index.ts @@ -5,6 +5,7 @@ import type { Store } from '../persistence' import { findGhosttyConfigPaths } from './discovery' import { parseGhosttyConfig } from './parser' import { mapGhosttyToOrca } from './mapper' +import { resolveGhosttyThemeColors } from './theme-resolution' // Why: defensive upper bound on the Ghostty config size we're willing to read // into memory on the main process. Real configs are a few KB; anything past @@ -61,6 +62,44 @@ function mergeParsedConfig( } } +function usesConditionalThemePair(themeName: string): boolean { + return themeName + .split(',') + .map((part) => part.trim()) + .some((part) => part.startsWith('light:') || part.startsWith('dark:')) +} + +// Why: `theme = ` is how most Ghostty configs pick their colors, so +// dropping it silently loses the entire palette. Resolve the referenced theme +// file and merge its colors as defaults: explicit config keys win, and config +// palette entries are appended after the theme's so per-index overrides apply. +async function applyThemeReference(parsed: Record): Promise { + const rawTheme = parsed['theme'] + if (rawTheme === undefined) { + return [] + } + delete parsed['theme'] + + const themeName = (Array.isArray(rawTheme) ? (rawTheme.at(-1) ?? '') : rawTheme).trim() + if (usesConditionalThemePair(themeName)) { + return ['theme (light:/dark: pairs not supported)'] + } + + const themeColors = await resolveGhosttyThemeColors(themeName) + if (themeColors === null) { + return ['theme (theme file not found)'] + } + + for (const [key, value] of Object.entries(themeColors)) { + if (key === 'palette') { + parsed[key] = [...asArray(value), ...asArray(parsed[key])] + } else if (parsed[key] === undefined) { + parsed[key] = value + } + } + return [] +} + export async function previewGhosttyImport(store: Store): Promise { const configPaths = await findGhosttyConfigPaths() if (configPaths.length === 0) { @@ -93,7 +132,13 @@ export async function previewGhosttyImport(store: Store): Promise = {} diff --git a/src/main/ghostty/mapper-extended.test.ts b/src/main/ghostty/mapper-extended.test.ts index 5c551139120..777110630c1 100644 --- a/src/main/ghostty/mapper-extended.test.ts +++ b/src/main/ghostty/mapper-extended.test.ts @@ -77,6 +77,110 @@ describe('mapGhosttyToOrca — window-padding', () => { expect(result.diff).toEqual({}) expect(result.unsupportedKeys).toEqual(['window-padding-x']) }) + + it('averages a dual-value window-padding-y', () => { + const result = mapGhosttyToOrca({ 'window-padding-y': '10,8' }) + expect(result.diff).toEqual({ terminalPaddingY: 9 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('averages a dual-value window-padding-x with surrounding whitespace', () => { + const result = mapGhosttyToOrca({ 'window-padding-x': '16, 12' }) + expect(result.diff).toEqual({ terminalPaddingX: 14 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('preserves odd-sum dual-value padding as a fractional average', () => { + const result = mapGhosttyToOrca({ 'window-padding-x': '1,2' }) + expect(result.diff).toEqual({ terminalPaddingX: 1.5 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('rejects a dual-value padding with an invalid half', () => { + const result = mapGhosttyToOrca({ 'window-padding-y': '10,wide' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['window-padding-y']) + }) + + it('rejects a dual-value padding with an out-of-range half', () => { + const result = mapGhosttyToOrca({ 'window-padding-y': '10,600' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['window-padding-y']) + }) + + it('rejects paddings with more than two values', () => { + const result = mapGhosttyToOrca({ 'window-padding-y': '10,8,6' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['window-padding-y']) + }) +}) + +describe('mapGhosttyToOrca — adjust-cell-height', () => { + it('maps a percentage to terminalLineHeight', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '35%' }) + expect(result.diff).toEqual({ terminalLineHeight: 1.35 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('rounds the mapped line height to two decimals', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '33%' }) + expect(result.diff).toEqual({ terminalLineHeight: 1.33 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('rounds half-boundary decimal percentages to the nearest hundredth', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '0.5%' }) + expect(result.diff).toEqual({ terminalLineHeight: 1.01 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('rounds higher half-boundary decimal percentages to the nearest hundredth', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '1.5%' }) + expect(result.diff).toEqual({ terminalLineHeight: 1.02 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('rounds high half-boundary decimal percentages without floating-point drift', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '101.5%' }) + expect(result.diff).toEqual({ terminalLineHeight: 2.02 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('accepts the inclusive 1x line-height floor', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '0%' }) + expect(result.diff).toEqual({ terminalLineHeight: 1 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('accepts the inclusive 3x line-height ceiling', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '200%' }) + expect(result.diff).toEqual({ terminalLineHeight: 3 }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('rejects a pixel value (not convertible to a line-height ratio)', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '2' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['adjust-cell-height']) + }) + + it('rejects a negative percentage (below the 1x line-height floor)', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '-10%' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['adjust-cell-height']) + }) + + it('rejects a percentage above the 3x line-height ceiling', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '250%' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['adjust-cell-height']) + }) + + it('rejects values that only fall inside the ceiling after rounding', () => { + const result = mapGhosttyToOrca({ 'adjust-cell-height': '200.4%' }) + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['adjust-cell-height']) + }) }) describe('mapGhosttyToOrca — cursor-text', () => { diff --git a/src/main/ghostty/mapper.ts b/src/main/ghostty/mapper.ts index 69abcbff4d7..7c0588f4c2b 100644 --- a/src/main/ghostty/mapper.ts +++ b/src/main/ghostty/mapper.ts @@ -4,6 +4,7 @@ import type { GhosttyImportPreview } from '../../shared/types' import { HEX_COLOR_RE } from '../../shared/color-validation' +import { parsePaddingValue, parseStrictInt } from './numeric-config-values' const PALETTE_INDEX_MAP: Record = { 0: 'black', @@ -34,18 +35,6 @@ type FieldResult = const normalizeHex = (v: string): string => (v.startsWith('#') ? v : `#${v}`) -// Why: `Number("1e10")` succeeds and passes `Number.isInteger`, so a Ghostty -// config with `window-padding-x = 1e9` would sail through the mapper and land -// an absurd value in the store. Restrict to plain decimal integers. -const STRICT_INT_RE = /^-?\d+$/ -const parseStrictInt = (v: string): number | null => { - if (!STRICT_INT_RE.test(v)) { - return null - } - const num = Number(v) - return Number.isFinite(num) ? num : null -} - type FieldParser = (value: string, rawValue: string | string[]) => FieldResult export function mapGhosttyToOrca( @@ -171,21 +160,38 @@ export function mapGhosttyToOrca( }, 'window-padding-x': (v) => { - const num = parseStrictInt(v) - if (num === null || num < 0 || num > 512) { + const num = parsePaddingValue(v) + if (num === null) { return null } return { key: 'terminalPaddingX', value: num } }, 'window-padding-y': (v) => { - const num = parseStrictInt(v) - if (num === null || num < 0 || num > 512) { + const num = parsePaddingValue(v) + if (num === null) { return null } return { key: 'terminalPaddingY', value: num } }, + // Why: only percentages translate to xterm's line-height ratio; Ghostty's + // pixel form depends on the rendered cell height, which we can't know here. + // The settings UI clamps terminalLineHeight to [1, 3], so reject outside it. + 'adjust-cell-height': (v) => { + const match = /^\+?(\d+(?:\.\d+)?)%$/.exec(v.trim()) + if (!match) { + return null + } + const percent = Number(match[1]) + const rawLineHeight = 1 + percent / 100 + if (rawLineHeight > 3) { + return null + } + const lineHeight = Math.round(100 + percent) / 100 + return { key: 'terminalLineHeight', value: lineHeight } + }, + 'cursor-text': (v) => { if (!HEX_COLOR_RE.test(v)) { return null diff --git a/src/main/ghostty/numeric-config-values.ts b/src/main/ghostty/numeric-config-values.ts new file mode 100644 index 00000000000..f8a7ce7b468 --- /dev/null +++ b/src/main/ghostty/numeric-config-values.ts @@ -0,0 +1,30 @@ +// Why: `Number("1e10")` succeeds and passes `Number.isInteger`, so a Ghostty +// config with `window-padding-x = 1e9` would sail through the mapper and land +// an absurd value in the store. Restrict to plain decimal integers. +const STRICT_INT_RE = /^-?\d+$/ +export const parseStrictInt = (v: string): number | null => { + if (!STRICT_INT_RE.test(v)) { + return null + } + const num = Number(v) + return Number.isFinite(num) ? num : null +} + +// Why: Ghostty accepts "top,bottom" / "left,right" pairs for window paddings, +// but Orca stores a single value per axis — average the pair so the total +// padding along the axis stays the same. +export const parsePaddingValue = (v: string): number | null => { + const parts = v.split(',') + if (parts.length > 2) { + return null + } + const nums: number[] = [] + for (const part of parts) { + const num = parseStrictInt(part.trim()) + if (num === null || num < 0 || num > 512) { + return null + } + nums.push(num) + } + return nums.reduce((sum, num) => sum + num, 0) / nums.length +} diff --git a/src/main/ghostty/theme-import.test.ts b/src/main/ghostty/theme-import.test.ts new file mode 100644 index 00000000000..c2d2ec61ab9 --- /dev/null +++ b/src/main/ghostty/theme-import.test.ts @@ -0,0 +1,171 @@ +import type { Store } from '../persistence' +import type { GlobalSettings } from '../../shared/types' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { statMock, readFileMock } = vi.hoisted(() => ({ + statMock: vi.fn(), + readFileMock: vi.fn() +})) + +vi.mock('fs/promises', () => ({ + stat: statMock, + readFile: readFileMock +})) + +vi.mock('os', () => ({ + platform: vi.fn(() => 'darwin'), + homedir: vi.fn(() => '/Users/alice') +})) + +import { previewGhosttyImport } from './index' + +const originalXdgConfigHome = process.env.XDG_CONFIG_HOME +const originalGhosttyResourcesDir = process.env.GHOSTTY_RESOURCES_DIR + +beforeEach(() => { + delete process.env.XDG_CONFIG_HOME + delete process.env.GHOSTTY_RESOURCES_DIR +}) + +afterEach(() => { + vi.clearAllMocks() + if (originalXdgConfigHome !== undefined) { + process.env.XDG_CONFIG_HOME = originalXdgConfigHome + } else { + delete process.env.XDG_CONFIG_HOME + } + if (originalGhosttyResourcesDir !== undefined) { + process.env.GHOSTTY_RESOURCES_DIR = originalGhosttyResourcesDir + } else { + delete process.env.GHOSTTY_RESOURCES_DIR + } +}) + +function createStore(settings: Record = {}): Store { + return { + getSettings: () => settings as GlobalSettings + } as Store +} + +describe('previewGhosttyImport theme references', () => { + it('resolves a theme reference into color overrides', async () => { + const configPath = '/Users/alice/Library/Application Support/com.mitchellh.ghostty/config' + const themePath = '/Users/alice/.config/ghostty/themes/Tomorrow Night Bright' + statMock.mockImplementation(async (p: string) => { + if (p === configPath || p === themePath) { + return { isFile: () => true, size: 128 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockImplementation(async (p: string) => { + if (p === themePath) { + return 'palette = 1=#d54e53\nbackground = #000000\nforeground = #eaeaea\n' + } + return 'theme = Tomorrow Night Bright\nfont-size = 14\n' + }) + + const result = await previewGhosttyImport(createStore()) + + expect(result.diff).toEqual({ + terminalFontSize: 14, + terminalColorOverrides: { + red: '#d54e53', + background: '#000000', + foreground: '#eaeaea' + } + }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('lets explicit config colors override theme colors', async () => { + const configPath = '/Users/alice/Library/Application Support/com.mitchellh.ghostty/config' + const themePath = '/Users/alice/.config/ghostty/themes/night' + statMock.mockImplementation(async (p: string) => { + if (p === configPath || p === themePath) { + return { isFile: () => true, size: 128 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockImplementation(async (p: string) => { + if (p === themePath) { + return 'palette = 1=#d54e53\npalette = 2=#b9ca4a\nbackground = #000000\n' + } + return 'theme = night\nbackground = #101010\npalette = 1=#ff0000\n' + }) + + const result = await previewGhosttyImport(createStore()) + + expect(result.diff).toEqual({ + terminalColorOverrides: { + // Why: config's palette index 1 and background win; theme keeps index 2. + red: '#ff0000', + green: '#b9ca4a', + background: '#101010' + } + }) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('resolves an absolute theme path without probing theme search dirs', async () => { + const configPath = '/Users/alice/Library/Application Support/com.mitchellh.ghostty/config' + const themePath = '/Users/alice/themes/work' + statMock.mockImplementation(async (p: string) => { + if (p === configPath || p === themePath) { + return { isFile: () => true, size: 128 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockImplementation(async (p: string) => { + if (p === themePath) { + return 'background = #202020\nforeground = #f0f0f0\n' + } + return `theme = ${themePath}\n` + }) + + const result = await previewGhosttyImport(createStore()) + + expect(result.diff).toEqual({ + terminalColorOverrides: { + background: '#202020', + foreground: '#f0f0f0' + } + }) + expect(statMock).toHaveBeenCalledWith(themePath) + expect(statMock).not.toHaveBeenCalledWith( + '/Users/alice/.config/ghostty/themes/Users/alice/themes/work' + ) + expect(result.unsupportedKeys).toEqual([]) + }) + + it('marks an unresolvable theme as unsupported', async () => { + const configPath = '/Users/alice/Library/Application Support/com.mitchellh.ghostty/config' + statMock.mockImplementation(async (p: string) => { + if (p === configPath) { + return { isFile: () => true, size: 128 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockResolvedValue('theme = Missing Theme\n') + + const result = await previewGhosttyImport(createStore()) + + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['theme (theme file not found)']) + }) + + it('marks light:/dark: theme pairs as unsupported', async () => { + const configPath = '/Users/alice/Library/Application Support/com.mitchellh.ghostty/config' + statMock.mockImplementation(async (p: string) => { + if (p === configPath) { + return { isFile: () => true, size: 128 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockResolvedValue('theme = light:Tomorrow,dark:Tomorrow Night\n') + + const result = await previewGhosttyImport(createStore()) + + expect(result.diff).toEqual({}) + expect(result.unsupportedKeys).toEqual(['theme (light:/dark: pairs not supported)']) + }) +}) diff --git a/src/main/ghostty/theme-resolution.test.ts b/src/main/ghostty/theme-resolution.test.ts new file mode 100644 index 00000000000..607e69a39e8 --- /dev/null +++ b/src/main/ghostty/theme-resolution.test.ts @@ -0,0 +1,171 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { statMock, readFileMock } = vi.hoisted(() => ({ + statMock: vi.fn(), + readFileMock: vi.fn() +})) + +vi.mock('fs/promises', () => ({ + stat: statMock, + readFile: readFileMock +})) + +vi.mock('os', () => ({ + platform: vi.fn(() => 'darwin'), + homedir: vi.fn(() => '/Users/alice') +})) + +import { getGhosttyThemeSearchDirs, resolveGhosttyThemeColors } from './theme-resolution' + +const originalXdgConfigHome = process.env.XDG_CONFIG_HOME +const originalGhosttyResourcesDir = process.env.GHOSTTY_RESOURCES_DIR + +beforeEach(() => { + delete process.env.XDG_CONFIG_HOME + delete process.env.GHOSTTY_RESOURCES_DIR +}) + +afterEach(() => { + vi.clearAllMocks() + if (originalXdgConfigHome !== undefined) { + process.env.XDG_CONFIG_HOME = originalXdgConfigHome + } else { + delete process.env.XDG_CONFIG_HOME + } + if (originalGhosttyResourcesDir !== undefined) { + process.env.GHOSTTY_RESOURCES_DIR = originalGhosttyResourcesDir + } else { + delete process.env.GHOSTTY_RESOURCES_DIR + } +}) + +const THEME_FILE = ` +palette = 0=#000000 +palette = 1=#d54e53 +background = #000000 +foreground = #eaeaea +cursor-color = #eaeaea +selection-background = #424242 +selection-foreground = #eaeaea +` + +describe('getGhosttyThemeSearchDirs', () => { + it('probes XDG, then the Ghostty.app resources dir', () => { + delete process.env.XDG_CONFIG_HOME + expect(getGhosttyThemeSearchDirs()).toEqual([ + '/Users/alice/.config/ghostty/themes', + '/Applications/Ghostty.app/Contents/Resources/ghostty/themes' + ]) + }) + + it('honors XDG_CONFIG_HOME for the custom themes dir', () => { + process.env.XDG_CONFIG_HOME = '/xdg' + expect(getGhosttyThemeSearchDirs()[0]).toBe('/xdg/ghostty/themes') + }) + + it('honors GHOSTTY_RESOURCES_DIR for bundled themes', () => { + process.env.GHOSTTY_RESOURCES_DIR = '/opt/ghostty' + expect(getGhosttyThemeSearchDirs()).toEqual([ + '/Users/alice/.config/ghostty/themes', + '/opt/ghostty/themes' + ]) + }) +}) + +describe('resolveGhosttyThemeColors', () => { + it('reads the first matching theme file and returns only color keys', async () => { + statMock.mockImplementation(async (p: string) => { + if (p === '/Users/alice/.config/ghostty/themes/Tomorrow Night Bright') { + return { isFile: () => true, size: THEME_FILE.length } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockResolvedValue(THEME_FILE) + + const result = await resolveGhosttyThemeColors('Tomorrow Night Bright') + expect(result).toEqual({ + palette: ['0=#000000', '1=#d54e53'], + background: '#000000', + foreground: '#eaeaea', + 'cursor-color': '#eaeaea', + 'selection-background': '#424242', + 'selection-foreground': '#eaeaea' + }) + }) + + it('falls through to later dirs when earlier ones miss', async () => { + statMock.mockImplementation(async (p: string) => { + if (p === '/Applications/Ghostty.app/Contents/Resources/ghostty/themes/Tomorrow') { + return { isFile: () => true, size: THEME_FILE.length } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockResolvedValue('background = #1d1f21') + + const result = await resolveGhosttyThemeColors('Tomorrow') + expect(result).toEqual({ background: '#1d1f21' }) + }) + + it('resolves absolute theme paths without searching named theme dirs', async () => { + statMock.mockImplementation(async (p: string) => { + if (p === '/Users/alice/themes/work') { + return { isFile: () => true, size: 64 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockResolvedValue('background = #1d1f21') + + const result = await resolveGhosttyThemeColors('/Users/alice/themes/work') + expect(result).toEqual({ background: '#1d1f21' }) + expect(statMock).toHaveBeenCalledTimes(1) + }) + + it('drops non-color keys a theme file might carry', async () => { + statMock.mockImplementation(async (p: string) => { + if (p === '/Users/alice/.config/ghostty/themes/custom') { + return { isFile: () => true, size: 64 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + readFileMock.mockResolvedValue('background = #101010\nfont-size = 20') + + const result = await resolveGhosttyThemeColors('custom') + expect(result).toEqual({ background: '#101010' }) + }) + + it('returns null when the theme is not found anywhere', async () => { + statMock.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })) + expect(await resolveGhosttyThemeColors('Missing Theme')).toBeNull() + }) + + it('rejects names containing path separators or traversal', async () => { + expect(await resolveGhosttyThemeColors('../etc/passwd')).toBeNull() + expect(await resolveGhosttyThemeColors('a/b')).toBeNull() + expect(await resolveGhosttyThemeColors('a\\b')).toBeNull() + expect(await resolveGhosttyThemeColors('..')).toBeNull() + expect(statMock).not.toHaveBeenCalled() + }) + + it('rejects oversized theme files', async () => { + statMock.mockImplementation(async () => ({ isFile: () => true, size: 10_000_000 })) + expect(await resolveGhosttyThemeColors('huge')).toBeNull() + expect(readFileMock).not.toHaveBeenCalled() + expect(statMock).toHaveBeenCalledTimes(1) + }) + + it('does not fall through when a shadowing theme path is invalid', async () => { + statMock.mockImplementation(async (p: string) => { + if (p === '/Users/alice/.config/ghostty/themes/night') { + return { isFile: () => false, size: 128 } + } + if (p === '/Applications/Ghostty.app/Contents/Resources/ghostty/themes/night') { + return { isFile: () => true, size: 128 } + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) + + expect(await resolveGhosttyThemeColors('night')).toBeNull() + expect(readFileMock).not.toHaveBeenCalled() + expect(statMock).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/ghostty/theme-resolution.ts b/src/main/ghostty/theme-resolution.ts new file mode 100644 index 00000000000..eedb2c1592d --- /dev/null +++ b/src/main/ghostty/theme-resolution.ts @@ -0,0 +1,120 @@ +import { homedir, platform } from 'node:os' +import path from 'node:path' +import { readFile, stat } from 'node:fs/promises' +import { parseGhosttyConfig } from './parser' + +// Why: theme files ship a few dozen short lines; anything larger is not a +// Ghostty theme and should not be read into the main process. +const MAX_THEME_BYTES = 262_144 +type ThemeColors = Record +type ThemeReadResult = + | { status: 'found'; colors: ThemeColors } + | { status: 'missing' } + | { status: 'invalid' } + +// Why: only color-bearing keys may flow from a theme into the import — a theme +// file must not be able to smuggle font/window settings past the user's config. +const THEME_COLOR_KEYS = new Set([ + 'palette', + 'background', + 'foreground', + 'cursor-color', + 'cursor-text', + 'selection-background', + 'selection-foreground', + 'bold-color', + 'split-divider-color' +]) + +function xdgThemeDirs(home: string): string[] { + if (process.env.XDG_CONFIG_HOME) { + return [path.posix.join(process.env.XDG_CONFIG_HOME, 'ghostty', 'themes')] + } + return [path.posix.join(home, '.config', 'ghostty', 'themes')] +} + +function resourceThemeDirs(plat: 'darwin' | 'linux'): string[] { + if (process.env.GHOSTTY_RESOURCES_DIR) { + return [path.posix.join(process.env.GHOSTTY_RESOURCES_DIR, 'themes')] + } + + if (plat === 'darwin') { + return ['/Applications/Ghostty.app/Contents/Resources/ghostty/themes'] + } + return ['/usr/share/ghostty/themes', '/usr/local/share/ghostty/themes'] +} + +// Why: mirrors Ghostty's lookup — user themes shadow the bundled themes shipped +// inside the Ghostty resources directory. +export function getGhosttyThemeSearchDirs(): string[] { + const home = homedir() + const plat = platform() + if (plat !== 'darwin' && plat !== 'linux') { + // Why: Ghostty has no Windows build, so there are no named theme dirs to probe. + return [] + } + return [...xdgThemeDirs(home), ...resourceThemeDirs(plat)] +} + +function isMissingThemeError(err: unknown): boolean { + if (typeof err !== 'object' || err === null || !('code' in err)) { + return false + } + const code = (err as { code?: unknown }).code + return code === 'ENOENT' || code === 'ENOTDIR' +} + +function isRelativeThemeName(name: string): boolean { + return !name.includes('/') && !name.includes('\\') && name !== '..' && name !== '.' +} + +function themeColorsFromContent(content: string): ThemeColors { + const parsed = parseGhosttyConfig(content) + const colors: ThemeColors = {} + for (const [key, value] of Object.entries(parsed)) { + if (THEME_COLOR_KEYS.has(key)) { + colors[key] = value + } + } + return colors +} + +async function readThemeColors(themePath: string): Promise { + let content: string + try { + const info = await stat(themePath) + if (!info.isFile() || info.size > MAX_THEME_BYTES) { + return { status: 'invalid' } + } + content = await readFile(themePath, 'utf-8') + } catch (err) { + return { status: isMissingThemeError(err) ? 'missing' : 'invalid' } + } + + return { status: 'found', colors: themeColorsFromContent(content) } +} + +export async function resolveGhosttyThemeColors(name: string): Promise { + if (path.isAbsolute(name)) { + const result = await readThemeColors(name) + return result.status === 'found' ? result.colors : null + } + + // Why: relative theme names become filenames below; only absolute paths may + // contain separators, matching Ghostty's traversal guard. + if (!isRelativeThemeName(name)) { + return null + } + + for (const dir of getGhosttyThemeSearchDirs()) { + const themePath = path.posix.join(dir, name) + const result = await readThemeColors(themePath) + if (result.status === 'found') { + return result.colors + } + if (result.status === 'invalid') { + return null + } + } + return null +} diff --git a/src/main/github/pr-start-point.test.ts b/src/main/github/pr-start-point.test.ts index 91d8e5ad07e..8783b2a4af4 100644 --- a/src/main/github/pr-start-point.test.ts +++ b/src/main/github/pr-start-point.test.ts @@ -22,12 +22,12 @@ describe('resolveGitHubPrStartPoint', () => { getPullRequestPushTargetMock.mockResolvedValue({ pushTarget: { remoteName: 'pr-contributor-orca', - branchName: 'feat/onboarding-model-choice-782', + branchName: 'fix-issue-6933', remoteUrl: 'git@github.com:contributor/orca.git' } }) const fetchRemoteTrackingRef = vi.fn(async (_remote: string, branch: string) => { - if (branch === 'feat/onboarding-model-choice-782') { + if (branch === 'fix-issue-6933') { throw new Error('fatal: could not find remote ref') } }) @@ -40,28 +40,25 @@ describe('resolveGitHubPrStartPoint', () => { const result = await resolveGitHubPrStartPoint({ repoPath: '/repo-root', - prNumber: 1849, - headRefName: 'feat/onboarding-model-choice-782', + prNumber: 6934, + headRefName: 'fix-issue-6933', baseRefName: 'main', gitExec, fetchRemoteTrackingRef, resolveRemote: async () => 'origin' }) - expect(fetchRemoteTrackingRef).toHaveBeenCalledWith( - 'origin', - 'feat/onboarding-model-choice-782' - ) + expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'fix-issue-6933') expect(fetchRemoteTrackingRef).toHaveBeenCalledWith('origin', 'main') - expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/1849/head']) + expect(gitExec).toHaveBeenCalledWith(['fetch', 'origin', 'refs/pull/6934/head']) expect(result).toEqual({ baseBranch: 'def456', compareBaseRef: 'refs/remotes/origin/main', headSha: 'def456', - branchNameOverride: 'feat/onboarding-model-choice-782', + branchNameOverride: 'fix-issue-6933', pushTarget: { remoteName: 'pr-contributor-orca', - branchName: 'feat/onboarding-model-choice-782', + branchName: 'fix-issue-6933', remoteUrl: 'git@github.com:contributor/orca.git' } }) diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index bc236e9f9bd..d37faa0ac57 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -98,6 +98,7 @@ describe('LocalPtyProvider', () => { let exitCb: ((info: { exitCode: number }) => void) | undefined let origShell: string | undefined let origPowerlevelWizardDisable: string | undefined + let origHistFile: string | undefined let origPlatform: PropertyDescriptor | undefined beforeEach(() => { @@ -105,8 +106,11 @@ describe('LocalPtyProvider', () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) origShell = process.env.SHELL origPowerlevelWizardDisable = process.env.POWERLEVEL9K_DISABLE_CONFIGURATION_WIZARD + origHistFile = process.env.HISTFILE process.env.SHELL = '/bin/zsh' delete process.env.POWERLEVEL9K_DISABLE_CONFIGURATION_WIZARD + // injectHistoryEnv preserves an inherited HISTFILE, so clear it for hermetic history assertions. + delete process.env.HISTFILE existsSyncMock.mockReturnValue(true) statSyncMock.mockReturnValue({ isDirectory: () => true, mode: 0o755 }) @@ -158,6 +162,11 @@ describe('LocalPtyProvider', () => { } else { process.env.POWERLEVEL9K_DISABLE_CONFIGURATION_WIZARD = origPowerlevelWizardDisable } + if (origHistFile === undefined) { + delete process.env.HISTFILE + } else { + process.env.HISTFILE = origHistFile + } }) describe('spawn', () => { diff --git a/src/renderer/src/components/pet/PetOverlay.keyframes.test.tsx b/src/renderer/src/components/pet/PetOverlay.keyframes.test.tsx new file mode 100644 index 00000000000..5e264ff06b6 --- /dev/null +++ b/src/renderer/src/components/pet/PetOverlay.keyframes.test.tsx @@ -0,0 +1,107 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { i18n } from '../../i18n/i18n' + +const storeState = vi.hoisted(() => ({ + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, + retainedAgentsByPaneKey: {}, + petSize: 180 +})) + +vi.mock('../../store', () => ({ + useAppStore: Object.assign( + (selector: (state: typeof storeState) => T): T => { + return selector(storeState) + }, + { + getState: () => storeState + } + ) +})) + +vi.mock('./usePetUrl', () => ({ + usePetUrl: () => ({ + url: 'blob:custom-pet', + ready: true, + sprite: { + frameWidth: 32, + frameHeight: 24, + columns: 6, + rows: 1, + sheetWidth: 192, + sheetHeight: 24, + fps: 6, + defaultAnimation: 'idle', + animations: { + idle: { row: 0, frames: 6 } + } + }, + detected: null + }) +})) + +import { PetOverlay } from './PetOverlay' + +function renderPetOverlay(): { container: HTMLDivElement; root: Root } { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + act(() => { + root.render() + }) + return { container, root } +} + +function installLocalStorage(): void { + const values = new Map() + Object.defineProperty(window, 'localStorage', { + configurable: true, + value: { + clear: () => values.clear(), + getItem: (key: string) => values.get(key) ?? null, + removeItem: (key: string) => values.delete(key), + setItem: (key: string, value: string) => values.set(key, value) + } + }) +} + +describe('PetOverlay sprite keyframes', () => { + let root: Root | null = null + let container: HTMLDivElement | null = null + + beforeEach(async () => { + await i18n.changeLanguage('zh') + installLocalStorage() + }) + + afterEach(async () => { + if (root) { + act(() => root?.unmount()) + } + container?.remove() + root = null + container = null + await i18n.changeLanguage('en') + }) + + it('keeps custom spritesheet keyframes valid in translated locales', () => { + ;({ container, root } = renderPetOverlay()) + + const css = Array.from(container.querySelectorAll('style')) + .map((style) => style.textContent ?? '') + .join('\n') + + expect(css).toContain('@keyframes pet-bob') + expect(css).toContain('transform: translateY(0)') + expect(css).toContain('background-position:') + expect(css).toContain('to { background-position:') + expect(css).not.toContain('变换') + expect(css).not.toContain('背景位置') + expect(css).not.toContain('到 {') + }) +}) diff --git a/src/renderer/src/components/pet/PetOverlay.tsx b/src/renderer/src/components/pet/PetOverlay.tsx index e55004a4788..89fc81dc6da 100644 --- a/src/renderer/src/components/pet/PetOverlay.tsx +++ b/src/renderer/src/components/pet/PetOverlay.tsx @@ -6,7 +6,6 @@ import type { CustomPet } from '../../../../shared/types' import { useAppStore } from '../../store' import { AGENT_STATUS_STALE_AFTER_MS } from '../../../../shared/agent-status-types' import { selectPetAnimationName, type PetAnimationName } from './pet-agent-state' -import { translate } from '@/i18n/i18n' type Sprite = NonNullable @@ -66,15 +65,12 @@ function SpriteFrame({ const startY = -(row * sprite.frameHeight * scale) const endX = -(frames * sprite.frameWidth * scale) const duration = Math.max(0.1, frames / Math.max(0.1, sprite.fps)) + // Why: sprite keyframes are runtime CSS, not user-visible copy; translated + // CSS keywords make the browser discard the animation. + const keyframesCss = `@keyframes pet-${animKeyframesId} { from { background-position: ${startX}px ${startY}px; } to { background-position: ${endX}px ${startY}px; } }` return ( <> - +
- + {sprite ? ( ) } - if (actionState === 'open') { - return ( - - {translate('auto.components.right.sidebar.checks.panel.content.7c1f0a2b11', 'Open')} - - ) - } if (actionState === 'resolved') { return ( diff --git a/src/renderer/src/components/right-sidebar/pr-comment-presentation.ts b/src/renderer/src/components/right-sidebar/pr-comment-presentation.ts index dcd72e755a7..6ce3560f6a1 100644 --- a/src/renderer/src/components/right-sidebar/pr-comment-presentation.ts +++ b/src/renderer/src/components/right-sidebar/pr-comment-presentation.ts @@ -41,7 +41,6 @@ export type PRCommentPresentationClasses = { audienceTab: string audienceTabActive: string sectionTriageLabel: string - statusBadgeOpen: string statusBadgeResolved: string statusBadgeQueued: string commentHeaderPrimary: string @@ -160,8 +159,6 @@ export function getPRCommentPresentationClasses( 'flex h-7 items-center justify-center gap-1 rounded-md px-1.5 text-[11px] font-medium text-muted-foreground transition-colors', audienceTabActive: 'bg-muted text-foreground', sectionTriageLabel: cn('px-3 pt-2', RESOLVED_SECTION_LABEL), - statusBadgeOpen: - 'shrink-0 rounded border border-status-success-border bg-status-success-background px-1.5 py-0.5 text-[10px] font-semibold uppercase tracking-wide text-status-success', statusBadgeResolved: 'shrink-0 rounded border border-border bg-muted px-1.5 py-0.5 text-[10px] font-semibold uppercase tracking-wide text-muted-foreground', statusBadgeQueued: @@ -211,8 +208,6 @@ export function getPRCommentPresentationClasses( 'flex h-7 items-center justify-center gap-1 rounded-md px-1.5 text-[11px] font-medium text-muted-foreground transition-colors', audienceTabActive: 'bg-muted text-foreground shadow-xs', sectionTriageLabel: cn('px-3 pt-1', RESOLVED_SECTION_LABEL), - statusBadgeOpen: - 'shrink-0 rounded border border-status-success-border bg-status-success-background px-1.5 py-0.5 text-[10px] font-semibold uppercase tracking-wide text-status-success', statusBadgeResolved: 'shrink-0 rounded border border-border bg-muted px-1.5 py-0.5 text-[10px] font-semibold uppercase tracking-wide text-muted-foreground', statusBadgeQueued: diff --git a/src/renderer/src/components/settings/AccountsPane.test.tsx b/src/renderer/src/components/settings/AccountsPane.test.tsx index 0bf97d581ea..3e247f88db5 100644 --- a/src/renderer/src/components/settings/AccountsPane.test.tsx +++ b/src/renderer/src/components/settings/AccountsPane.test.tsx @@ -54,8 +54,8 @@ describe('AccountsPane', () => { it('keeps the runtime label inside the localized account copy', () => { const markup = renderPane(getDefaultSettings('/tmp')) - expect(markup).toContain('Showing accounts for This device. New accounts are added there.') - expect(markup).toContain('authenticate with Google for This device. This uses credentials') + expect(markup).toContain('Showing accounts for this device. New accounts are added there.') + expect(markup).toContain('authenticate with Google for this device. This uses credentials') expect(markup).not.toContain('ShowingThis device') expect(markup).not.toContain('forThis device') }) @@ -65,8 +65,8 @@ describe('AccountsPane', () => { const markup = renderPane(getDefaultSettings('/tmp')) - expect(markup).toContain( - 'Mostrando cuentas para este dispositivo. Las nuevas cuentas se agregan allí.' + expect(markup).toMatch( + /Mostrando cuentas para [Ee]ste dispositivo\. Las nuevas cuentas se agregan allí\./ ) expect(markup).not.toContain('This device') }) diff --git a/src/renderer/src/components/settings/AccountsPane.tsx b/src/renderer/src/components/settings/AccountsPane.tsx index 947ec64c228..d50003b5122 100644 --- a/src/renderer/src/components/settings/AccountsPane.tsx +++ b/src/renderer/src/components/settings/AccountsPane.tsx @@ -264,6 +264,11 @@ export function AccountsPane({ wslDistros, wslCapabilitiesLoading ) + // Why: host runtime labels are standalone UI labels; interpolated prose needs sentence casing. + const accountRuntimeSentenceLabel = + accountRuntime.runtime === 'host' && !navigator.userAgent.includes('Windows') + ? `${accountRuntime.label.charAt(0).toLocaleLowerCase()}${accountRuntime.label.slice(1)}` + : accountRuntime.label const [codexAccounts, setCodexAccounts] = useState({ accounts: [], @@ -611,7 +616,7 @@ export function AccountsPane({ {translate( 'auto.components.settings.AccountsPane.c0a52abfc5', 'Showing accounts for {{value0}}. New accounts are added there.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )}

@@ -693,7 +698,7 @@ export function AccountsPane({ {translate( 'auto.components.settings.AccountsPane.e05d0ff737', 'Use your current {{value0}} Claude login.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )} @@ -703,7 +708,7 @@ export function AccountsPane({ {translate( 'auto.components.settings.AccountsPane.3fe7862418', "No managed Claude accounts for {{value0}}. Orca will use that environment's system default Claude login until you add one here.", - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )} ) : ( @@ -863,7 +868,7 @@ export function AccountsPane({ : translate( 'auto.components.settings.AccountsPane.e4a28e8894', 'Codex reported that the {{value0}} login needs a fresh sign-in. Sign in again before starting new Codex sessions.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )} @@ -877,7 +882,7 @@ export function AccountsPane({ {translate( 'auto.components.settings.AccountsPane.c0a52abfc5', 'Showing accounts for {{value0}}. New accounts are added there.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )}

@@ -964,12 +969,12 @@ export function AccountsPane({ ? translate( 'auto.components.settings.AccountsPane.fd62f37c24', 'Codex reported this {{value0}} login is out of date.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } ) : translate( 'auto.components.settings.AccountsPane.fcc4093fc1', 'Use your current {{value0}} Codex login.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )} @@ -979,7 +984,7 @@ export function AccountsPane({ {translate( 'auto.components.settings.AccountsPane.b4c9450319', "No managed Codex accounts for {{value0}}. Orca will use that environment's system default Codex login until you add one here.", - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )} ) : ( @@ -1177,7 +1182,7 @@ export function AccountsPane({ {translate( 'auto.components.settings.AccountsPane.c2aee76420', 'Extracts OAuth credentials from your local Gemini CLI installation to authenticate with Google for {{value0}}. This uses credentials issued to the Gemini CLI app, not Orca. May break if Google updates the CLI. Use at your own risk.', - { value0: accountRuntime.label } + { value0: accountRuntimeSentenceLabel } )}

diff --git a/src/renderer/src/components/settings/BrowserProfileRow.tsx b/src/renderer/src/components/settings/BrowserProfileRow.tsx index 6c3c06dd3df..0d1c377c45f 100644 --- a/src/renderer/src/components/settings/BrowserProfileRow.tsx +++ b/src/renderer/src/components/settings/BrowserProfileRow.tsx @@ -174,8 +174,11 @@ export function BrowserProfileRow({ browser.profiles.length > 1 ? ( - {translate('auto.components.settings.BrowserProfileRow.7df818977e', 'From')} - {browser.label} + {translate( + 'auto.components.settings.BrowserProfileRow.c5a273a809', + 'From {{value0}}', + { value0: browser.label } + )} @@ -197,8 +200,11 @@ export function BrowserProfileRow({ key={browser.family} onSelect={() => void handleImportFromBrowser(browser.family)} > - {translate('auto.components.settings.BrowserProfileRow.7df818977e', 'From')} - {browser.label} + {translate( + 'auto.components.settings.BrowserProfileRow.c5a273a809', + 'From {{value0}}', + { value0: browser.label } + )} ) )} diff --git a/src/renderer/src/components/settings/BrowserUseCookieImportStep.tsx b/src/renderer/src/components/settings/BrowserUseCookieImportStep.tsx index 5104ace581f..f73668fa908 100644 --- a/src/renderer/src/components/settings/BrowserUseCookieImportStep.tsx +++ b/src/renderer/src/components/settings/BrowserUseCookieImportStep.tsx @@ -116,9 +116,9 @@ export function BrowserUseCookieImportStep({

{translate( 'auto.components.settings.BrowserUsePane.112f70adc4', - 'Last imported from' + 'Last imported from {{value0}}', + { value0: sourceLabel } )} - {sourceLabel}

) : null} {onConfigureMoreBrowsers ? ( @@ -163,8 +163,11 @@ export function BrowserUseCookieImportStep({ browser.profiles.length > 1 ? ( - {translate('auto.components.settings.BrowserUsePane.e44c5d681e', 'From')} - {browser.label} + {translate( + 'auto.components.settings.BrowserUsePane.5301857d88', + 'From {{value0}}', + { value0: browser.label } + )} @@ -186,8 +189,11 @@ export function BrowserUseCookieImportStep({ key={browser.family} onSelect={() => void handleImportFromBrowser(browser.family)} > - {translate('auto.components.settings.BrowserUsePane.e44c5d681e', 'From')} - {browser.label} + {translate( + 'auto.components.settings.BrowserUsePane.5301857d88', + 'From {{value0}}', + { value0: browser.label } + )} ) )} diff --git a/src/renderer/src/components/settings/GhosttyImportModal.test.ts b/src/renderer/src/components/settings/GhosttyImportModal.test.ts index 6806f9a2151..67590e710f2 100644 --- a/src/renderer/src/components/settings/GhosttyImportModal.test.ts +++ b/src/renderer/src/components/settings/GhosttyImportModal.test.ts @@ -99,6 +99,25 @@ describe('GhosttyImportModal', () => { expect(onOpenChange).toHaveBeenCalledWith(false) }) + it('labels Ghostty line-height imports with the settings display name', () => { + const element = GhosttyImportModal({ + open: true, + onOpenChange: () => {}, + preview: { + found: true, + configPath: '/Users/alice/.config/ghostty/config', + diff: { terminalLineHeight: 1.35 }, + unsupportedKeys: [] + }, + loading: false, + onApply: () => {}, + applied: false + }) + + expect(containsText(element, 'Line Height')).toBe(true) + expect(containsText(element, 'terminalLineHeight')).toBe(false) + }) + it('renders success summary with done button when applied', () => { const onOpenChange = vi.fn() const onApply = vi.fn() diff --git a/src/renderer/src/components/settings/browser-cookie-import-label.test.ts b/src/renderer/src/components/settings/browser-cookie-import-label.test.ts new file mode 100644 index 00000000000..108a2e4c8d2 --- /dev/null +++ b/src/renderer/src/components/settings/browser-cookie-import-label.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, it } from 'vitest' + +import en from '@/i18n/locales/en.json' +import { i18n, translate } from '@/i18n/i18n' + +// Keys for the "From " import-cookies menu entries. The label is built +// by interpolating the browser name into a single localized string; a regression +// where the name was concatenated directly onto "From" dropped the space and +// rendered e.g. "FromGoogle Chrome". +const IMPORT_FROM_KEYS = [ + 'auto.components.settings.BrowserProfileRow.c5a273a809', + 'auto.components.settings.BrowserUsePane.5301857d88' +] +const LAST_IMPORTED_FROM_KEY = 'auto.components.settings.BrowserUsePane.112f70adc4' + +function catalogEntry(key: string): unknown { + return key.split('.').reduce((node, part) => { + if (typeof node === 'object' && node !== null) { + return (node as Record)[part] + } + return undefined + }, en) +} + +describe('import-cookies "From " label', () => { + beforeEach(async () => { + await i18n.changeLanguage('en') + }) + + it('keeps a space between "From" and the browser name', () => { + for (const key of IMPORT_FROM_KEYS) { + expect(translate(key, 'From {{value0}}', { value0: 'Google Chrome' })).toBe( + 'From Google Chrome' + ) + expect(translate(key, 'From {{value0}}', { value0: 'Safari' })).toBe('From Safari') + } + }) + + it('stores the catalog entry with an interpolation placeholder', () => { + for (const key of IMPORT_FROM_KEYS) { + expect(catalogEntry(key)).toBe('From {{value0}}') + } + expect(catalogEntry(LAST_IMPORTED_FROM_KEY)).toBe('Last imported from {{value0}}') + }) + + it('keeps a space in the last-imported source label', () => { + expect( + translate(LAST_IMPORTED_FROM_KEY, 'Last imported from {{value0}}', { + value0: 'Google Chrome' + }) + ).toBe('Last imported from Google Chrome') + }) +}) diff --git a/src/renderer/src/components/settings/setting-labels.ts b/src/renderer/src/components/settings/setting-labels.ts index cdc459d9790..0f108fadeae 100644 --- a/src/renderer/src/components/settings/setting-labels.ts +++ b/src/renderer/src/components/settings/setting-labels.ts @@ -4,6 +4,7 @@ export const SETTING_LABELS: Partial> = { terminalFontSize: 'Font Size', terminalFontFamily: 'Font Family', terminalFontWeight: 'Font Weight', + terminalLineHeight: 'Line Height', terminalScrollSensitivity: 'Normal Scroll Speed', terminalFastScrollSensitivity: 'Fast Scroll Speed', terminalTuiScrollSensitivity: 'TUI Scroll Speed', diff --git a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx index 217d2fd6735..5163a839ebf 100644 --- a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx +++ b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx @@ -11,8 +11,11 @@ import { import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import { parseGitHubIssueOrPRNumber } from '@/lib/github-links' -import { buildWorktreeMetaUpdates, type WorktreeMetaSavedPayload } from './worktree-meta-updates' +import { + buildWorktreeMetaUpdates, + parseGitHubWorkItemNumberForMetaField, + type WorktreeMetaSavedPayload +} from './worktree-meta-updates' import { useWorktreeIssueLink } from './use-worktree-issue-link' import { getScreenSubmitShortcutLabel, isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut' import { ExternalLink, LoaderCircle } from 'lucide-react' @@ -95,8 +98,10 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { } const trimmedIssue = issueInput.trim() const trimmedPR = prInput.trim() - const issueValid = trimmedIssue === '' || parseGitHubIssueOrPRNumber(trimmedIssue) !== null - const prValid = trimmedPR === '' || parseGitHubIssueOrPRNumber(trimmedPR) !== null + const issueValid = + trimmedIssue === '' || parseGitHubWorkItemNumberForMetaField(trimmedIssue, 'issue') !== null + const prValid = + trimmedPR === '' || parseGitHubWorkItemNumberForMetaField(trimmedPR, 'pr') !== null return issueValid && prValid }, [worktreeId, issueInput, prInput]) diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts new file mode 100644 index 00000000000..a53eeae4163 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' +import { buildWorktreeMetaUpdates } from './worktree-meta-updates' + +describe('buildWorktreeMetaUpdates', () => { + it('rejects issue URLs in the PR input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: '', + prInput: 'https://github.com/stablyai/orca/issues/6933', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedIssue: null + }) + }) + + it('accepts PR URLs in the PR input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: '', + prInput: 'https://github.com/stablyai/orca/pull/6934', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedIssue: null, + linkedPR: 6934 + }) + }) + + it('accepts issue URLs in the issue input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: 'https://github.com/stablyai/orca/issues/6933', + prInput: '', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedIssue: 6933, + linkedPR: null + }) + }) + + it('rejects PR URLs in the issue input', () => { + expect( + buildWorktreeMetaUpdates({ + displayNameInput: 'Workspace', + currentDisplayName: 'Workspace', + issueInput: 'https://github.com/stablyai/orca/pull/6934', + prInput: '', + commentInput: '' + }) + ).toEqual({ + comment: '', + linkedPR: null + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-meta-updates.ts b/src/renderer/src/components/sidebar/worktree-meta-updates.ts index 7f6f5d70935..26769c65061 100644 --- a/src/renderer/src/components/sidebar/worktree-meta-updates.ts +++ b/src/renderer/src/components/sidebar/worktree-meta-updates.ts @@ -16,6 +16,20 @@ export function parseExplicitGitHubIssueUrl(input: string): string | null { return trimmed } +export function parseGitHubWorkItemNumberForMetaField( + input: string, + expectedType: 'issue' | 'pr' +): number | null { + const link = parseGitHubIssueOrPRLink(input) + if (link) { + // Why: issue and PR numbers live in separate GitHub namespaces for refs; + // a URL path mismatch must not silently link the other field. + return link.type === expectedType ? link.number : null + } + + return parseGitHubIssueOrPRNumber(input) +} + /** Pure save-payload builder for the worktree meta dialog: empty inputs clear * the link (null), unparseable inputs leave it untouched (omitted). */ export function buildWorktreeMetaUpdates(args: { @@ -26,11 +40,11 @@ export function buildWorktreeMetaUpdates(args: { commentInput: string }): Partial { const trimmedIssue = args.issueInput.trim() - const linkedIssueNumber = parseGitHubIssueOrPRNumber(trimmedIssue) + const linkedIssueNumber = parseGitHubWorkItemNumberForMetaField(trimmedIssue, 'issue') const finalLinkedIssue = trimmedIssue === '' ? null : linkedIssueNumber !== null ? linkedIssueNumber : undefined const trimmedPR = args.prInput.trim() - const linkedPRNumber = parseGitHubIssueOrPRNumber(trimmedPR) + const linkedPRNumber = parseGitHubWorkItemNumberForMetaField(trimmedPR, 'pr') const finalLinkedPR = trimmedPR === '' ? null : linkedPRNumber !== null ? linkedPRNumber : undefined diff --git a/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts b/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts index f45f7ee3c17..c43a051c8ab 100644 --- a/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts +++ b/src/renderer/src/hooks/useComposerState-host-context-boundaries.test.ts @@ -209,6 +209,8 @@ describe('useComposerState host-context boundaries', () => { 'const applyLinkedWorkItem = useCallback' ) expect(directLookup).toContain('sourceContext: selectedRepoGitHubSourceContext') + expect(directLookup).toContain('lookupGitHubWorkItemByOwnerRepoForSource') + expect(directLookup).toContain('type: normalizedLinkQuery.directLink.type') const submitLookup = sourceBetween( HOOK_SOURCE, @@ -236,7 +238,11 @@ describe('useComposerState host-context boundaries', () => { 'const intent = getSmartGitHubSubmitIntent(name)' ) expect(selectedPrSubmitLookup).toContain('smartGitHubPrStartPointSelectionRef.current') - expect(selectedPrSubmitLookup).toContain("linkedWorkItem.type === 'pr'") + expect(selectedPrSubmitLookup).toContain("linkedWorkItemIdentity?.type === 'pr'") + expect(selectedPrSubmitLookup).toContain("startPointIdentity?.type === 'pr'") + expect(selectedPrSubmitLookup).toContain( + 'startPointIdentity.number === linkedWorkItemIdentity.number' + ) expect(selectedPrSubmitLookup).toContain('resolveGitHubPrStartPointForRepo') expect(selectedPrSubmitLookup.indexOf('resolveGitHubPrStartPointForRepo')).toBeLessThan( selectedPrSubmitLookup.indexOf("return { kind: 'none' }") @@ -421,8 +427,12 @@ describe('useComposerState host-context boundaries', () => { ) expect(projectGroupSmartHandlers).toContain('setLinkedGitLabIssue(null)') expect(projectGroupSmartHandlers).toContain('setLinkedGitLabMR(null)') - expect(projectGroupSmartHandlers).toContain("setLinkedIssue('')") - expect(projectGroupSmartHandlers).toContain('setLinkedPR(null)') + expect(projectGroupSmartHandlers).toContain( + "setLinkedIssue(identity.type === 'issue' ? String(identity.number) : '')" + ) + expect(projectGroupSmartHandlers).toContain( + "setLinkedPR(identity.type === 'pr' ? identity.number : null)" + ) }) it('disables repo-backed folder smart lookup when a folder target has no source repos', () => { diff --git a/src/renderer/src/hooks/useComposerState.ts b/src/renderer/src/hooks/useComposerState.ts index 202563742eb..dc6de85382b 100644 --- a/src/renderer/src/hooks/useComposerState.ts +++ b/src/renderer/src/hooks/useComposerState.ts @@ -95,6 +95,10 @@ import { lookupGitHubWorkItemByOwnerRepoForSource, lookupGitHubWorkItemForSource } from '@/lib/github-work-item-source-lookup' +import { + resolveGitHubWorkItemIdentity, + type GitHubWorkItemIdentity +} from '@/lib/github-work-item-identity' import { resolveGitHubPrStartPointForRepo } from '@/lib/github-pr-start-point' import { isWorkItemLookupText } from '@/lib/work-item-lookup-text' import { @@ -475,6 +479,38 @@ function getLinkedWorkItemSeedName(item: LinkedWorkItemSummary | null | undefine return getLinkedWorkItemWorkspaceName(item)?.seedName ?? getLinkedWorkItemSuggestedName(item) } +function getGitHubLinkedWorkItemIdentity( + item: LinkedWorkItemSummary | null | undefined +): GitHubWorkItemIdentity | null { + if ( + !item || + getLinkedWorkItemProvider(item) !== 'github' || + (item.type !== 'issue' && item.type !== 'pr') + ) { + return null + } + + return resolveGitHubWorkItemIdentity({ + type: item.type, + number: item.number, + url: item.url + }) +} + +function normalizeGitHubLinkedWorkItem( + item: LinkedWorkItemSummary | null | undefined +): LinkedWorkItemSummary | null { + if (!item) { + return null + } + const identity = getGitHubLinkedWorkItemIdentity(item) + if (!identity || (identity.type === item.type && identity.number === item.number)) { + return item + } + + return { ...item, type: identity.type, number: identity.number } +} + export function getInitialAutoManagedWorkspaceName({ draftName, draftLinkedWorkItem, @@ -905,10 +941,16 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const [attachmentPaths, setAttachmentPaths] = useState( persistDraft ? (newWorkspaceDraft?.attachments ?? []) : [] ) + const initialLinkedWorkItemSeed = normalizeGitHubLinkedWorkItem(initialLinkedWorkItem) + const draftLinkedWorkItemSeed = persistDraft + ? normalizeGitHubLinkedWorkItem(newWorkspaceDraft?.linkedWorkItem) + : null + const linkedWorkItemSeed = persistDraft + ? (draftLinkedWorkItemSeed ?? initialLinkedWorkItemSeed) + : initialLinkedWorkItemSeed + const linkedWorkItemSeedIdentity = getGitHubLinkedWorkItemIdentity(linkedWorkItemSeed) const [linkedWorkItem, setLinkedWorkItem] = useState( - persistDraft - ? (newWorkspaceDraft?.linkedWorkItem ?? initialLinkedWorkItem) - : initialLinkedWorkItem + () => linkedWorkItemSeed ) const taskSourceContext = useMemo(() => { if ( @@ -982,6 +1024,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS }) }, [projects, selectedRepo, selectedRepoIsGit, selectedWorkspaceTarget, taskSourceContext]) const [linkedIssue, setLinkedIssue] = useState(() => { + if (linkedWorkItemSeedIdentity?.type === 'issue') { + return String(linkedWorkItemSeedIdentity.number) + } if (persistDraft && newWorkspaceDraft?.linkedIssue) { return newWorkspaceDraft.linkedIssue } @@ -994,6 +1039,12 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS return '' }) const [linkedPR, setLinkedPR] = useState(() => { + if (linkedWorkItemSeedIdentity?.type === 'pr') { + return linkedWorkItemSeedIdentity.number + } + if (linkedWorkItemSeedIdentity?.type === 'issue') { + return null + } if (persistDraft && newWorkspaceDraft?.linkedPR !== undefined) { return newWorkspaceDraft.linkedPR } @@ -1134,9 +1185,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const lastAutoNameRef = useRef( getInitialAutoManagedWorkspaceName({ draftName: persistDraft ? newWorkspaceDraft?.name : null, - draftLinkedWorkItem: persistDraft ? newWorkspaceDraft?.linkedWorkItem : null, + draftLinkedWorkItem: persistDraft ? draftLinkedWorkItemSeed : null, initialName, - initialLinkedWorkItem + initialLinkedWorkItem: initialLinkedWorkItemSeed }) ) const nameRef = useRef(name) @@ -1922,16 +1973,28 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS let cancelled = false setLinkDirectLoading(true) // Why: Superset lets users paste a full GitHub URL or type a raw issue/PR - // number and still get a concrete selectable result. Orca mirrors that by - // resolving direct lookups against the selected repo instead of requiring a - // text match in the recent-items list. + // number and still get a concrete selectable result. Full URLs carry + // issue-vs-PR intent, so preserve the URL route instead of probing by + // number only. const lookupRepoId = selectedRepo.id - void lookupGitHubWorkItemForSource({ - repoPath: selectedRepo.path, - repoId: selectedRepo.id, - sourceContext: selectedRepoGitHubSourceContext, - number: normalizedLinkQuery.directNumber - }) + const lookup = + normalizedLinkQuery.directLink !== undefined + ? lookupGitHubWorkItemByOwnerRepoForSource({ + repoPath: selectedRepo.path, + repoId: selectedRepo.id, + sourceContext: selectedRepoGitHubSourceContext, + owner: normalizedLinkQuery.directLink.slug.owner, + repo: normalizedLinkQuery.directLink.slug.repo, + number: normalizedLinkQuery.directLink.number, + type: normalizedLinkQuery.directLink.type + }) + : lookupGitHubWorkItemForSource({ + repoPath: selectedRepo.path, + repoId: selectedRepo.id, + sourceContext: selectedRepoGitHubSourceContext, + number: normalizedLinkQuery.directNumber + }) + void lookup .then((item) => { if (!cancelled) { setLinkDirectItem( @@ -1954,6 +2017,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS cancelled = true } }, [ + normalizedLinkQuery.directLink, linkPopoverOpen, normalizedLinkQuery.directNumber, selectedRepo, @@ -1963,24 +2027,31 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const applyLinkedWorkItem = useCallback( (item: GitHubWorkItem, options: { preserveBranchNameOverride?: boolean } = {}): void => { - if (item.type === 'issue') { - setLinkedIssue(String(item.number)) + const identity = resolveGitHubWorkItemIdentity(item) + const normalizedItem: GitHubWorkItem = { + ...item, + type: identity.type, + number: identity.number + } + if (identity.type === 'issue') { + setLinkedIssue(String(identity.number)) setLinkedPR(null) } else { setLinkedIssue('') - setLinkedPR(item.number) + setLinkedPR(identity.number) } setLinkedGitLabIssue(null) setLinkedGitLabMR(null) setLinkedWorkItem({ - type: item.type, + type: identity.type, provider: 'github', - number: item.number, + number: identity.number, title: item.title, url: item.url }) const suggestedName = - getLinkedWorkItemWorkspaceName(item)?.seedName ?? getLinkedWorkItemSuggestedName(item) + getLinkedWorkItemWorkspaceName(normalizedItem)?.seedName ?? + getLinkedWorkItemSuggestedName(normalizedItem) // Why: a pasted URL/#123 in the field is the lookup query that found // this item, not a deliberate name — replace it with the title-derived // name or it silently becomes a slugified-URL workspace name. @@ -2002,20 +2073,25 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS useCallback(async (): Promise => { if (linkedWorkItem) { const startPointSelection = smartGitHubPrStartPointSelectionRef.current + const linkedWorkItemIdentity = getGitHubLinkedWorkItemIdentity(linkedWorkItem) + const startPointIdentity = startPointSelection + ? resolveGitHubWorkItemIdentity(startPointSelection.item) + : null if ( !isProjectGroupTarget && - linkedWorkItem.type === 'pr' && + linkedWorkItemIdentity?.type === 'pr' && + startPointIdentity?.type === 'pr' && getLinkedWorkItemProvider(linkedWorkItem) === 'github' && selectedRepo && selectedRepoIsGit && startPointSelection?.repoId === selectedRepo.id && - startPointSelection.item.number === linkedWorkItem.number + startPointIdentity.number === linkedWorkItemIdentity.number ) { const selectedPrStartPoint = startPointSelection.resolved ?? (await resolveGitHubPrStartPointForRepo({ repoId: selectedRepo.id, - prNumber: startPointSelection.item.number, + prNumber: startPointIdentity.number, settings: getSettingsForRepoRuntimeOwner( { repos: [selectedRepo], settings }, selectedRepo.id @@ -2102,11 +2178,12 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS throw new Error('Could not resolve the GitHub item before creating the workspace.') } + const itemIdentity = resolveGitHubWorkItemIdentity(item) const prStartPoint = - !isProjectGroupTarget && item.type === 'pr' && selectedRepo && selectedRepoIsGit + !isProjectGroupTarget && itemIdentity.type === 'pr' && selectedRepo && selectedRepoIsGit ? await resolveGitHubPrStartPointForRepo({ repoId: selectedRepo.id, - prNumber: item.number, + prNumber: itemIdentity.number, settings: getSettingsForRepoRuntimeOwner( { repos: [selectedRepo], settings }, selectedRepo.id @@ -2757,8 +2834,9 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS // worktree's comment should surface (`orca worktree current`, sidebar). // Prefill the note if it's empty or still equal to a prior auto-fill, so // we don't overwrite anything the user has typed. - if (item.type === 'pr') { - const suggestedNote = `PR #${item.number} — ${item.title}` + const identity = resolveGitHubWorkItemIdentity(item) + if (identity.type === 'pr') { + const suggestedNote = `PR #${identity.number} — ${item.title}` const currentNote = noteRef.current if (!currentNote.trim() || currentNote === lastAutoNoteRef.current) { setNote(suggestedNote) @@ -2800,10 +2878,16 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS const handleSmartGitHubItemSelect = useCallback( (item: GitHubWorkItem): void => { + const identity = resolveGitHubWorkItemIdentity(item) + const normalizedItem: GitHubWorkItem = { + ...item, + type: identity.type, + number: identity.number + } if (isProjectGroupTarget) { - const linkedItem = toGitHubLinkedWorkItem(item) - setLinkedIssue(String(item.number)) - setLinkedPR(item.type === 'pr' ? item.number : null) + const linkedItem = toGitHubLinkedWorkItem(normalizedItem) + setLinkedIssue(identity.type === 'issue' ? String(identity.number) : '') + setLinkedPR(identity.type === 'pr' ? identity.number : null) setLinkedGitLabIssue(null) setLinkedGitLabMR(null) setLinkedWorkItem(linkedItem) @@ -2826,8 +2910,8 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS // selected run host. Resolve git refs against the run repo; keep item // metadata/source context separate for provider identity. const runRepo = selectedRepo ?? eligibleRepos.find((repo) => repo.id === item.repoId) - applyLinkedWorkItem(item) - if (item.type !== 'pr' || !runRepo) { + applyLinkedWorkItem(normalizedItem) + if (identity.type !== 'pr' || !runRepo) { setBaseBranch(undefined) setCompareBaseRef(undefined) setPushTarget(undefined) @@ -2838,7 +2922,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS setPushTarget(undefined) const startPointSelection: SmartGitHubPrStartPointSelection = { repoId: runRepo.id, - item + item: normalizedItem } smartGitHubPrStartPointSelectionRef.current = startPointSelection const itemRepoSettings = getSettingsForRepoRuntimeOwner( @@ -2847,12 +2931,12 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS ) const resolvePrBase = resolveGitHubPrStartPointForRepo({ repoId: runRepo.id, - prNumber: item.number, + prNumber: identity.number, settings: itemRepoSettings, - ...(item.branchName ? { headRefName: item.branchName } : {}), - ...(item.baseRefName ? { baseRefName: item.baseRefName } : {}), - ...(item.isCrossRepository !== undefined - ? { isCrossRepository: item.isCrossRepository } + ...(normalizedItem.branchName ? { headRefName: normalizedItem.branchName } : {}), + ...(normalizedItem.baseRefName ? { baseRefName: normalizedItem.baseRefName } : {}), + ...(normalizedItem.isCrossRepository !== undefined + ? { isCrossRepository: normalizedItem.isCrossRepository } : {}) }) void resolvePrBase @@ -2863,7 +2947,7 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS startPointSelection.resolved = result handleBaseBranchPrSelect( result.baseBranch, - item, + normalizedItem, result.pushTarget, result.branchNameOverride, result.compareBaseRef diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 20f44425297..ca2bbd195a2 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -4882,7 +4882,8 @@ "d420c43729": "Imported {{value0}} cookies from {{value1}}{{value2}} into {{value3}}.", "b4c167764d": "Imported {{value0}} cookies from file into {{value1}}.", "a3f8c2d1e0b4": "Imported {{value0}} cookies from {{value1}} ({{value2}}) into {{value3}}.", - "b4e9d3f2a1c5": "Imported {{value0}} cookies from {{value1}} into {{value2}}." + "b4e9d3f2a1c5": "Imported {{value0}} cookies from {{value1}} into {{value2}}.", + "c5a273a809": "From {{value0}}" }, "BrowserUseComputerUseNotice": { "15b5e680ba": "Open Computer Use", @@ -4906,7 +4907,7 @@ "be6df68384": "From File…", "e44c5d681e": "From", "67d9a53f47": "Manage profiles for separate logins", - "112f70adc4": "Last imported from", + "112f70adc4": "Last imported from {{value0}}", "72d4815523": "Bring your existing logins into Orca so agents can reach authenticated pages. Imports into the default profile.", "2eb906706c": "Import Browser Cookies", "af8c83ed61": "Import cookies from Chrome, Edge, or other browsers so agents can reuse your logins.", @@ -4928,7 +4929,8 @@ "ad8cb0ee22": "Fix PATH", "0289434ed6": "Enabled", "8b3054dac7": "Registering...", - "8f2675c2f3": "Imported {{value0}} cookies from file." + "8f2675c2f3": "Imported {{value0}} cookies from file.", + "5301857d88": "From {{value0}}" }, "BrowserUseSkillStep": { "0871b6998d": "Enables agents to navigate and verify pages in Orca's browser.", @@ -9793,10 +9795,6 @@ "a84d5677ff": "OpenCode", "2528586aa7": "Claudino" } - }, - "PetOverlay": { - "de932b0e8f": "@keyframes pet-bob { 0%,100% { transform: translateY(0); } 50% { transform: translateY(-4px); } }", - "4712d196c6": "@keyframes pet-{{value0}} { from { background-position: {{value1}}px {{value2}}px; } to { background-position: {{value3}}px {{value4}}px; } }" } }, "onboarding": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 2dc3780a125..9d4cdf90b27 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -4882,7 +4882,8 @@ "d420c43729": "Se importaron {{value0}} cookies de {{value1}}{{value2}} a {{value3}}.", "b4c167764d": "Se importaron {{value0}} cookies desde un archivo a {{value1}}.", "a3f8c2d1e0b4": "Se importaron {{value0}} cookies de {{value1}} ({{value2}}) a {{value3}}.", - "b4e9d3f2a1c5": "Se importaron {{value0}} cookies de {{value1}} a {{value2}}." + "b4e9d3f2a1c5": "Se importaron {{value0}} cookies de {{value1}} a {{value2}}.", + "c5a273a809": "Desde {{value0}}" }, "BrowserUseComputerUseNotice": { "15b5e680ba": "Abrir Computer Use", @@ -4906,7 +4907,7 @@ "be6df68384": "Desde archivo…", "e44c5d681e": "Desde ", "67d9a53f47": "Gestionar perfiles para logins separados", - "112f70adc4": "Última importación desde ", + "112f70adc4": "Última importación desde {{value0}}", "72d4815523": "Trae tus logins existentes a Orca para que los agents puedan acceder a páginas autenticadas. Importa al perfil predeterminado.", "2eb906706c": "Importar cookies del navegador", "af8c83ed61": "Importa cookies de Chrome, Edge u otros navegadores para que los agents puedan reutilizar tus logins.", @@ -4928,7 +4929,8 @@ "ad8cb0ee22": "Corregir PATH", "0289434ed6": "Activado", "8b3054dac7": "Registrando...", - "8f2675c2f3": "Se importaron {{value0}} cookies desde un archivo." + "8f2675c2f3": "Se importaron {{value0}} cookies desde un archivo.", + "5301857d88": "Desde {{value0}}" }, "BrowserUseSkillStep": { "0871b6998d": "Permite a los agents navegar y verificar páginas en el navegador de Orca.", @@ -9793,10 +9795,6 @@ "a84d5677ff": "OpenCode", "2528586aa7": "Claudino" } - }, - "PetOverlay": { - "de932b0e8f": "@keyframes pet-bob { 0%,100% { transform: translateY(0); } 50% { transform: translateY(-4px); } }", - "4712d196c6": "@keyframes pet-{{value0}} { from { background-position: {{value1}}px {{value2}}px; } to { background-position: {{value3}}px {{value4}}px; } }" } }, "onboarding": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 28a1044c682..775f4b584b3 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -4867,7 +4867,8 @@ "d420c43729": "{{value0}} Cookie を {{value1}}{{value2}} から {{value3}} にインポートしました。", "b4c167764d": "ファイルから {{value0}} 個の Cookie を {{value1}} にインポートしました。", "a3f8c2d1e0b4": "{{value1}} ({{value2}}) から {{value0}} 件の Cookie を {{value3}} にインポートしました。", - "b4e9d3f2a1c5": "{{value1}} から {{value0}} 件の Cookie を {{value2}} にインポートしました。" + "b4e9d3f2a1c5": "{{value1}} から {{value0}} 件の Cookie を {{value2}} にインポートしました。", + "c5a273a809": "{{value0}} から" }, "BrowserUseComputerUseNotice": { "15b5e680ba": "Open Computer Use", @@ -4891,7 +4892,7 @@ "be6df68384": "ファイルから…", "e44c5d681e": "から", "67d9a53f47": "個別のログインのプロファイルを管理する", - "112f70adc4": "最後にインポートされたのは", + "112f70adc4": "{{value0}} から最後にインポート", "72d4815523": "既存のログインを Orca に取り込み、agents が認証されたページにアクセスできるようにします。デフォルトのプロファイルにインポートします。", "2eb906706c": "ブラウザの Cookie をインポートする", "af8c83ed61": "Chrome、Edge、またはその他のブラウザから Cookie をインポートすると、agents がログインを再利用できるようになります。", @@ -4913,7 +4914,8 @@ "ad8cb0ee22": "パスを修正する", "0289434ed6": "有効", "8b3054dac7": "登録中...", - "8f2675c2f3": "ファイルから {{value0}} 個の Cookie をインポートしました。" + "8f2675c2f3": "ファイルから {{value0}} 個の Cookie をインポートしました。", + "5301857d88": "{{value0}} から" }, "BrowserUseSkillStep": { "0871b6998d": "agents が Orca のブラウザでページに移動して確認できるようにします。", @@ -9793,10 +9795,6 @@ "a84d5677ff": "OpenCode", "2528586aa7": "クラウディーノ" } - }, - "PetOverlay": { - "de932b0e8f": "@keyframes ペットボブ { 0%,100% { 変換:translateY(0); } 50% { 変換:translateY(-4px); } }", - "4712d196c6": "@keyframes pet-{{value0}} { from { 背景位置: {{value1}}px {{value2}}px; } から { 背景位置: {{value3}}px {{value4}}px; } }" } }, "onboarding": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 88c4baada28..62132106ac7 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -4867,7 +4867,8 @@ "d420c43729": "{{value1}}{{value2}}에서 {{value0}}개의 쿠키를 {{value3}}(으)로 가져왔습니다.", "b4c167764d": "파일에서 {{value0}}개의 쿠키를 {{value1}}에 가져왔습니다.", "a3f8c2d1e0b4": "{{value1}} ({{value2}})에서 {{value0}}개의 쿠키를 {{value3}}(으)로 가져왔습니다.", - "b4e9d3f2a1c5": "{{value1}}에서 {{value0}}개의 쿠키를 {{value2}}(으)로 가져왔습니다." + "b4e9d3f2a1c5": "{{value1}}에서 {{value0}}개의 쿠키를 {{value2}}(으)로 가져왔습니다.", + "c5a273a809": "{{value0}}에서" }, "BrowserUseComputerUseNotice": { "15b5e680ba": "컴퓨터 사용 열기", @@ -4891,7 +4892,7 @@ "be6df68384": "파일에서…", "e44c5d681e": "에서", "67d9a53f47": "별도의 로그인을 위한 프로필 관리", - "112f70adc4": "마지막으로 가져온 위치", + "112f70adc4": "{{value0}}에서 마지막으로 가져옴", "72d4815523": "agents가 인증된 페이지에 접근할 수 있도록 기존 로그인을 Orca로 가져오세요. 기본 프로필로 가져옵니다.", "2eb906706c": "브라우저 쿠키 가져오기", "af8c83ed61": "agents가 로그인을 재사용할 수 있도록 Chrome, Edge 또는 기타 브라우저에서 쿠키를 가져옵니다.", @@ -4913,7 +4914,8 @@ "ad8cb0ee22": "경로 수정", "0289434ed6": "활성화됨", "8b3054dac7": "등록 중...", - "8f2675c2f3": "파일에서 {{value0}}개의 쿠키를 가져왔습니다." + "8f2675c2f3": "파일에서 {{value0}}개의 쿠키를 가져왔습니다.", + "5301857d88": "{{value0}}에서" }, "BrowserUseSkillStep": { "0871b6998d": "agents가 Orca 브라우저에서 페이지를 탐색하고 확인할 수 있습니다.", @@ -9793,10 +9795,6 @@ "a84d5677ff": "OpenCode", "2528586aa7": "클라우디노" } - }, - "PetOverlay": { - "de932b0e8f": "@keyframes pet-bob { 0%,100% { transform: translateY(0); } 50% { transform: translateY(-4px); } }", - "4712d196c6": "@keyframes pet-{{value0}} { from { background-position: {{value1}}px {{value2}}px; } to { background-position: {{value3}}px {{value4}}px; } }" } }, "onboarding": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 18fd3bae5fc..41d3bbeb76c 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -4867,7 +4867,8 @@ "d420c43729": "将 {{value0}} cookie 从 {{value1}}{{value2}} 导入到 {{value3}} 中。", "b4c167764d": "已从文件将 {{value0}} 个 Cookie 导入到 {{value1}}。", "a3f8c2d1e0b4": "已从 {{value1}} ({{value2}}) 导入 {{value0}} 个 Cookie 到 {{value3}}。", - "b4e9d3f2a1c5": "已从 {{value1}} 导入 {{value0}} 个 Cookie 到 {{value2}}。" + "b4e9d3f2a1c5": "已从 {{value1}} 导入 {{value0}} 个 Cookie 到 {{value2}}。", + "c5a273a809": "从 {{value0}}" }, "BrowserUseComputerUseNotice": { "15b5e680ba": "开放计算机使用", @@ -4891,7 +4892,7 @@ "be6df68384": "从文件...", "e44c5d681e": "从", "67d9a53f47": "管理单独登录的配置文件", - "112f70adc4": "上次导入自", + "112f70adc4": "上次导入自 {{value0}}", "72d4815523": "将您现有的登录信息带入 Orca，以便智能体可以访问经过身份验证的页面。导入到默认配置文件中。", "2eb906706c": "导入浏览器 Cookie", "af8c83ed61": "从 Chrome、Edge 或其他浏览器导入 cookie，以便智能体可以重复使用您的登录信息。", @@ -4913,7 +4914,8 @@ "ad8cb0ee22": "修复路径", "0289434ed6": "启用", "8b3054dac7": "注册...", - "8f2675c2f3": "已从文件导入 {{value0}} 个 Cookie。" + "8f2675c2f3": "已从文件导入 {{value0}} 个 Cookie。", + "5301857d88": "从 {{value0}}" }, "BrowserUseSkillStep": { "0871b6998d": "使智能体能够在 Orca 浏览器中导航和验证页面。", @@ -9793,10 +9795,6 @@ "a84d5677ff": "OpenCode", "2528586aa7": "克劳迪诺" } - }, - "PetOverlay": { - "de932b0e8f": "@keyframes pet-bob { 0%,100% { 变换：translateY(0); } 50% { 变换：translateY(-4px); } }", - "4712d196c6": "@keyframes pet-{{value0}} { from { 背景位置：{{value1}}px {{value2}}px; } 到 { 背景位置：{{value3}}px {{value4}}px; } }" } }, "onboarding": { diff --git a/src/renderer/src/lib/github-links.test.ts b/src/renderer/src/lib/github-links.test.ts index 31c7f72aadd..64a9bd59df3 100644 --- a/src/renderer/src/lib/github-links.test.ts +++ b/src/renderer/src/lib/github-links.test.ts @@ -133,7 +133,36 @@ describe('normalizeGitHubLinkQuery', () => { it('accepts full GitHub URLs whose slug differs from the selected repo slug', () => { expect(normalizeGitHubLinkQuery('https://github.com/stablyai/orca/issues/923')).toEqual({ query: 'https://github.com/stablyai/orca/issues/923', - directNumber: 923 + directNumber: 923, + directLink: { + slug: { owner: 'stablyai', repo: 'orca' }, + number: 923, + type: 'issue' + } + }) + }) + + it('preserves PR route intent for full GitHub URLs', () => { + expect(normalizeGitHubLinkQuery('https://github.com/stablyai/orca/pull/6934')).toEqual({ + query: 'https://github.com/stablyai/orca/pull/6934', + directNumber: 6934, + directLink: { + slug: { owner: 'stablyai', repo: 'orca' }, + number: 6934, + type: 'pr' + } + }) + }) + + it('preserves route intent for URLs with uppercase schemes', () => { + expect(normalizeGitHubLinkQuery('HTTPS://github.com/stablyai/orca/pull/6934')).toEqual({ + query: 'HTTPS://github.com/stablyai/orca/pull/6934', + directNumber: 6934, + directLink: { + slug: { owner: 'stablyai', repo: 'orca' }, + number: 6934, + type: 'pr' + } }) }) diff --git a/src/renderer/src/lib/github-links.ts b/src/renderer/src/lib/github-links.ts index 3ef7119d32f..57e1988c33e 100644 --- a/src/renderer/src/lib/github-links.ts +++ b/src/renderer/src/lib/github-links.ts @@ -3,15 +3,19 @@ // Re-exported here so renderer consumers keep their '@/lib' import path. // normalizeGitHubLinkQuery stays renderer-side: its too-large guard is link- // picker input policy, not parsing. +import type { GitHubIssueOrPRLink } from '../../../shared/github-links' import { parseGitHubIssueOrPRLink, parseGitHubIssueOrPRNumber } from '../../../shared/github-links' import { isWorkItemLinkQueryTooLarge } from './work-item-link-query-bounds' export * from '../../../shared/github-links' +const HTTP_URL_PREFIX_RE = /^https?:\/\//i + export type GitHubLinkQuery = { query: string directNumber: number | null + directLink?: GitHubIssueOrPRLink tooLarge?: boolean } @@ -29,7 +33,7 @@ export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { } const direct = parseGitHubIssueOrPRNumber(trimmed) - if (direct !== null && !trimmed.startsWith('http')) { + if (direct !== null && !HTTP_URL_PREFIX_RE.test(trimmed)) { return { query: trimmed, directNumber: direct } } @@ -43,6 +47,7 @@ export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { // slug differs from the origin remote. return { query: trimmed, - directNumber: link.number + directNumber: link.number, + directLink: link } } diff --git a/src/renderer/src/lib/github-work-item-background-create.test.ts b/src/renderer/src/lib/github-work-item-background-create.test.ts index a4e6d8f0adf..ef7adfdb2b9 100644 --- a/src/renderer/src/lib/github-work-item-background-create.test.ts +++ b/src/renderer/src/lib/github-work-item-background-create.test.ts @@ -163,6 +163,46 @@ describe('createGitHubWorkItemWorkspaceInBackground', () => { ) }) + it('does not resolve a PR start point for a stale PR-typed issue URL', async () => { + const deps = makeDeps() + + await createGitHubWorkItemWorkspaceInBackground( + { + item: makeIssue({ + type: 'pr', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933', + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + }), + repoId: 'repo-1', + telemetrySource: 'sidebar', + openModalFallback: vi.fn() + }, + deps + ) + + expect(deps.resolvePrStartPoint).not.toHaveBeenCalled() + const beginCalls = deps.beginBackgroundCreate.mock.calls as unknown as [ + WorktreeCreationRequest + ][] + expect(beginCalls[0]?.[0].linkedIssue).toBe(6933) + expect(beginCalls[0]?.[0].linkedPR).toBeUndefined() + const continueCalls = deps.continueBackgroundCreate.mock.calls as unknown as [ + string, + WorktreeCreationRequest + ][] + const request = continueCalls[0]?.[1] + expect(request?.linkedIssue).toBe(6933) + expect(request?.linkedPR).toBeUndefined() + expect(request?.baseBranch).toBeUndefined() + expect(request?.pushTarget).toBeUndefined() + expect(request?.branchNameOverride).toBeUndefined() + expect(request?.compareBaseRef).toBeUndefined() + }) + it('shows the pending workspace before async preflight resolves', async () => { let resolveSetupDecision: ((value: { kind: 'decided'; decision: 'inherit' }) => void) | null = null @@ -566,17 +606,36 @@ describe('createGitHubWorkItemWorkspaceInBackground', () => { await createGitHubWorkItemWorkspaceInBackground( { - item: makeIssue({ type: 'pr', number: 7, url: 'https://github.com/stablyai/orca/pull/7' }), + item: makeIssue({ + type: 'pr', + number: 6934, + url: 'https://github.com/stablyai/orca/pull/6934', + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + }), repoId: 'repo-1', openModalFallback: vi.fn() }, deps ) + expect(deps.resolvePrStartPoint).toHaveBeenCalledWith( + 'repo-1', + 6934, + expect.anything(), + expect.objectContaining({ + type: 'pr', + number: 6934, + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + }) + ) expect(deps.continueBackgroundCreate).toHaveBeenCalledWith( 'creation-1', expect.objectContaining({ - linkedPR: 7, + linkedPR: 6934, baseBranch: 'feature/from-pr', pushTarget: { remote: 'origin', branch: 'feature/from-pr' }, branchNameOverride: 'feature/from-pr', diff --git a/src/renderer/src/lib/github-work-item-background-create.ts b/src/renderer/src/lib/github-work-item-background-create.ts index 7fa94fd9d2b..260cd2ad657 100644 --- a/src/renderer/src/lib/github-work-item-background-create.ts +++ b/src/renderer/src/lib/github-work-item-background-create.ts @@ -32,6 +32,7 @@ import { import type { GitHubWorkItem, SetupDecision } from '../../../shared/types' import type { Repo } from '../../../shared/types' import type { TaskSourceContext, WorkspaceRunContext } from '../../../shared/task-source-context' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' export type BackgroundGitHubWorkItemCreateResult = | { kind: 'background-started' } @@ -182,6 +183,7 @@ export async function createGitHubWorkItemWorkspaceInBackground( const restoreView = deps.getActiveView() const creationId = deps.beginBackgroundCreate(initialRequest) + const itemIdentity = resolveGitHubWorkItemIdentity(args.item) try { const repoOwnerSettings = getSettingsForRepoRuntimeOwner(store, args.repoId) @@ -201,11 +203,11 @@ export async function createGitHubWorkItemWorkspaceInBackground( let pushTarget: WorktreeCreationRequest['pushTarget'] let branchNameOverride: string | undefined let compareBaseRef: string | undefined - if (args.item.type === 'pr' && args.item.number) { + if (itemIdentity.type === 'pr' && itemIdentity.number) { try { const result = await deps.resolvePrStartPoint( args.repoId, - args.item.number, + itemIdentity.number, repoOwnerSettings, args.item ) diff --git a/src/renderer/src/lib/github-work-item-background-request.ts b/src/renderer/src/lib/github-work-item-background-request.ts index f37c71ded0a..5b157525053 100644 --- a/src/renderer/src/lib/github-work-item-background-request.ts +++ b/src/renderer/src/lib/github-work-item-background-request.ts @@ -14,6 +14,7 @@ import { CLIENT_PLATFORM, getWorkspaceIntentName, getWorkspaceSeedName } from '@ import { getLocalRepoProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' import { resolveSourceControlLaunchPlatform } from '@/lib/source-control-launch-platform' import { repoIsRemote } from '../../../shared/agent-launch-remote' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' import { resolveTuiAgentLaunchArgs, resolveTuiAgentLaunchEnv @@ -232,19 +233,20 @@ export function buildGitHubWorkItemStartupPlan(args: { } function getGitHubWorkItemName(item: GitHubWorkItem): { seedName: string; displayName?: string } { + const identity = resolveGitHubWorkItemIdentity(item) const intent = - item.number !== null + identity.number !== null ? getWorkspaceIntentName({ sourceText: item.title, - workItem: { type: item.type, number: item.number, title: item.title } + workItem: { type: identity.type, number: identity.number, title: item.title } }) : null return { seedName: getWorkspaceSeedName({ explicitName: intent?.seedName ?? '', prompt: '', - linkedIssueNumber: item.type === 'issue' ? item.number : null, - linkedPR: item.type === 'pr' ? item.number : null + linkedIssueNumber: identity.type === 'issue' ? identity.number : null, + linkedPR: identity.type === 'pr' ? identity.number : null }), ...(intent?.displayName ? { displayName: intent.displayName } : {}) } @@ -257,6 +259,7 @@ export function buildInitialGitHubWorkItemRequest( const { seedName, displayName } = getGitHubWorkItemName(args.item) const workspaceRunContext = getWorkspaceRunContextForRepo(repo, args.workspaceRunContext) const ownerHost = parseExecutionHostId(getRepoExecutionHostId(repo)) + const identity = resolveGitHubWorkItemIdentity(args.item) return { repoId: args.repoId, worktreeCreateProgressMode: ownerHost?.kind === 'local' ? 'stepped' : 'indeterminate', @@ -264,8 +267,8 @@ export function buildInitialGitHubWorkItemRequest( ...(workspaceRunContext ? { workspaceRunContext } : {}), name: seedName, ...(displayName ? { displayName } : {}), - ...(args.item.type === 'issue' && args.item.number ? { linkedIssue: args.item.number } : {}), - ...(args.item.type === 'pr' && args.item.number ? { linkedPR: args.item.number } : {}), + ...(identity.type === 'issue' && identity.number ? { linkedIssue: identity.number } : {}), + ...(identity.type === 'pr' && identity.number ? { linkedPR: identity.number } : {}), ...(args.telemetrySource ? { telemetrySource: args.telemetrySource } : {}), setupDecision: 'inherit', agent: null, diff --git a/src/renderer/src/lib/github-work-item-identity.ts b/src/renderer/src/lib/github-work-item-identity.ts new file mode 100644 index 00000000000..c9b24e316ff --- /dev/null +++ b/src/renderer/src/lib/github-work-item-identity.ts @@ -0,0 +1,20 @@ +import { parseGitHubIssueOrPRLink } from '@/lib/github-links' + +export type GitHubWorkItemIdentity = { + type: 'issue' | 'pr' + number: number +} + +export function resolveGitHubWorkItemIdentity(item: { + type: 'issue' | 'pr' + number: number + url?: string | null +}): GitHubWorkItemIdentity { + const link = item.url ? parseGitHubIssueOrPRLink(item.url) : null + if (link) { + // Why: stale cached work-item payloads can disagree with a pasted URL. The + // URL path is the user-visible intent, so it decides issue-vs-PR launches. + return { type: link.type, number: link.number } + } + return { type: item.type, number: item.number } +} diff --git a/src/renderer/src/lib/launch-work-item-direct.test.ts b/src/renderer/src/lib/launch-work-item-direct.test.ts index 98913111af8..33ae88fde57 100644 --- a/src/renderer/src/lib/launch-work-item-direct.test.ts +++ b/src/renderer/src/lib/launch-work-item-direct.test.ts @@ -229,9 +229,9 @@ describe('launchWorkItemDirect', () => { openModalFallback: vi.fn(), item: { type: 'pr', - number: 42, + number: 6934, title: 'Fix the bug', - url: 'https://github.com/acme/repo/pull/42', + url: 'https://github.com/stablyai/orca/pull/6934', branchName: 'feature/fix', baseRefName: 'main', isCrossRepository: true @@ -240,21 +240,21 @@ describe('launchWorkItemDirect', () => { expect(mocks.resolvePrBase).toHaveBeenCalledWith({ repoId: 'repo-1', - prNumber: 42, + prNumber: 6934, headRefName: 'feature/fix', baseRefName: 'main', isCrossRepository: true }) expect(mocks.createWorktree).toHaveBeenCalledWith( 'repo-1', - 'review-pr-42', + 'review-pr-6934', 'abc123', 'inherit', undefined, 'sidebar', - 'Review PR 42', + 'Review PR 6934', undefined, - 42, + 6934, { remoteName: 'origin', branchName: 'feature/fix' }, undefined, undefined, @@ -274,6 +274,41 @@ describe('launchWorkItemDirect', () => { ) }) + it('treats a PR-typed GitHub issue URL as an issue without resolving a PR head', async () => { + const { launchWorkItemDirect } = await import('./launch-work-item-direct') + const openModalFallback = vi.fn() + + await expect( + launchWorkItemDirect({ + repoId: 'repo-1', + launchSource: 'task_page', + telemetrySource: 'sidebar', + openModalFallback, + item: { + type: 'pr', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933', + branchName: 'fix-issue-6933', + baseRefName: 'main', + isCrossRepository: true + } + }) + ).resolves.toBe(true) + + expect(mocks.resolvePrBase).not.toHaveBeenCalled() + expect(openModalFallback).not.toHaveBeenCalled() + const createArgs = mocks.createWorktree.mock.calls[0] + expect(createArgs?.[1]).toBe('issue-6933') + expect(createArgs?.[2]).toBeUndefined() + expect(createArgs?.[6]).toBe('Issue 6933') + expect(createArgs?.[7]).toBe(6933) + expect(createArgs?.[8]).toBeUndefined() + expect(createArgs?.[9]).toBeUndefined() + expect(createArgs?.[12]).toBeUndefined() + expect(createArgs?.[24]).toBeUndefined() + }) + it('uses the Linear identifier in direct-launch workspace names', async () => { const { launchWorkItemDirect } = await import('./launch-work-item-direct') diff --git a/src/renderer/src/lib/launch-work-item-direct.ts b/src/renderer/src/lib/launch-work-item-direct.ts index def1c5c6c8a..61f49274edb 100644 --- a/src/renderer/src/lib/launch-work-item-direct.ts +++ b/src/renderer/src/lib/launch-work-item-direct.ts @@ -16,6 +16,7 @@ import { ensureHooksConfirmed } from '@/lib/ensure-hooks-confirmed' import { getConnectionId } from '@/lib/connection-context' import type { GitPushTarget, SetupDecision, TuiAgent } from '../../../shared/types' import { getLinearIssueWorkspaceName } from '../../../shared/workspace-name' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' import { buildDirectWorkItemAgentStartupPlan, buildDirectWorkItemStartupOpts, @@ -70,6 +71,16 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom const repoOwnerSettings = getSettingsForRepoRuntimeOwner(store, repoId) const promptDelivery = args.promptDelivery ?? 'draft' const repoConnectionId = repo.connectionId?.trim() || null + const githubIdentity = + item.number !== null && (item.type === 'issue' || item.type === 'pr') + ? resolveGitHubWorkItemIdentity({ + type: item.type, + number: item.number, + url: item.url + }) + : null + const itemType = githubIdentity?.type ?? item.type + const itemNumber = githubIdentity?.number ?? item.number const repoProjectRuntime = repoConnectionId ? undefined : getLocalRepoProjectExecutionRuntimeContext(store, repoId, CLIENT_PLATFORM) @@ -107,10 +118,10 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom trustDecision === 'skip' ? 'skip' : setupResolution.decision const workspaceIntentName = - item.number !== null + itemNumber !== null ? getWorkspaceIntentName({ sourceText: item.pasteContent, - workItem: { ...item, number: item.number } + workItem: { ...item, type: itemType, number: itemNumber } }) : null const workspaceName = getWorkspaceSeedName({ @@ -118,18 +129,18 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom ? getLinearIssueWorkspaceName({ identifier: item.linearIdentifier, title: item.title }) : (workspaceIntentName?.seedName ?? ''), prompt: '', - linkedIssueNumber: item.type === 'issue' ? (item.number ?? null) : null, - linkedPR: item.type === 'pr' ? (item.number ?? null) : null + linkedIssueNumber: itemType === 'issue' ? (itemNumber ?? null) : null, + linkedPR: itemType === 'pr' ? (itemNumber ?? null) : null }) let resolvedBaseBranch = baseBranch let resolvedPushTarget: GitPushTarget | undefined let resolvedBranchNameOverride: string | undefined let resolvedCompareBaseRef: string | undefined - if (!resolvedBaseBranch && item.type === 'pr' && item.number) { + if (!resolvedBaseBranch && itemType === 'pr' && itemNumber) { try { // Why: direct "Use PR" launches bypass the Start-from picker, so they // must still resolve the PR head before `git worktree add`. - const result = await resolveDirectPrStartPoint(repoId, item.number, repoOwnerSettings, item) + const result = await resolveDirectPrStartPoint(repoId, itemNumber, repoOwnerSettings, item) resolvedBaseBranch = result.baseBranch resolvedPushTarget = result.pushTarget resolvedBranchNameOverride = result.branchNameOverride @@ -157,15 +168,15 @@ export async function launchWorkItemDirect(args: LaunchWorkItemDirectArgs): Prom undefined, telemetrySource, workspaceIntentName?.displayName ?? item.title, - item.type === 'issue' && item.number ? item.number : undefined, - item.type === 'pr' && item.number ? item.number : undefined, + itemType === 'issue' && itemNumber ? itemNumber : undefined, + itemType === 'pr' && itemNumber ? itemNumber : undefined, resolvedPushTarget, undefined, item.linearIdentifier, resolvedBranchNameOverride, undefined, - item.type === 'mr' && item.number ? item.number : undefined, - gitLabIssueNumber(item), + itemType === 'mr' && itemNumber ? itemNumber : undefined, + gitLabIssueNumber({ ...item, type: itemType, number: itemNumber }), undefined, undefined, undefined, diff --git a/src/renderer/src/lib/smart-github-submit.test.ts b/src/renderer/src/lib/smart-github-submit.test.ts index 43ec3c36325..64e58824e0c 100644 --- a/src/renderer/src/lib/smart-github-submit.test.ts +++ b/src/renderer/src/lib/smart-github-submit.test.ts @@ -299,4 +299,26 @@ describe('getSmartGitHubSubmitResolution', () => { expect(resolution.linkedIssueNumber).toBe(2050) expect(resolution.linkedPR).toBeNull() }) + + it('uses the URL path to normalize stale PR-typed issue results', () => { + expect( + getSmartGitHubSubmitResolution({ + type: 'pr', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933' + }) + ).toEqual({ + workspaceName: 'the-board-columns-are-displayed-backwards', + displayName: 'The board columns are displayed backwards', + linkedWorkItem: { + type: 'issue', + number: 6933, + title: 'The board columns are displayed backwards', + url: 'https://github.com/stablyai/orca/issues/6933' + }, + linkedIssueNumber: 6933, + linkedPR: null + }) + }) }) diff --git a/src/renderer/src/lib/smart-github-submit.ts b/src/renderer/src/lib/smart-github-submit.ts index c6ee5abff47..1b4e818f26a 100644 --- a/src/renderer/src/lib/smart-github-submit.ts +++ b/src/renderer/src/lib/smart-github-submit.ts @@ -4,6 +4,7 @@ import { getTaskSourceCacheScope } from '../../../shared/task-source-context' import { getLinkedWorkItemWorkspaceName } from '../../../shared/workspace-name' import type { LinkedWorkItemSummary } from './new-workspace' import { parseGitHubIssueOrPRLink } from './github-links' +import { resolveGitHubWorkItemIdentity } from '@/lib/github-work-item-identity' export type SmartGitHubSubmitIntent = | { @@ -196,12 +197,14 @@ export function getSmartGitHubSubmitLookupCacheSizeForTests(): number { export function getSmartGitHubSubmitResolution( item: Pick ): SmartGitHubSubmitResolution { - const fallbackName = `${item.type}-${item.number}` - const titleName = getLinkedWorkItemWorkspaceName(item) + const identity = resolveGitHubWorkItemIdentity(item) + const normalizedItem = { ...item, type: identity.type, number: identity.number } + const fallbackName = `${identity.type}-${identity.number}` + const titleName = getLinkedWorkItemWorkspaceName(normalizedItem) const workspaceName = titleName?.seedName || fallbackName const linkedWorkItem: LinkedWorkItemSummary = { - type: item.type, - number: item.number, + type: identity.type, + number: identity.number, title: item.title, url: item.url } @@ -210,7 +213,7 @@ export function getSmartGitHubSubmitResolution( workspaceName, displayName: titleName?.displayName ?? fallbackName, linkedWorkItem, - linkedIssueNumber: item.type === 'issue' ? item.number : null, - linkedPR: item.type === 'pr' ? item.number : null + linkedIssueNumber: identity.type === 'issue' ? identity.number : null, + linkedPR: identity.type === 'pr' ? identity.number : null } } diff --git a/src/shared/github-links.ts b/src/shared/github-links.ts index 0d55bbacdee..a53562f5d5d 100644 --- a/src/shared/github-links.ts +++ b/src/shared/github-links.ts @@ -8,6 +8,12 @@ export type RepoSlug = { repo: string } +export type GitHubIssueOrPRLink = { + slug: RepoSlug + number: number + type: 'issue' | 'pr' +} + export function buildGitHubRepoUrl(slug: RepoSlug | null | undefined): string | null { if (!slug?.owner || !slug.repo) { return null @@ -62,11 +68,7 @@ export function parseGitHubIssueOrPRNumber(input: string): number | null { * Parses an owner/repo slug plus issue/PR number from a GitHub URL. Returns * null for anything that isn't a recognizable GitHub-shaped issue or pull URL. */ -export function parseGitHubIssueOrPRLink(input: string): { - slug: RepoSlug - number: number - type: 'issue' | 'pr' -} | null { +export function parseGitHubIssueOrPRLink(input: string): GitHubIssueOrPRLink | null { const trimmed = input.trim() if (!trimmed) { return null diff --git a/src/shared/secure-file-coarse-ctime.test.ts b/src/shared/secure-file-coarse-ctime.test.ts new file mode 100644 index 00000000000..64c33ba36f1 --- /dev/null +++ b/src/shared/secure-file-coarse-ctime.test.ts @@ -0,0 +1,123 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { chmodSyncMock, existsSyncMock, paths, statSyncMock, state } = vi.hoisted(() => ({ + chmodSyncMock: vi.fn(), + existsSyncMock: vi.fn(), + paths: { + dir: '/secure', + file: '/secure/secret.json' + }, + statSyncMock: vi.fn(), + state: { + dirMode: 0o755, + fileMode: 0o600 + } +})) + +vi.mock('fs', () => ({ + chmodSync: chmodSyncMock, + existsSync: existsSyncMock, + mkdirSync: vi.fn(), + renameSync: vi.fn(), + rmSync: vi.fn(), + statSync: statSyncMock, + writeFileSync: vi.fn() +})) + +vi.mock('child_process', () => ({ + execFile: vi.fn(), + execFileSync: vi.fn() +})) + +import { + __resetSecureFileHardenedPathsForTests, + __resetSecureFileWindowsUserSidForTests, + hardenExistingSecureFile +} from './secure-file' + +describe('secure-file coarse-ctime mode drift', () => { + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + state.dirMode = 0o755 + state.fileMode = 0o600 + existsSyncMock.mockReturnValue(true) + statSyncMock.mockImplementation((targetPath: string) => { + const path = String(targetPath) + if (path === paths.dir) { + return fakeStats(true, state.dirMode) + } + if (path === paths.file) { + return fakeStats(false, state.fileMode) + } + throw new Error(`unexpected stat path ${path}`) + }) + chmodSyncMock.mockImplementation((targetPath: string, mode: number) => { + const path = String(targetPath) + if (path === paths.dir) { + state.dirMode = mode & 0o777 + } else if (path === paths.file) { + state.fileMode = mode & 0o777 + } + }) + __resetSecureFileWindowsUserSidForTests() + __resetSecureFileHardenedPathsForTests() + }) + + afterEach(() => { + chmodSyncMock.mockReset() + existsSyncMock.mockReset() + statSyncMock.mockReset() + __resetSecureFileWindowsUserSidForTests() + __resetSecureFileHardenedPathsForTests() + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + }) + + it('re-hardens a directory when only POSIX mode changes under stable timestamps', () => { + hardenExistingSecureFile(paths.file) + expect(chmodSyncMock.mock.calls).toEqual([ + [paths.dir, 0o700], + [paths.file, 0o600] + ]) + + chmodSyncMock.mockClear() + // Coarse-ctime filesystems can report identical timestamps after chmod; only mode changes. + state.dirMode = 0o755 + + hardenExistingSecureFile(paths.file) + + expect(chmodSyncMock.mock.calls).toEqual([[paths.dir, 0o700]]) + }) + + it('re-hardens a file when only POSIX mode changes under stable timestamps', () => { + hardenExistingSecureFile(paths.file) + expect(chmodSyncMock.mock.calls).toEqual([ + [paths.dir, 0o700], + [paths.file, 0o600] + ]) + + chmodSyncMock.mockClear() + // Coarse-ctime filesystems can report identical timestamps after chmod; only mode changes. + state.fileMode = 0o644 + + hardenExistingSecureFile(paths.file) + + expect(chmodSyncMock.mock.calls).toEqual([[paths.file, 0o600]]) + }) +}) + +function fakeStats(isDirectory: boolean, mode: number) { + return { + birthtimeMs: 10, + ctimeMs: 20, + dev: 30, + ino: isDirectory ? 40 : 41, + isDirectory: () => isDirectory, + mode, + mtimeMs: 50, + size: isDirectory ? 0 : 2 + } +} diff --git a/src/shared/secure-file.ts b/src/shared/secure-file.ts index 146c62b09e0..1ed451b5633 100644 --- a/src/shared/secure-file.ts +++ b/src/shared/secure-file.ts @@ -18,6 +18,7 @@ type HardenedPathCacheEntry = { dev: number ino: number size: number + mode: number ctimeMs: number mtimeMs: number birthtimeMs: number @@ -130,6 +131,7 @@ export function hardenExistingSecureFile(targetPath: string): void { } } +/** Applies the platform-appropriate permission restriction to a path once, bypassing the cache. */ export function hardenSecurePath( targetPath: string, options: { @@ -146,6 +148,7 @@ export function hardenSecurePath( ) } +/** Applies hardening; async Windows calls only report that best-effort ACL work was accepted. */ function applySecurePathRestriction( targetPath: string, isDirectory: boolean, @@ -170,6 +173,7 @@ function applySecurePathRestriction( return true } +/** Caches the current metadata snapshot for a just-hardened path, or clears it if the path is gone. */ function rememberHardenedPath(targetPath: string, isDirectory: boolean): void { const entry = getHardenedPathCacheEntry(targetPath, isDirectory) if (entry) { @@ -179,6 +183,10 @@ function rememberHardenedPath(targetPath: string, isDirectory: boolean): void { } } +/** + * Snapshots a path's identity, mode, and timestamps so later drift is detectable. + * Mode is tracked directly so a chmod is caught even where coarse ctime granularity hides it. + */ function getHardenedPathCacheEntry( targetPath: string, isDirectory: boolean @@ -193,6 +201,7 @@ function getHardenedPathCacheEntry( dev: stats.dev, ino: stats.ino, size: stats.size, + mode: stats.mode & 0o777, ctimeMs: stats.ctimeMs, mtimeMs: stats.mtimeMs, birthtimeMs: stats.birthtimeMs @@ -202,6 +211,7 @@ function getHardenedPathCacheEntry( } } +/** True when two snapshots describe the same unchanged path (identity, mode, timestamps). */ function hardenedPathCacheEntriesMatch( a: HardenedPathCacheEntry, b: HardenedPathCacheEntry @@ -211,6 +221,7 @@ function hardenedPathCacheEntriesMatch( a.dev === b.dev && a.ino === b.ino && a.size === b.size && + a.mode === b.mode && a.ctimeMs === b.ctimeMs && a.mtimeMs === b.mtimeMs && a.birthtimeMs === b.birthtimeMs