diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index 2f76c8079..b3465ac16 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -42,6 +42,8 @@ const { discoverCommitMessageModelsRemoteMock, cancelGenerateCommitMessageLocalMock, cancelGeneratePullRequestFieldsLocalMock, + getPullRequestDraftContextMock, + resolveHostedReviewBodyForGenerationMock, getSshFilesystemProviderMock, getSshGitProviderMock, tryDeleteWslUncPathMock, @@ -85,6 +87,8 @@ const { discoverCommitMessageModelsRemoteMock: vi.fn(), cancelGenerateCommitMessageLocalMock: vi.fn(), cancelGeneratePullRequestFieldsLocalMock: vi.fn(), + getPullRequestDraftContextMock: vi.fn(), + resolveHostedReviewBodyForGenerationMock: vi.fn(), getSshFilesystemProviderMock: vi.fn(), getSshGitProviderMock: vi.fn(), tryDeleteWslUncPathMock: vi.fn(), @@ -189,6 +193,16 @@ vi.mock('../text-generation/commit-message-text-generation', () => ({ cancelGeneratePullRequestFieldsLocal: cancelGeneratePullRequestFieldsLocalMock })) +vi.mock('../text-generation/pull-request-context', () => ({ + getPullRequestDraftContext: getPullRequestDraftContextMock +})) + +vi.mock('../source-control/pull-request-template', () => ({ + readHostedPullRequestTemplate: vi.fn(), + readHostedReviewTemplate: vi.fn(), + resolveHostedReviewBodyForGeneration: resolveHostedReviewBodyForGenerationMock +})) + import { registerFilesystemHandlers } from './filesystem' import { invalidateAuthorizedRootsCache, registerWorktreeRootsForRepo } from './filesystem-auth' @@ -289,6 +303,8 @@ describe('registerFilesystemHandlers', () => { resolveCommitMessageSettingsMock, generateCommitMessageFromContextMock, generatePullRequestFieldsFromContextMock, + getPullRequestDraftContextMock, + resolveHostedReviewBodyForGenerationMock, discoverCommitMessageModelsLocalMock, discoverCommitMessageModelsRemoteMock, cancelGenerateCommitMessageLocalMock, @@ -2056,6 +2072,220 @@ describe('registerFilesystemHandlers', () => { }) }) + it('enriches the local commit context with a validated worktree linked issue', async () => { + const context = { + branch: 'feature/ai', + stagedSummary: 'M\tREADME.md', + stagedPatch: '+hello' + } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + const worktreeId = `repo-1::${WORKTREE_FEATURE_PATH}` + resolveCommitMessageSettingsMock.mockReturnValue({ ok: true, params }) + getStagedCommitContextMock.mockResolvedValue(context) + generateCommitMessageFromContextMock.mockResolvedValue({ success: true, message: 'Update' }) + const linkedStore = { + ...store, + getWorktreeMeta: (id: string) => (id === worktreeId ? { linkedIssue: 123 } : undefined) + } + + registerFilesystemHandlers(linkedStore as never) + + await handlers.get('git:generateCommitMessage')!(null, { + worktreePath: WORKTREE_FEATURE_PATH, + worktreeId + }) + + expect(generateCommitMessageFromContextMock).toHaveBeenCalledWith( + { ...context, linkedIssue: 123 }, + params, + expect.objectContaining({ kind: 'local' }) + ) + }) + + // Why: folder-repo instances keep `::workspace:` on the meta key while the + // request path is the stripped cwd. A strip-before-lookup "cleanup" would still + // pass plain-id tests and silently lose enrichment on second workspaces. + it('enriches local commit context when the worktree id carries a folder-repo workspace suffix', async () => { + const context = { + branch: 'feature/ai', + stagedSummary: 'M\tREADME.md', + stagedPatch: '+hello' + } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + const instanceId = `repo-1::${WORKTREE_FEATURE_PATH}::workspace:${'0'.repeat(8)}-0000-0000-0000-${'0'.repeat(12)}` + resolveCommitMessageSettingsMock.mockReturnValue({ ok: true, params }) + getStagedCommitContextMock.mockResolvedValue(context) + generateCommitMessageFromContextMock.mockResolvedValue({ success: true, message: 'Update' }) + const getWorktreeMeta = vi.fn((id: string) => + id === instanceId ? { linkedIssue: 9 } : undefined + ) + + registerFilesystemHandlers({ ...store, getWorktreeMeta } as never) + + await handlers.get('git:generateCommitMessage')!(null, { + worktreePath: WORKTREE_FEATURE_PATH, + worktreeId: instanceId + }) + + expect(getWorktreeMeta).toHaveBeenCalledWith(instanceId) + expect(generateCommitMessageFromContextMock).toHaveBeenCalledWith( + { ...context, linkedIssue: 9 }, + params, + expect.objectContaining({ kind: 'local' }) + ) + }) + + // Why: the renderer derives worktreePath from worktreeId, so a mismatched pair + // models an independent caller (relay/CLI/future), not a stale renderer context. + it('ignores an independently supplied id that does not own the requested worktree path', async () => { + const context = { + branch: 'feature/ai', + stagedSummary: 'M\tREADME.md', + stagedPatch: '+hello' + } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + const getWorktreeMeta = vi.fn(() => ({ linkedIssue: 123 })) + resolveCommitMessageSettingsMock.mockReturnValue({ ok: true, params }) + getStagedCommitContextMock.mockResolvedValue(context) + generateCommitMessageFromContextMock.mockResolvedValue({ success: true, message: 'Update' }) + + registerFilesystemHandlers({ ...store, getWorktreeMeta } as never) + + await handlers.get('git:generateCommitMessage')!(null, { + worktreePath: WORKTREE_FEATURE_PATH, + worktreeId: `repo-1::${path.resolve('/workspace/repo-other')}` + }) + + expect(getWorktreeMeta).not.toHaveBeenCalled() + // Why: without this the assertion below passes vacuously on an early return. + expect(generateCommitMessageFromContextMock.mock.calls).toHaveLength(1) + expect(generateCommitMessageFromContextMock.mock.calls[0]?.[0]).not.toHaveProperty( + 'linkedIssue' + ) + }) + + it('enriches the SSH commit context from host meta using the remote path', async () => { + const context = { branch: 'main', stagedSummary: 'A\tremote.txt', stagedPatch: '+remote' } + const params = { agentId: 'custom', model: '', customAgentCommand: 'agent' } + const worktreeId = 'repo-1::/remote/repo' + resolveCommitMessageSettingsMock.mockReturnValue({ ok: true, params }) + getSshGitProviderMock.mockReturnValue({ + getStagedCommitContext: vi.fn().mockResolvedValue(context), + executeCommitMessagePlan: vi.fn() + }) + generateCommitMessageFromContextMock.mockResolvedValue({ success: true, message: 'Add file' }) + const linkedStore = { + ...store, + getWorktreeMeta: (id: string) => (id === worktreeId ? { linkedIssue: 77 } : undefined) + } + + registerFilesystemHandlers(linkedStore as never) + + await handlers.get('git:generateCommitMessage')!(null, { + worktreePath: '/remote/repo', + worktreeId, + connectionId: 'conn-1' + }) + + expect(generateCommitMessageFromContextMock).toHaveBeenCalledWith( + { ...context, linkedIssue: 77 }, + params, + expect.objectContaining({ kind: 'remote' }) + ) + }) + + describe('git:generatePullRequestFields linked issue', () => { + const PULL_REQUEST_CONTEXT = { + base: 'main', + branch: 'feature/ai', + branchChangedByPreparation: false, + commitSummary: 'a1b2c3d Add generation', + changeSummary: 'README.md | 2 +-', + patch: '+hello', + currentTitle: '', + currentBody: '', + currentDraft: false + } + const PULL_REQUEST_ARGS = { base: 'main', title: '', body: '', draft: false } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + + beforeEach(() => { + resolveCommitMessageSettingsMock.mockReturnValue({ ok: true, params }) + resolveHostedReviewBodyForGenerationMock.mockResolvedValue('') + getPullRequestDraftContextMock.mockResolvedValue(PULL_REQUEST_CONTEXT) + generatePullRequestFieldsFromContextMock.mockResolvedValue({ success: true, fields: {} }) + }) + + it('enriches the local pull-request context with a validated worktree linked issue', async () => { + const worktreeId = `repo-1::${WORKTREE_FEATURE_PATH}` + const linkedStore = { + ...store, + getWorktreeMeta: (id: string) => (id === worktreeId ? { linkedIssue: 123 } : undefined) + } + + registerFilesystemHandlers(linkedStore as never) + + await handlers.get('git:generatePullRequestFields')!(null, { + ...PULL_REQUEST_ARGS, + worktreePath: WORKTREE_FEATURE_PATH, + worktreeId + }) + + expect(generatePullRequestFieldsFromContextMock).toHaveBeenCalledWith( + { ...PULL_REQUEST_CONTEXT, linkedIssue: 123 }, + params, + expect.objectContaining({ kind: 'local' }) + ) + }) + + it('enriches the SSH pull-request context from host meta using the remote path', async () => { + const worktreeId = 'repo-1::/remote/repo' + getSshGitProviderMock.mockReturnValue({ + exec: vi.fn(), + executeCommitMessagePlan: vi.fn() + }) + const linkedStore = { + ...store, + getWorktreeMeta: (id: string) => (id === worktreeId ? { linkedIssue: 77 } : undefined) + } + + registerFilesystemHandlers(linkedStore as never) + + await handlers.get('git:generatePullRequestFields')!(null, { + ...PULL_REQUEST_ARGS, + worktreePath: '/remote/repo', + worktreeId, + connectionId: 'conn-1' + }) + + expect(generatePullRequestFieldsFromContextMock).toHaveBeenCalledWith( + { ...PULL_REQUEST_CONTEXT, linkedIssue: 77 }, + params, + expect.objectContaining({ kind: 'remote' }) + ) + }) + + it('ignores a pull-request worktree id that does not own the requested path', async () => { + const getWorktreeMeta = vi.fn(() => ({ linkedIssue: 123 })) + + registerFilesystemHandlers({ ...store, getWorktreeMeta } as never) + + await handlers.get('git:generatePullRequestFields')!(null, { + ...PULL_REQUEST_ARGS, + worktreePath: WORKTREE_FEATURE_PATH, + worktreeId: `repo-1::${path.resolve('/workspace/repo-other')}` + }) + + expect(getWorktreeMeta).not.toHaveBeenCalled() + // Why: without the length guard the property assertion passes vacuously on `undefined`, + // so an unrelated early return would read as "enrichment correctly suppressed". + expect(generatePullRequestFieldsFromContextMock.mock.calls).toHaveLength(1) + expect(generatePullRequestFieldsFromContextMock.mock.calls[0]?.[0]).not.toHaveProperty( + 'linkedIssue' + ) + }) + }) + it('returns a sanitized error when local agent account preparation fails', async () => { const context = { branch: 'feature/ai', diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 414b67516..726d5b0b8 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -87,6 +87,7 @@ import { assertGitPushTargetShape } from '../../shared/git-push-target-validatio import { getCommitMessageModelDiscoveryHostKey } from '../../shared/commit-message-host-key' import type { HostedReviewProvider } from '../../shared/hosted-review' import type { ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' +import { withLinkedIssueDraftContext } from '../../shared/source-control-ai-action-variables' import { validateGitPushTarget } from '../git/push-target-validation' import { getRemoteCommitUrl, getRemoteFileUrl } from '../git/repo' import { @@ -100,6 +101,7 @@ import { listQuickOpenFiles } from './filesystem-list-files' import { registerFilesystemMutationHandlers } from './filesystem-mutations' import { searchWithGitGrep } from './filesystem-search-git' import { getLocalGitOptionsForRegisteredWorktree } from './local-worktree-runtime-options' +import { resolveSourceControlAiLinkedIssue } from './source-control-ai-linked-issue' import { listMarkdownDocuments, markdownDocumentsFromRelativePaths } from './markdown-documents' import { checkRgAvailable } from './rg-availability' import { @@ -1360,6 +1362,8 @@ export function registerFilesystemHandlers( _event, args: { worktreePath: string + // Raw (unstripped) meta key; validated against worktreePath before any meta read. + worktreeId?: string repoId?: string connectionId?: string sourceControlAiResolvedParams?: ResolvedSourceControlAiGenerationParams @@ -1408,6 +1412,10 @@ export function registerFilesystemHandlers( if (!context) { return { success: false, error: 'No staged changes to summarize.' } } + context = withLinkedIssueDraftContext( + context, + resolveSourceControlAiLinkedIssue(store, args) + ) return generateCommitMessageFromContext(context, resolvedSettings.params, { kind: 'remote', cwd: args.worktreePath, @@ -1435,6 +1443,10 @@ export function registerFilesystemHandlers( if (!context) { return { success: false, error: 'No staged changes to summarize.' } } + context = withLinkedIssueDraftContext( + context, + resolveSourceControlAiLinkedIssue(store, args, worktreePath) + ) const localEnv = await prepareLocalCommitMessageAgentEnv( resolvedSettings.params.agentId, commitMessageAgentEnv, @@ -1532,6 +1544,8 @@ export function registerFilesystemHandlers( _event, args: { worktreePath: string + // Raw (unstripped) meta key; validated against worktreePath before any meta read. + worktreeId?: string repoId?: string base: string title: string @@ -1601,6 +1615,10 @@ export function registerFilesystemHandlers( if (!context) { return { success: false, error: 'No branch changes to summarize.' } } + context = withLinkedIssueDraftContext( + context, + resolveSourceControlAiLinkedIssue(store, args) + ) return generatePullRequestFieldsFromContext(context, resolvedSettings.params, { kind: 'remote', cwd: args.worktreePath, @@ -1644,6 +1662,10 @@ export function registerFilesystemHandlers( if (!context) { return { success: false, error: 'No branch changes to summarize.' } } + context = withLinkedIssueDraftContext( + context, + resolveSourceControlAiLinkedIssue(store, args, worktreePath) + ) const localEnv = await prepareLocalCommitMessageAgentEnv( resolvedSettings.params.agentId, commitMessageAgentEnv, diff --git a/src/main/ipc/source-control-ai-linked-issue.test.ts b/src/main/ipc/source-control-ai-linked-issue.test.ts new file mode 100644 index 000000000..1a5291fc6 --- /dev/null +++ b/src/main/ipc/source-control-ai-linked-issue.test.ts @@ -0,0 +1,173 @@ +import path from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import type { Store } from '../persistence' +import { resolveSourceControlAiLinkedIssue } from './source-control-ai-linked-issue' + +const LOCAL_PATH = path.resolve('/workspace/repo-feature') +const LOCAL_ID = `repo-1::${LOCAL_PATH}` +const REMOTE_PATH = '/home/tester/wt' +const REMOTE_ID = `repo-1::${REMOTE_PATH}` + +function makeStore(meta: Record): Store { + return { + getWorktreeMeta: vi.fn((worktreeId: string) => meta[worktreeId]) + } as unknown as Store +} + +describe('resolveSourceControlAiLinkedIssue', () => { + it('reads meta with the raw id when the id matches the request path', () => { + const store = makeStore({ [LOCAL_ID]: { linkedIssue: 123 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: LOCAL_ID, + worktreePath: LOCAL_PATH, + repoId: 'repo-1' + }) + ).toBe(123) + expect(store.getWorktreeMeta).toHaveBeenCalledWith(LOCAL_ID) + }) + + it('keeps the folder-repo instance suffix in the meta key while validating the stripped path', () => { + const instanceId = `${LOCAL_ID}::workspace:${'0'.repeat(8)}-0000-0000-0000-${'0'.repeat(12)}` + const store = makeStore({ [instanceId]: { linkedIssue: 9 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: instanceId, + worktreePath: LOCAL_PATH + }) + ).toBe(9) + expect(store.getWorktreeMeta).toHaveBeenCalledWith(instanceId) + }) + + // Why: the desktop renderer derives `worktreePath` from `worktreeId`, so it can + // never send a mismatched pair — these cases model an independent caller (relay, + // CLI, future in-process caller) and assert the guard fails closed for them. + // They are not evidence that a stale renderer context is rejected; it is not. + it('rejects an independently supplied id whose path does not match the request', () => { + const store = makeStore({ [LOCAL_ID]: { linkedIssue: 123 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: LOCAL_ID, + worktreePath: path.resolve('/workspace/repo-other') + }) + ).toBeNull() + expect(store.getWorktreeMeta).not.toHaveBeenCalled() + }) + + it('accepts the resolved worktree path as an alternate local candidate', () => { + const store = makeStore({ [LOCAL_ID]: { linkedIssue: 5 } }) + + expect( + resolveSourceControlAiLinkedIssue( + store, + { worktreeId: LOCAL_ID, worktreePath: path.resolve('/workspace/symlinked') }, + LOCAL_PATH + ) + ).toBe(5) + }) + + it('rejects an independently supplied id whose repoId contradicts the request repoId', () => { + const store = makeStore({ [LOCAL_ID]: { linkedIssue: 123 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: LOCAL_ID, + worktreePath: LOCAL_PATH, + repoId: 'repo-2' + }) + ).toBeNull() + expect(store.getWorktreeMeta).not.toHaveBeenCalled() + }) + + it('fails closed on an empty-string repoId instead of skipping the cross-check', () => { + const store = makeStore({ [LOCAL_ID]: { linkedIssue: 123 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: LOCAL_ID, + worktreePath: LOCAL_PATH, + repoId: '' + }) + ).toBeNull() + expect(store.getWorktreeMeta).not.toHaveBeenCalled() + }) + + it('compares SSH remote paths as raw strings', () => { + const store = makeStore({ [REMOTE_ID]: { linkedIssue: 77 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: REMOTE_ID, + worktreePath: `${REMOTE_PATH}/`, + connectionId: 'conn-1' + }) + ).toBe(77) + }) + + it('matches SSH remote paths from a Windows host without path rewriting', () => { + const original = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const store = makeStore({ [REMOTE_ID]: { linkedIssue: 77 } }) + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: REMOTE_ID, + worktreePath: REMOTE_PATH, + connectionId: 'conn-1' + }) + ).toBe(77) + } finally { + Object.defineProperty(process, 'platform', original) + } + }) + + it('returns null without touching the store when no id is supplied', () => { + const store = makeStore({ [LOCAL_ID]: { linkedIssue: 123 } }) + + expect(resolveSourceControlAiLinkedIssue(store, { worktreePath: LOCAL_PATH })).toBeNull() + expect(store.getWorktreeMeta).not.toHaveBeenCalled() + }) + + it('tolerates a store without a meta accessor', () => { + expect( + resolveSourceControlAiLinkedIssue({} as Store, { + worktreeId: LOCAL_ID, + worktreePath: LOCAL_PATH + }) + ).toBeNull() + }) + + it('returns null for unparsable ids and unlinked or unusable meta', () => { + expect( + resolveSourceControlAiLinkedIssue(makeStore({}), { + worktreeId: 'no-separator', + worktreePath: LOCAL_PATH + }) + ).toBeNull() + for (const linkedIssue of [null, undefined, Number.NaN, 0, -7, 12.9]) { + expect( + resolveSourceControlAiLinkedIssue(makeStore({ [LOCAL_ID]: { linkedIssue } }), { + worktreeId: LOCAL_ID, + worktreePath: LOCAL_PATH + }) + ).toBeNull() + } + }) + + it('does not fall back to a GitLab-linked issue', () => { + const store = { + getWorktreeMeta: vi.fn(() => ({ linkedIssue: null, linkedGitLabIssue: 456 })) + } as unknown as Store + + expect( + resolveSourceControlAiLinkedIssue(store, { + worktreeId: LOCAL_ID, + worktreePath: LOCAL_PATH + }) + ).toBeNull() + }) +}) diff --git a/src/main/ipc/source-control-ai-linked-issue.ts b/src/main/ipc/source-control-ai-linked-issue.ts new file mode 100644 index 000000000..559d39c73 --- /dev/null +++ b/src/main/ipc/source-control-ai-linked-issue.ts @@ -0,0 +1,83 @@ +import { resolve } from 'node:path' +import type { Store } from '../persistence' +import { isLinkedIssueNumber } from '../../shared/source-control-ai-action-variables' +import { splitWorktreeIdForFilesystem } from '../../shared/worktree-id' + +export type LinkedIssueLookupArgs = { + worktreeId?: string + worktreePath: string + repoId?: string + connectionId?: string +} + +function trimTrailingSeparators(value: string): string { + return value.replace(/[\\/]+$/g, '') +} + +function comparableLocalPath(value: string): string { + const normalized = resolve(value) + return process.platform === 'win32' ? normalized.toLowerCase() : normalized +} + +function matchesRequestPath( + idWorktreePath: string, + args: LinkedIssueLookupArgs, + resolvedWorktreePath: string | undefined +): boolean { + if (args.connectionId) { + // Why: the SSH branch never resolves the path, so it is a remote POSIX string. + // resolve()/case-folding it from a Windows host would rewrite it and never match. + return trimTrailingSeparators(idWorktreePath) === trimTrailingSeparators(args.worktreePath) + } + const candidates = new Set( + [args.worktreePath, resolvedWorktreePath ?? args.worktreePath].map(comparableLocalPath) + ) + return candidates.has(comparableLocalPath(idWorktreePath)) +} + +/** + * Resolve the workspace's linked GitHub issue for Source Control AI generation. + * + * The renderer-supplied `worktreeId` is advisory — the same trust model as + * `getRepoForSourceControlAi` — so it is validated against the request's path + * (and `repoId`) before the meta read. + * + * Scope of that guarantee: today's desktop renderer derives `worktreePath` from + * `worktreeId` (`resolveLocalWorktreePath`), so its two operands always agree and + * this check cannot reject a renderer call — including a stale id after a + * workspace switch, which produces a matching stale *pair* (git then runs in that + * same stale worktree, so the number still belongs to the tree being committed). + * The validation exists for callers that supply id and path independently — a + * relay, the CLI, or a future in-process caller — where a mismatched pair really + * would read another workspace's meta. Keep it: it is cheap and fails closed. + * + * Meta is keyed by the raw id: the `::workspace:` suffix of folder-repo + * workspace instances is part of the key, while validation uses the stripped path. + */ +export function resolveSourceControlAiLinkedIssue( + store: Store, + args: LinkedIssueLookupArgs, + resolvedWorktreePath?: string +): number | null { + if (typeof args.worktreeId !== 'string' || !args.worktreeId) { + return null + } + if (typeof store.getWorktreeMeta !== 'function') { + return null + } + const parsed = splitWorktreeIdForFilesystem(args.worktreeId) + if (!parsed) { + return null + } + // Why: `typeof` rather than truthiness, so an empty-string repoId fails closed + // instead of silently disabling the cross-check. + if (typeof args.repoId === 'string' && parsed.repoId !== args.repoId) { + return null + } + if (!matchesRequestPath(parsed.worktreePath, args, resolvedWorktreePath)) { + return null + } + const linkedIssue = store.getWorktreeMeta(args.worktreeId)?.linkedIssue + // Why: GitHub only in v1 — no `linkedGitLabIssue` dual-read. + return isLinkedIssueNumber(linkedIssue) ? linkedIssue : null +} diff --git a/src/main/runtime/orca-runtime-git.test.ts b/src/main/runtime/orca-runtime-git.test.ts index 07f2cb61d..1103852ed 100644 --- a/src/main/runtime/orca-runtime-git.test.ts +++ b/src/main/runtime/orca-runtime-git.test.ts @@ -60,19 +60,25 @@ vi.mock('../source-control/pull-request-template', () => ({ const tempDirs: string[] = [] -function makeWorktree(path: string): ResolvedRuntimeGitWorktree { - return { +function makeWorktree(path: string, linkedIssue: number | null = null): ResolvedRuntimeGitWorktree { + // Why: `satisfies Partial<…>` keeps every field name and type checked against the + // real worktree shape (the widening cast alone would let these tests keep passing + // against a `linkedIssue` key production no longer has) while still allowing the + // fixture to omit the fields these tests never read. + const worktree = { id: 'wt-1', repoId: 'repo-1', path, + linkedIssue, git: { path, branch: 'main', - bare: false, - detached: false, + isBare: false, + isMainWorktree: false, head: 'a'.repeat(40) } - } as unknown as ResolvedRuntimeGitWorktree + } satisfies Partial + return worktree as unknown as ResolvedRuntimeGitWorktree } function makeCommands(worktreePath: string): RuntimeGitCommands { @@ -588,4 +594,250 @@ describe('RuntimeGitCommands', () => { }) ) }) + + it('enriches the local commit context with the workspace linked issue', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params }) + mocks.getStagedCommitContext.mockResolvedValue(context) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath, 123) }), + getRuntimeSettings: () => ({}) as GlobalSettings + }) + + await commands.generateRuntimeCommitMessage('id:wt-1') + + expect(mocks.generateCommitMessageFromContext).toHaveBeenCalledWith( + { ...context, linkedIssue: 123 }, + params, + expect.objectContaining({ kind: 'local' }) + ) + }) + + it('enriches the SSH commit context with the workspace linked issue', async () => { + const worktreePath = '/home/tester/wt' + const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } + const params = { agentId: 'cursor', model: 'remote-model' } + mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params }) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + mocks.getSshGitProvider.mockReturnValue({ + getStagedCommitContext: vi.fn().mockResolvedValue(context), + executeCommitMessagePlan: vi.fn() + }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ + worktree: makeWorktree(worktreePath, 77), + connectionId: 'conn-1' + }), + getRuntimeSettings: () => ({}) as GlobalSettings + }) + + await commands.generateRuntimeCommitMessage('id:wt-1') + + expect(mocks.generateCommitMessageFromContext).toHaveBeenCalledWith( + { ...context, linkedIssue: 77 }, + params, + expect.objectContaining({ kind: 'remote' }) + ) + }) + + it('prefers live meta over the linked issue projected onto the resolved worktree', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params }) + mocks.getStagedCommitContext.mockResolvedValue(context) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + const getWorktreeLinkedIssue = vi.fn(() => 321) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath, 123) }), + getRuntimeSettings: () => ({}) as GlobalSettings, + getWorktreeLinkedIssue + }) + + await commands.generateRuntimeCommitMessage('id:wt-1') + + expect(getWorktreeLinkedIssue).toHaveBeenCalledWith('wt-1') + expect(mocks.generateCommitMessageFromContext).toHaveBeenCalledWith( + { ...context, linkedIssue: 321 }, + params, + expect.objectContaining({ kind: 'local' }) + ) + }) + + it('drops a stale worktree issue number when live meta reports the workspace unlinked', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } + mocks.resolveCommitMessageSettings.mockReturnValue({ + ok: true, + params: { agentId: 'codex', model: 'gpt-5.4-mini' } + }) + mocks.getStagedCommitContext.mockResolvedValue(context) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath, 123) }), + getRuntimeSettings: () => ({}) as GlobalSettings, + getWorktreeLinkedIssue: () => null + }) + + await commands.generateRuntimeCommitMessage('id:wt-1') + + expect(mocks.generateCommitMessageFromContext.mock.calls[0][0]).not.toHaveProperty( + 'linkedIssue' + ) + }) + + it('keeps the cached issue number when live meta is unavailable rather than unlinked', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params }) + mocks.getStagedCommitContext.mockResolvedValue(context) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath, 123) }), + getRuntimeSettings: () => ({}) as GlobalSettings, + // Why: what the host reports when its store is not initialized yet. + getWorktreeLinkedIssue: () => undefined + }) + + await commands.generateRuntimeCommitMessage('id:wt-1') + + expect(mocks.generateCommitMessageFromContext).toHaveBeenCalledWith( + { ...context, linkedIssue: 123 }, + params, + expect.objectContaining({ kind: 'local' }) + ) + }) + + it('reads pull-request linked issues from live meta too', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { + base: 'main', + branch: 'feature/login', + commitSummary: 'abc123 feat: test', + changeSummary: 'M README.md', + patch: '+hello', + currentTitle: '', + currentBody: '', + currentDraft: false + } + mocks.getPullRequestDraftContext.mockResolvedValue(context) + mocks.generatePullRequestFieldsFromContext.mockResolvedValue({ success: true, fields: {} }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath, 123) }), + getRuntimeSettings: () => ({}) as GlobalSettings, + getWorktreeLinkedIssue: () => 321 + }) + + await commands.generateRuntimePullRequestFields( + 'id:wt-1', + { base: 'main', title: '', body: '', draft: false }, + { sourceControlAiResolvedParams: { agentId: 'codex' as const, model: 'gpt-5.5' } } + ) + + expect(mocks.generatePullRequestFieldsFromContext.mock.calls[0][0]).toEqual({ + ...context, + linkedIssue: 321 + }) + }) + + it('leaves the commit context untouched when no issue is linked', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } + const params = { agentId: 'codex', model: 'gpt-5.4-mini' } + mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params }) + mocks.getStagedCommitContext.mockResolvedValue(context) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + + await makeCommands(worktreePath).generateRuntimeCommitMessage('id:wt-1') + + expect(mocks.generateCommitMessageFromContext.mock.calls[0][0]).not.toHaveProperty( + 'linkedIssue' + ) + }) + + it('shares one linked-issue attach across both pull-request branches', async () => { + const context = { + base: 'main', + branch: 'feature/login', + commitSummary: 'abc123 feat: test', + changeSummary: 'M README.md', + patch: '+hello', + currentTitle: '', + currentBody: '', + currentDraft: false + } + const params = { agentId: 'codex' as const, model: 'gpt-5.5' } + mocks.generatePullRequestFieldsFromContext.mockResolvedValue({ success: true, fields: {} }) + mocks.getSshGitProvider.mockReturnValue({ + exec: vi.fn(), + executeCommitMessagePlan: vi.fn() + }) + + for (const connectionId of [undefined, 'conn-1']) { + const worktreePath = connectionId + ? '/home/tester/wt' + : mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + if (!connectionId) { + tempDirs.push(worktreePath) + } + mocks.getPullRequestDraftContext.mockResolvedValue(context) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ + worktree: makeWorktree(worktreePath, 55), + ...(connectionId ? { connectionId } : {}) + }), + getRuntimeSettings: () => ({}) as GlobalSettings + }) + + await commands.generateRuntimePullRequestFields( + 'id:wt-1', + { base: 'main', title: '', body: '', draft: false }, + { sourceControlAiResolvedParams: params } + ) + } + + expect(mocks.generatePullRequestFieldsFromContext.mock.calls).toHaveLength(2) + for (const call of mocks.generatePullRequestFieldsFromContext.mock.calls) { + expect(call[0]).toEqual({ ...context, linkedIssue: 55 }) + } + }) + + it('leaves the pull-request context untouched when no issue is linked', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-')) + tempDirs.push(worktreePath) + const context = { + base: 'main', + branch: 'feature/login', + commitSummary: 'abc123 feat: test', + changeSummary: 'M README.md', + patch: '+hello', + currentTitle: '', + currentBody: '', + currentDraft: false + } + const params = { agentId: 'codex' as const, model: 'gpt-5.5' } + mocks.getPullRequestDraftContext.mockResolvedValue(context) + mocks.generatePullRequestFieldsFromContext.mockResolvedValue({ success: true, fields: {} }) + + await makeCommands(worktreePath).generateRuntimePullRequestFields( + 'id:wt-1', + { base: 'main', title: '', body: '', draft: false }, + { sourceControlAiResolvedParams: params } + ) + + expect(mocks.generatePullRequestFieldsFromContext.mock.calls).toHaveLength(1) + expect(mocks.generatePullRequestFieldsFromContext.mock.calls[0][0]).not.toHaveProperty( + 'linkedIssue' + ) + }) }) diff --git a/src/main/runtime/orca-runtime-git.ts b/src/main/runtime/orca-runtime-git.ts index 7aaa8ede5..a2dd69844 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -23,6 +23,7 @@ import { mergeLegacyCommitMessageAiIntoSourceControlAi, type ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' +import { withLinkedIssueDraftContext } from '../../shared/source-control-ai-action-variables' import type { SourceControlAiOperation } from '../../shared/source-control-ai-types' import type { GitProviderStatusOptions } from '../providers/types' import { getRemoteCommitUrl, getRemoteFileUrl } from '../git/repo' @@ -160,11 +161,26 @@ export type RuntimeGitCommandHost = { resolveRuntimeGitTarget(selector: string): Promise getRuntimeSettings(): GlobalSettings getCommitMessageAgentEnvironment?(): CommitMessageAgentEnvironmentResolvers | undefined + /** + * Live linked-issue read by worktree id. Resolved worktrees come from a + * short-TTL cache, so link/unlink would otherwise lag generation; hosts that + * implement this are authoritative, including the `null` unlinked answer. + * Return `undefined` when metadata is unavailable (store not ready) so the + * caller keeps the resolved worktree's cached value instead of reading it as + * unlinked. + */ + getWorktreeLinkedIssue?(worktreeId: string): number | null | undefined } export class RuntimeGitCommands { constructor(private readonly host: RuntimeGitCommandHost) {} + private linkedIssueForTarget(target: RuntimeGitTarget): number | null | undefined { + const live = this.host.getWorktreeLinkedIssue?.(target.worktree.id) + // Why: `undefined` means the host could not answer, not "unlinked". + return live === undefined ? target.worktree.linkedIssue : live + } + async getRuntimeGitStatus( worktreeSelector: string, options?: GitProviderStatusOptions @@ -615,6 +631,7 @@ export class RuntimeGitCommands { if (!context) { return { success: false, error: 'No staged changes to summarize.' } } + context = withLinkedIssueDraftContext(context, this.linkedIssueForTarget(target)) return generateCommitMessageFromContext(context, resolvedSettings.params, { kind: 'remote', cwd: target.worktree.path, @@ -634,6 +651,7 @@ export class RuntimeGitCommands { if (!context) { return { success: false, error: 'No staged changes to summarize.' } } + context = withLinkedIssueDraftContext(context, this.linkedIssueForTarget(target)) const localEnv = await prepareLocalCommitMessageAgentEnv( resolvedSettings.params.agentId, this.host.getCommitMessageAgentEnvironment?.(), @@ -738,6 +756,8 @@ export class RuntimeGitCommands { if (!context) { return { success: false, error: 'No branch changes to summarize.' } } + // Why: both SSH and local branches share this context, so one attach covers each. + context = withLinkedIssueDraftContext(context, this.linkedIssueForTarget(target)) if (target.connectionId) { return generatePullRequestFieldsFromContext(context, resolvedSettings.params, { diff --git a/src/main/runtime/orca-runtime-linked-issue-live-meta.integration.test.ts b/src/main/runtime/orca-runtime-linked-issue-live-meta.integration.test.ts new file mode 100644 index 000000000..fea4e93a3 --- /dev/null +++ b/src/main/runtime/orca-runtime-linked-issue-live-meta.integration.test.ts @@ -0,0 +1,141 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { WorktreeMeta } from '../../shared/types' +import type * as GitStatusModule from '../git/status' +import type * as CommitMessageTextGenerationModule from '../text-generation/commit-message-text-generation' +import type * as WorktreeModule from '../git/worktree' +import { OrcaRuntimeService } from './orca-runtime' + +const mocks = vi.hoisted(() => ({ + listWorktrees: vi.fn(), + getStagedCommitContext: vi.fn(), + generateCommitMessageFromContext: vi.fn(), + resolveCommitMessageSettings: vi.fn() +})) + +vi.mock('../git/worktree', async () => ({ + ...(await vi.importActual('../git/worktree')), + listWorktrees: mocks.listWorktrees +})) + +vi.mock('../git/status', async () => ({ + ...(await vi.importActual('../git/status')), + getStagedCommitContext: mocks.getStagedCommitContext +})) + +vi.mock('../text-generation/commit-message-text-generation', async () => ({ + ...(await vi.importActual( + '../text-generation/commit-message-text-generation' + )), + generateCommitMessageFromContext: mocks.generateCommitMessageFromContext, + resolveCommitMessageSettings: mocks.resolveCommitMessageSettings +})) + +const REPO_ID = 'repo-1' +const STAGED_CONTEXT = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' } +const PARAMS = { agentId: 'codex', model: 'gpt-5.4-mini' } + +const tempDirs: string[] = [] + +/** + * Store double narrow enough to drive worktree resolution, so the runtime runs + * its real hydration (`listResolvedWorktrees` → `mergeWorktree` → cache) instead + * of a hand-built worktree fixture carrying `linkedIssue`. + */ +function makeStore(worktreePath: string) { + const worktreeId = `${REPO_ID}::${worktreePath}` + const worktreeMeta: Record = { + [worktreeId]: { + instanceId: worktreeId, + displayName: 'wt', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } + } + return { + worktreeId, + updateLinkedIssue: (linkedIssue: number | null): void => { + // Why: mirrors `worktrees:updateMeta`, which persists the link without + // invalidating the runtime's resolved-worktree cache. + worktreeMeta[worktreeId] = { ...worktreeMeta[worktreeId], linkedIssue } + }, + store: { + getRepos: () => [ + { id: REPO_ID, path: worktreePath, displayName: 'repo', badgeColor: 'blue', addedAt: 1 } + ], + getRepo: (id: string) => + id === REPO_ID + ? { id: REPO_ID, path: worktreePath, displayName: 'repo', badgeColor: 'blue', addedAt: 1 } + : undefined, + getAllWorktreeMeta: () => worktreeMeta, + getWorktreeMeta: (id: string) => worktreeMeta[id], + setWorktreeMeta: (id: string, updates: Partial) => { + worktreeMeta[id] = { ...worktreeMeta[id], ...updates } + return worktreeMeta[id] + }, + getSettings: () => ({}) + } + } +} + +async function generatedCommitContext( + runtime: OrcaRuntimeService, + worktreeId: string +): Promise> { + mocks.generateCommitMessageFromContext.mockClear() + await runtime.generateRuntimeCommitMessage(`id:${worktreeId}`) + return mocks.generateCommitMessageFromContext.mock.calls[0][0] +} + +describe('runtime commit-message generation linked-issue freshness', () => { + beforeEach(() => { + mocks.listWorktrees.mockReset() + mocks.getStagedCommitContext.mockReset() + mocks.generateCommitMessageFromContext.mockReset() + mocks.resolveCommitMessageSettings.mockReset() + mocks.getStagedCommitContext.mockResolvedValue(STAGED_CONTEXT) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' }) + mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params: PARAMS }) + }) + + afterEach(() => { + while (tempDirs.length > 0) { + rmSync(tempDirs.pop()!, { recursive: true, force: true }) + } + }) + + it('substitutes the linked issue persisted since the last worktree resolution', async () => { + const worktreePath = mkdtempSync(join(tmpdir(), 'orca-linked-issue-')) + tempDirs.push(worktreePath) + const { store, worktreeId, updateLinkedIssue } = makeStore(worktreePath) + mocks.listWorktrees.mockResolvedValue([ + { + path: worktreePath, + head: 'a'.repeat(40), + branch: 'main', + isBare: false, + isMainWorktree: true + } + ]) + const runtime = new OrcaRuntimeService(store as never) + + // Why: the first generation warms the resolved-worktree cache with the + // unlinked projection, so a stale read would still answer `unlinked` below. + expect(await generatedCommitContext(runtime, worktreeId)).not.toHaveProperty('linkedIssue') + + updateLinkedIssue(321) + expect(await generatedCommitContext(runtime, worktreeId)).toMatchObject({ linkedIssue: 321 }) + + updateLinkedIssue(null) + expect(await generatedCommitContext(runtime, worktreeId)).not.toHaveProperty('linkedIssue') + }) +}) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index b24f59498..d3a9cb257 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -7062,7 +7062,18 @@ export class OrcaRuntimeService { private readonly gitCommands = new RuntimeGitCommands({ resolveRuntimeGitTarget: (selector) => this.resolveRuntimeGitTarget(selector), getRuntimeSettings: () => this.requireStore().getSettings() as GlobalSettings, - getCommitMessageAgentEnvironment: () => this.commitMessageAgentEnv ?? undefined + getCommitMessageAgentEnvironment: () => this.commitMessageAgentEnv ?? undefined, + // Why: resolved worktrees are cached for a second, so link/unlink would lag + // generation; meta is keyed by the same id the resolver returns. + getWorktreeLinkedIssue: (worktreeId) => { + const store = this.store + // Why: an unreadable store is "unknown", not "unlinked" — undefined keeps + // the resolver's cached linkedIssue instead of suppressing {linkedIssue}. + if (!store?.getWorktreeMeta) { + return undefined + } + return store.getWorktreeMeta(worktreeId)?.linkedIssue ?? null + } }) getRuntimeGitStatus: RuntimeGitCommands['getRuntimeGitStatus'] = diff --git a/src/main/text-generation/commit-message-text-generation.test.ts b/src/main/text-generation/commit-message-text-generation.test.ts index 3b921daf6..13feda400 100644 --- a/src/main/text-generation/commit-message-text-generation.test.ts +++ b/src/main/text-generation/commit-message-text-generation.test.ts @@ -1888,6 +1888,168 @@ describe('generateBranchNameFromContext', () => { }) }) +describe('linkedIssue template substitution', () => { + const COMMIT_CONTEXT = { + branch: 'feature/login', + stagedSummary: 'M src/login.ts', + stagedPatch: 'diff --git a/src/login.ts b/src/login.ts' + } + const PULL_REQUEST_CONTEXT = { + branch: 'feature/login', + base: 'main', + branchChangedByPreparation: false, + currentTitle: 'Fix login', + currentBody: '', + currentDraft: false, + commitSummary: 'a1b2c3d Fix login', + changeSummary: 'src/login.ts | 4 ++--', + patch: 'diff --git a/src/login.ts b/src/login.ts' + } + + function capturingTarget(capture: (prompt: string) => void): { + kind: 'remote' + cwd: string + missingBinaryLocation: string + execute: (plan: { stdinPayload: string | null }) => Promise<{ + stdout: string + stderr: string + exitCode: number + timedOut: boolean + }> + } { + return { + kind: 'remote', + cwd: '/repo', + missingBinaryLocation: 'remote PATH', + execute: async (plan) => { + capture(plan.stdinPayload ?? '') + return { + stdout: '{"base":"main","title":"Fix login","body":"body","draft":false}', + stderr: '', + exitCode: 0, + timedOut: false + } + } + } + } + + const templateParams = { + agentId: 'custom' as const, + model: '', + customAgentCommand: 'agent', + commandInputTemplate: '{basePrompt}\n\nFixes #{linkedIssue}' + } + + it('substitutes the linked issue into the commit-message prompt', async () => { + let prompt = '' + await generateCommitMessageFromContext( + { ...COMMIT_CONTEXT, linkedIssue: 42 }, + templateParams, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).toContain('Fixes #42') + expect(prompt).not.toContain('{linkedIssue}') + }) + + it('renders an empty commit-message issue for null and omitted fields', async () => { + for (const context of [{ ...COMMIT_CONTEXT, linkedIssue: null }, COMMIT_CONTEXT]) { + let prompt = '' + await generateCommitMessageFromContext( + context, + templateParams, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).toContain('Fixes #') + expect(prompt).not.toContain('{linkedIssue}') + } + }) + + // Why: a fixture-unique sentinel — a short number like 42 also appears in the + // character counts that truncateDiffForPrompt/limitSection emit, so growing any + // fixture past its limit would fail these guards for reasons unrelated to leakage. + const BUILT_IN_PROMPT_SENTINEL_ISSUE = 987654 + const builtInPromptParams = { + agentId: 'custom' as const, + model: '', + customAgentCommand: 'agent' + } + + it('leaves the built-in commit prompt free of issue guidance', async () => { + let prompt = '' + await generateCommitMessageFromContext( + { ...COMMIT_CONTEXT, linkedIssue: BUILT_IN_PROMPT_SENTINEL_ISSUE }, + builtInPromptParams, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).not.toContain(String(BUILT_IN_PROMPT_SENTINEL_ISSUE)) + expect(prompt).not.toContain('linkedIssue') + }) + + it('leaves the built-in pull-request prompt free of issue guidance', async () => { + let prompt = '' + await generatePullRequestFieldsFromContext( + { ...PULL_REQUEST_CONTEXT, linkedIssue: BUILT_IN_PROMPT_SENTINEL_ISSUE }, + builtInPromptParams, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).not.toContain(String(BUILT_IN_PROMPT_SENTINEL_ISSUE)) + expect(prompt).not.toContain('linkedIssue') + }) + + it('substitutes the linked issue into the pull-request prompt', async () => { + let prompt = '' + await generatePullRequestFieldsFromContext( + { ...PULL_REQUEST_CONTEXT, linkedIssue: 7 }, + templateParams, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).toContain('Fixes #7') + expect(prompt).not.toContain('{linkedIssue}') + }) + + it('renders an empty pull-request issue when none resolves', async () => { + let prompt = '' + await generatePullRequestFieldsFromContext( + PULL_REQUEST_CONTEXT, + templateParams, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).toContain('Fixes #') + expect(prompt).not.toContain('{linkedIssue}') + }) + + it('leaves a hand-typed linkedIssue literal in branch-name templates', async () => { + let prompt = '' + await generateBranchNameFromContext( + { firstPrompt: 'Fix login flow' }, + { ...templateParams, commandInputTemplate: '{basePrompt}\n\nIssue {linkedIssue}' }, + capturingTarget((value) => { + prompt = value + }) + ) + + expect(prompt).toContain('Issue {linkedIssue}') + }) +}) + describe('trimGeneratedCommitMessage', () => { it('removes trailing whitespace from generated messages', () => { const message = trimGeneratedCommitMessage('Update docs\n\n') diff --git a/src/main/text-generation/commit-message-text-generation.ts b/src/main/text-generation/commit-message-text-generation.ts index ff0c9e5fc..fde91c1f7 100644 --- a/src/main/text-generation/commit-message-text-generation.ts +++ b/src/main/text-generation/commit-message-text-generation.ts @@ -44,6 +44,7 @@ import { type ResolvedSourceControlAiGenerationParams } from '../../shared/source-control-ai' import type { SourceControlAiOperation } from '../../shared/source-control-ai-types' +import { formatLinkedIssueTemplateValue } from '../../shared/source-control-ai-action-variables' import { renderSourceControlActionCommandTemplate } from '../../shared/source-control-ai-actions' import { resolveCliCommand } from '../codex-cli/command' import { @@ -876,7 +877,9 @@ export async function generateCommitMessageFromContext( basePrompt, branch: context.branch ?? '(detached)', stagedFiles: context.stagedSummary, - stagedPatch: context.stagedPatch + stagedPatch: context.stagedPatch, + // Why: always pass the key so `{linkedIssue}` never survives as a literal token. + linkedIssue: formatLinkedIssueTemplateValue(context.linkedIssue) }) : buildCommitMessagePrompt(context, params.customPrompt ?? '') const planned = planCommitMessageGeneration(params, prompt) @@ -947,7 +950,9 @@ export async function generatePullRequestFieldsFromContext( currentBody: context.currentBody, commitSummary: context.commitSummary, changedFiles: context.changeSummary, - patch: context.patch + patch: context.patch, + // Why: always pass the key so `{linkedIssue}` never survives as a literal token. + linkedIssue: formatLinkedIssueTemplateValue(context.linkedIssue) }) : buildPullRequestFieldsPrompt(context, params.customPrompt ?? '') const planned = planCommitMessageGeneration(params, prompt) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index b2d78e851..b72739e98 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2744,6 +2744,8 @@ export type PreloadApi = { }) => Promise<{ success: boolean; error?: string }> generateCommitMessage: (args: { worktreePath: string + /** Raw (unstripped) worktree meta key; validated against worktreePath in main. */ + worktreeId?: string repoId?: string connectionId?: string sourceControlAiResolvedParams?: ResolvedSourceControlAiGenerationParams @@ -2772,6 +2774,8 @@ export type PreloadApi = { }) => Promise generatePullRequestFields: (args: { worktreePath: string + /** Raw (unstripped) worktree meta key; validated against worktreePath in main. */ + worktreeId?: string repoId?: string base: string title: string diff --git a/src/preload/index.ts b/src/preload/index.ts index 009924197..7de16fc46 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -3137,6 +3137,7 @@ const api = { }): Promise<{ success: boolean; error?: string }> => ipcRenderer.invoke('git:commit', args), generateCommitMessage: (args: { worktreePath: string + worktreeId?: string repoId?: string connectionId?: string sourceControlAiResolvedParams?: unknown @@ -3154,6 +3155,7 @@ const api = { }): Promise => ipcRenderer.invoke('git:cancelGenerateCommitMessage', args), generatePullRequestFields: (args: { worktreePath: string + worktreeId?: string repoId?: string base: string title: string diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 6fea1c659..9887b10da 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -6323,6 +6323,7 @@ function SourceControlInner(): React.JSX.Element { settings={settings} repo={activeRepo ?? null} discoveryHostKey={sourceControlAiDiscoveryHostKey} + linkedIssue={activeWorktree?.linkedIssue ?? null} onGenerate={(params) => { void handleGenerate({ sourceControlAiResolvedParams: params }) }} @@ -6344,6 +6345,7 @@ function SourceControlInner(): React.JSX.Element { settings={settings} repo={activeRepo ?? null} discoveryHostKey={sourceControlAiDiscoveryHostKey} + linkedIssue={activeWorktree?.linkedIssue ?? null} onGenerate={(params) => { void handleGeneratePullRequestFields({ sourceControlAiResolvedParams: params }) }} diff --git a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts index f17eaea08..1af9d5d8b 100644 --- a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts +++ b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts @@ -38,6 +38,37 @@ vi.mock('@/components/ui/select', () => ({ SelectValue: () => React.createElement('span') })) +/** The mocked chips serialize their previews into an attribute, so quotes arrive escaped. */ +function escapeHtml(value: string): string { + return value.replace(/"/g, '"') +} + +function renderTextGenerationForm(input: { + commandInputTemplate: string + linkedIssue: number | null +}): string { + return renderToStaticMarkup( + React.createElement(SourceControlTextGenerationDialogForm, { + actionId: 'commitMessage', + generateLabel: 'Generate commit message', + settings: null, + repo: null, + baseParams: { + agentId: 'codex', + model: 'gpt-5.4-mini', + commandInputTemplate: input.commandInputTemplate + }, + linkedIssue: input.linkedIssue, + saveTargets: [ + { target: { type: 'global' }, label: 'Save as global default', successMessage: '' } + ], + onGenerate: () => {}, + onOpenChange: () => {}, + onSaveDefaults: () => {} + }) + ) +} + describe('buildCommitMessageGenerationParams', () => { it('defaults saved text-generation recipes to the global target when repo and global are available', () => { expect( @@ -79,6 +110,82 @@ describe('buildCommitMessageGenerationParams', () => { expect(markup).toContain('You are generating a single git commit message.') }) + // Why: with no preview the chips fall back to the synthetic `123`, promising an unlinked + // workspace `Fixes #123` where it renders `Fixes #`. + it.each([ + { label: 'the linked issue', linkedIssue: 42, expected: '42' }, + { label: 'an empty value when unlinked', linkedIssue: null, expected: '' } + ])('previews $label for a workspace-scoped dialog', ({ linkedIssue, expected }) => { + const markup = renderToStaticMarkup( + React.createElement(SourceControlTextGenerationDialogForm, { + actionId: 'commitMessage', + generateLabel: 'Generate', + settings: null, + repo: null, + baseParams: { + agentId: 'codex', + model: 'gpt-5.4-mini', + commandInputTemplate: '{basePrompt}' + }, + linkedIssue, + saveTargets: [], + onGenerate: () => {}, + onOpenChange: () => {}, + onSaveDefaults: () => {} + }) + ) + + expect(markup).toContain(escapeHtml(JSON.stringify({ linkedIssue: expected }))) + }) + + it('omits the issue preview when no workspace is in scope', () => { + const markup = renderToStaticMarkup( + React.createElement(SourceControlTextGenerationDialogForm, { + actionId: 'commitMessage', + generateLabel: 'Generate', + settings: null, + repo: null, + baseParams: { + agentId: 'codex', + model: 'gpt-5.4-mini', + commandInputTemplate: '{basePrompt}' + }, + saveTargets: [], + onGenerate: () => {}, + onOpenChange: () => {}, + onSaveDefaults: () => {} + }) + ) + + expect(markup).not.toContain('linkedIssue') + }) + + // Why: the recipe saves repo- or globally scoped, so gating on the active workspace's + // empty `{linkedIssue}` would block a global write. + it.each([ + { label: 'a linked workspace', linkedIssue: 42 }, + { label: 'an unlinked workspace', linkedIssue: null } + ])('keeps a bare {linkedIssue} recipe runnable from $label', ({ linkedIssue }) => { + const markup = renderTextGenerationForm({ + commandInputTemplate: '{linkedIssue}', + linkedIssue + }) + + // Why: the "Command input is empty." copy is click-driven state, so `disabled=""` is the + // only gate static markup can observe. + expect(markup).toContain('Save defaults') + expect(markup).toContain('Generate commit message') + expect(markup).not.toContain('disabled=""') + }) + + it('still disables both actions for a template that renders empty for everyone', () => { + // Why: negative control — without it the two cases above pass even if the buttons could + // never disable at all. + const markup = renderTextGenerationForm({ commandInputTemplate: ' ', linkedIssue: 42 }) + + expect(markup).toContain('disabled=""') + }) + it('preserves the resolved model and thinking level for the selected agent', () => { expect( buildCommitMessageGenerationParams({ diff --git a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.tsx b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.tsx index f3bb32767..56c3e52f8 100644 --- a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.tsx @@ -31,6 +31,8 @@ type SourceControlTextGenerationBaseDialogProps = { settings: GlobalSettings | null repo?: Pick | null discoveryHostKey: string + /** Omitted by workspace-less callers (Settings dry-run); `null` means "linked to nothing". */ + linkedIssue?: number | null onGenerate: (params: ResolvedSourceControlAiGenerationParams) => void onSaveDefaults: ( target: SourceControlAiWriteTarget, @@ -89,6 +91,7 @@ export function SourceControlTextGenerationDialog({ settings, repo, discoveryHostKey, + linkedIssue, onGenerate, onSaveDefaults }: SourceControlTextGenerationDialogProps): React.JSX.Element { @@ -179,6 +182,7 @@ export function SourceControlTextGenerationDialog({ repo={repo ?? null} baseParams={baseParams} basePromptPreview={buildBasePromptPreview(actionId)} + linkedIssue={linkedIssue} saveTargets={saveTargets} onGenerate={onGenerate} onOpenChange={onOpenChange} diff --git a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialogForm.tsx b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialogForm.tsx index 7315ba662..dc59ac3da 100644 --- a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialogForm.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialogForm.tsx @@ -19,6 +19,7 @@ import { listCommitMessageAgentCapabilities } from '../../../../shared/commit-message-agent-spec' import type { ResolvedSourceControlAiGenerationParams } from '../../../../shared/source-control-ai' +import { formatLinkedIssueTemplateValue } from '../../../../shared/source-control-ai-action-variables' import type { SourceControlTextActionId } from '../../../../shared/source-control-ai-actions' import type { SourceControlAiWriteTarget } from '../../../../shared/source-control-ai-recipe-save' import type { GlobalSettings, Repo, TuiAgent } from '../../../../shared/types' @@ -46,6 +47,8 @@ type SourceControlTextGenerationDialogFormProps = { repo: Pick | null baseParams: ResolvedSourceControlAiGenerationParams | null basePromptPreview?: string + /** Omitted by workspace-less callers (Settings dry-run); `null` means "linked to nothing". */ + linkedIssue?: number | null saveTargets: SourceControlTextGenerationSaveTarget[] onGenerate: (params: ResolvedSourceControlAiGenerationParams) => void onOpenChange: (open: boolean) => void @@ -80,6 +83,7 @@ export function SourceControlTextGenerationDialogForm({ repo, baseParams, basePromptPreview, + linkedIssue, saveTargets, onGenerate, onOpenChange, @@ -114,6 +118,18 @@ export function SourceControlTextGenerationDialogForm({ settings, customAgentCommand: baseParams?.customAgentCommand }) + // Why: chip previews only. The plan below stays synthetic so a workspace-empty + // `{linkedIssue}` cannot disable Save/Generate for a repo- or global-scoped recipe. + const variablePreviews = useMemo(() => { + const previews: Record = {} + if (basePromptPreview) { + previews.basePrompt = basePromptPreview + } + if (linkedIssue !== undefined) { + previews.linkedIssue = formatLinkedIssueTemplateValue(linkedIssue) + } + return Object.keys(previews).length > 0 ? previews : undefined + }, [basePromptPreview, linkedIssue]) const paramsPlanResult = params ? planSourceControlTextGeneration(actionId, params) : null const canRunGeneration = Boolean(params && paramsPlanResult?.ok) const saving = savingTargetKey !== null @@ -287,7 +303,7 @@ export function SourceControlTextGenerationDialogForm({ /> { const separator = commandTemplate.endsWith('\n') || commandTemplate.length === 0 ? '' : ' ' diff --git a/src/renderer/src/components/source-control/SourceControlActionVariableChips.test.tsx b/src/renderer/src/components/source-control/SourceControlActionVariableChips.test.tsx index 13e13eb50..8a3681925 100644 --- a/src/renderer/src/components/source-control/SourceControlActionVariableChips.test.tsx +++ b/src/renderer/src/components/source-control/SourceControlActionVariableChips.test.tsx @@ -32,4 +32,41 @@ describe('SourceControlActionVariableChips', () => { expect(markup).toContain('overflow-y-auto') expect(markup).toContain('Generate a commit message.') }) + + // Why: the description is the only in-product warning that a bare `Fixes #{linkedIssue}` + // degrades to `Fixes #`, and the only place a GitLab user learns why it is always empty. + // A workspace preview must add to it, never replace it. + it.each([ + { label: 'a linked workspace', preview: '4242', expected: '4242' }, + { label: 'an unlinked workspace', preview: '', expected: '(empty)' } + ])( + 'keeps the linkedIssue description and example alongside $label preview', + ({ preview, expected }) => { + const markup = renderToStaticMarkup( + {}} + /> + ) + + expect(markup).toContain('Empty when no GitHub issue is linked') + expect(markup).toContain('GitLab-linked') + expect(markup).toContain('Example') + expect(markup).toContain('This workspace') + expect(markup).toContain(expected) + } + ) + + it('shows only the rendered prompt for a basePrompt preview', () => { + const markup = renderToStaticMarkup( + {}} + /> + ) + + expect(markup).not.toContain('built-in prompt for this action') + }) }) diff --git a/src/renderer/src/components/source-control/SourceControlActionVariableChips.tsx b/src/renderer/src/components/source-control/SourceControlActionVariableChips.tsx index b6ee822ed..7a99c800e 100644 --- a/src/renderer/src/components/source-control/SourceControlActionVariableChips.tsx +++ b/src/renderer/src/components/source-control/SourceControlActionVariableChips.tsx @@ -3,8 +3,9 @@ import { Braces } from 'lucide-react' import { SOURCE_CONTROL_ACTION_VARIABLE_INFO, SOURCE_CONTROL_ACTION_VARIABLES, - type SourceControlActionId -} from '../../../../shared/source-control-ai-actions' + type SourceControlActionVariable +} from '../../../../shared/source-control-ai-action-variables' +import type { SourceControlActionId } from '../../../../shared/source-control-ai-actions' import { Button } from '../ui/button' import { HoverCard, HoverCardContent, HoverCardTrigger } from '../ui/hover-card' import { translate } from '@/i18n/i18n' @@ -28,37 +29,46 @@ function hasVariablePreview( ) } +function SourceControlVariableSample({ + label, + value +}: { + label: string + value: string +}): React.JSX.Element { + return ( +
+
+ {label} +
+ 
+        {value ||
+          translate(
+            'auto.components.source.control.SourceControlActionVariableChips.4bf6d88039',
+            '(empty)'
+          )}
+
+
+ ) +} + function SourceControlVariableDetails({ variable, preview }: { - variable: string + variable: SourceControlActionVariable preview?: string }): React.JSX.Element { - if (preview !== undefined) { - if (variable === 'basePrompt') { - return ( - 
-          {preview ||
-            translate(
-              'auto.components.source.control.SourceControlActionVariableChips.4bf6d88039',
-              '(empty)'
-            )}
-
- ) - } - + // Why: for `basePrompt` the preview *is* the content — the static card only restates it. + if (preview !== undefined && variable === 'basePrompt') { return ( -
-
{`{${variable}}`}
- 
-          {preview ||
-            translate(
-              'auto.components.source.control.SourceControlActionVariableChips.4bf6d88039',
-              '(empty)'
-            )}
-
-
+ 
+        {preview ||
+          translate(
+            'auto.components.source.control.SourceControlActionVariableChips.4bf6d88039',
+            '(empty)'
+          )}
+
) } @@ -69,17 +79,24 @@ function SourceControlVariableDetails({
{`{${variable}}`}
{info.description}
-
-
- {translate( - 'auto.components.source.control.SourceControlActionVariableChips.6b921a0ac2', - 'Example' + + {/* Why: a preview never replaces the description — it is the only in-product warning + that a bare `Fixes #{linkedIssue}` degrades to `Fixes #` on an unlinked workspace. */} + {preview !== undefined ? ( + - 
-          {info.example}
-
-
+ value={preview} + /> + ) : null}
) } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index f73ec3513..2b60a1d4e 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -3680,7 +3680,8 @@ "SourceControlActionVariableChips": { "1b77798d5f": "Variables", "6b921a0ac2": "Example", - "4bf6d88039": "(empty)" + "4bf6d88039": "(empty)", + "7377483644": "This workspace" } } }, diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 7bd234766..73082fa62 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -3657,7 +3657,8 @@ "SourceControlActionVariableChips": { "1b77798d5f": "Variables", "6b921a0ac2": "Ejemplo", - "4bf6d88039": "(vacío)" + "4bf6d88039": "(vacío)", + "7377483644": "Este espacio de trabajo" } } }, diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index e45b172ac..7834b7fc8 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -3657,7 +3657,8 @@ "SourceControlActionVariableChips": { "1b77798d5f": "変数", "6b921a0ac2": "例", - "4bf6d88039": "（空の）" + "4bf6d88039": "（空の）", + "7377483644": "このワークスペース" } } }, diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index d032b5bdb..349a383ba 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -3657,7 +3657,8 @@ "SourceControlActionVariableChips": { "1b77798d5f": "변수", "6b921a0ac2": "예", - "4bf6d88039": "(비어 있음)" + "4bf6d88039": "(비어 있음)", + "7377483644": "이 워크스페이스" } } }, diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index cd83fea80..c33b1dd21 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -3657,7 +3657,8 @@ "SourceControlActionVariableChips": { "1b77798d5f": "变量", "6b921a0ac2": "例子", - "4bf6d88039": "（空的）" + "4bf6d88039": "（空的）", + "7377483644": "此工作区" } } }, diff --git a/src/renderer/src/lib/source-control-generation-plan.test.ts b/src/renderer/src/lib/source-control-generation-plan.test.ts index a51e12a1d..e769c3f3e 100644 --- a/src/renderer/src/lib/source-control-generation-plan.test.ts +++ b/src/renderer/src/lib/source-control-generation-plan.test.ts @@ -50,6 +50,44 @@ describe('planSourceControlCommitMessageGeneration', () => { expect(result.ok && result.commandLabel).toContain('codex exec') }) + it('expands linkedIssue when validating commit and pull-request recipes', () => { + for (const actionId of ['commitMessage', 'pullRequest'] as const) { + // Why: `{prompt}` puts the rendered template in argv, so commandLabel shows it. + const result = planSourceControlTextGeneration(actionId, { + agentId: 'custom', + model: '', + customAgentCommand: 'echo {prompt}', + commandInputTemplate: 'Fixes #{linkedIssue}' + }) + + expect(result.ok && result.commandLabel).toBe('echo Fixes #123') + } + }) + + it('accepts a bare {linkedIssue} recipe, which is only empty per workspace', () => { + // Why: the recipe is saved repo- or globally scoped, so validation must not depend on + // whichever workspace happens to be active — an unlinked one must not fail the plan. + const result = planSourceControlTextGeneration('commitMessage', { + agentId: 'custom', + model: '', + customAgentCommand: 'echo {prompt}', + commandInputTemplate: '{linkedIssue}' + }) + + expect(result).toEqual(expect.objectContaining({ ok: true, commandLabel: 'echo 123' })) + }) + + it('leaves linkedIssue literal when validating branch-name recipes', () => { + const result = planSourceControlTextGeneration('branchName', { + agentId: 'custom', + model: '', + customAgentCommand: 'echo {prompt}', + commandInputTemplate: 'issue {linkedIssue}' + }) + + expect(result.ok && result.commandLabel).toBe('echo issue {linkedIssue}') + }) + it('shows per-action CLI arguments in dry-run command labels', () => { const result = planSourceControlTextGeneration('pullRequest', { agentId: 'codex', diff --git a/src/renderer/src/lib/source-control-generation-plan.ts b/src/renderer/src/lib/source-control-generation-plan.ts index 07b992b68..a813129e5 100644 --- a/src/renderer/src/lib/source-control-generation-plan.ts +++ b/src/renderer/src/lib/source-control-generation-plan.ts @@ -23,7 +23,8 @@ const SYNTHETIC_TEXT_GENERATION_CONTEXT: Record< basePrompt: SYNTHETIC_COMMIT_PROMPT, branch: 'feature/example', stagedFiles: 'M src/example.ts', - stagedPatch: 'diff --git a/src/example.ts b/src/example.ts' + stagedPatch: 'diff --git a/src/example.ts b/src/example.ts', + linkedIssue: '123' }, pullRequest: { basePrompt: SYNTHETIC_PULL_REQUEST_PROMPT, @@ -33,7 +34,8 @@ const SYNTHETIC_TEXT_GENERATION_CONTEXT: Record< currentBody: 'Draft description', commitSummary: 'a1b2c3d Add source-control AI recipes', changedFiles: 'src/example.ts | 12 ++++++++++--', - patch: 'diff --git a/src/example.ts b/src/example.ts' + patch: 'diff --git a/src/example.ts b/src/example.ts', + linkedIssue: '123' }, branchName: { basePrompt: 'Generate a git branch name for a synthetic task.', @@ -48,6 +50,12 @@ const SYNTHETIC_BASE_PROMPTS: Record = { branchName: 'Generate a git branch name for a synthetic task.' } +/** + * Validates a recipe, so it always renders against the synthetic context above. + * Workspace values must not leak in here: the recipe is saved repo- or globally + * scoped, and gating it on the active workspace's `{linkedIssue}` would disable + * Save/Generate for a template that is not actually empty. + */ export function planSourceControlTextGeneration( actionId: SourceControlTextActionId, params: ResolvedSourceControlAiGenerationParams diff --git a/src/renderer/src/runtime/runtime-git-client.test.ts b/src/renderer/src/runtime/runtime-git-client.test.ts index 78ed0853a..36bd5d1ae 100644 --- a/src/renderer/src/runtime/runtime-git-client.test.ts +++ b/src/renderer/src/runtime/runtime-git-client.test.ts @@ -8,6 +8,7 @@ import { fastForwardRuntimeGit, fetchRuntimeGit, generateRuntimeCommitMessage, + generateRuntimePullRequestFields, getRuntimeGitDiff, getRuntimeGitHistory, getRuntimeGitIgnoredPaths, @@ -36,6 +37,7 @@ const gitFastForward = vi.fn() const gitPush = vi.fn() const gitRebaseFromBase = vi.fn() const gitGenerateCommitMessage = vi.fn() +const gitGeneratePullRequestFields = vi.fn() const gitDiscoverCommitMessageModels = vi.fn() const gitCancelGenerateCommitMessage = vi.fn() const runtimeEnvironmentCall = vi.fn() @@ -59,6 +61,7 @@ beforeEach(() => { gitPush.mockReset() gitRebaseFromBase.mockReset() gitGenerateCommitMessage.mockReset() + gitGeneratePullRequestFields.mockReset() gitDiscoverCommitMessageModels.mockReset() gitCancelGenerateCommitMessage.mockReset() runtimeEnvironmentCall.mockReset() @@ -84,6 +87,7 @@ beforeEach(() => { push: gitPush, rebaseFromBase: gitRebaseFromBase, generateCommitMessage: gitGenerateCommitMessage, + generatePullRequestFields: gitGeneratePullRequestFields, discoverCommitMessageModels: gitDiscoverCommitMessageModels, cancelGenerateCommitMessage: gitCancelGenerateCommitMessage }, @@ -691,6 +695,7 @@ describe('runtime git client', () => { expect(gitGenerateCommitMessage).toHaveBeenCalledWith({ worktreePath: '/repo', + worktreeId: 'repo-1::/repo', repoId: 'repo-1', connectionId: undefined, sourceControlAiResolvedParams @@ -733,4 +738,42 @@ describe('runtime git client', () => { }) expect(gitDiscoverCommitMessageModels).not.toHaveBeenCalled() }) + + it('passes the raw worktree id to local generation IPC', async () => { + // Why: the meta key keeps the `::workspace:` suffix that the cwd path strips. + const workspaceId = '123e4567-e89b-12d3-a456-426614174000' + const worktreeId = `folder-repo::/home/user::workspace:${workspaceId}` + const context = { + settings: { activeRuntimeEnvironmentId: null }, + worktreeId, + worktreePath: `/home/user::workspace:${workspaceId}` + } + + await generateRuntimeCommitMessage(context) + await generateRuntimePullRequestFields(context, { + base: 'main', + title: '', + body: '', + draft: false + }) + + expect(gitGenerateCommitMessage).toHaveBeenCalledWith( + expect.objectContaining({ worktreeId, worktreePath: '/home/user' }) + ) + expect(gitGeneratePullRequestFields).toHaveBeenCalledWith( + expect.objectContaining({ worktreeId, worktreePath: '/home/user' }) + ) + }) + + it('omits worktreeId from local generation IPC when the context has none', async () => { + const context = { + settings: { activeRuntimeEnvironmentId: null }, + worktreeId: null, + worktreePath: '/repo' + } + + await generateRuntimeCommitMessage(context) + + expect(gitGenerateCommitMessage.mock.calls[0][0]).not.toHaveProperty('worktreeId') + }) }) diff --git a/src/renderer/src/runtime/runtime-git-client.ts b/src/renderer/src/runtime/runtime-git-client.ts index 019eaef50..2398252af 100644 --- a/src/renderer/src/runtime/runtime-git-client.ts +++ b/src/renderer/src/runtime/runtime-git-client.ts @@ -648,6 +648,8 @@ export async function generateRuntimeCommitMessage( if (target.kind === 'local' || !context.worktreeId) { return window.api.git.generateCommitMessage({ worktreePath: resolveLocalWorktreePath(context), + // Why: raw id — the `::workspace:` suffix is part of the worktree meta key. + ...(context.worktreeId ? { worktreeId: context.worktreeId } : {}), repoId: context.worktreeId ? getRepoIdFromWorktreeId(context.worktreeId) : undefined, connectionId: context.connectionId, ...(overrides?.sourceControlAiResolvedParams @@ -727,6 +729,8 @@ export async function generateRuntimePullRequestFields( if (target.kind === 'local' || !context.worktreeId) { return window.api.git.generatePullRequestFields({ worktreePath: resolveLocalWorktreePath(context), + // Why: raw id — the `::workspace:` suffix is part of the worktree meta key. + ...(context.worktreeId ? { worktreeId: context.worktreeId } : {}), repoId: context.worktreeId ? getRepoIdFromWorktreeId(context.worktreeId) : undefined, connectionId: context.connectionId, ...input, diff --git a/src/shared/commit-message-generation.ts b/src/shared/commit-message-generation.ts index 7d08242bc..cd3f8d72a 100644 --- a/src/shared/commit-message-generation.ts +++ b/src/shared/commit-message-generation.ts @@ -7,6 +7,8 @@ export type CommitMessageDraftContext = { branch: string | null stagedSummary: string stagedPatch: string + /** Workspace-linked GitHub issue number. Omitted entirely when none resolves. */ + linkedIssue?: number | null } export type CommitMessageDraftOptions = { diff --git a/src/shared/pull-request-generation.ts b/src/shared/pull-request-generation.ts index fd927b82a..073b7f433 100644 --- a/src/shared/pull-request-generation.ts +++ b/src/shared/pull-request-generation.ts @@ -16,6 +16,8 @@ export type PullRequestDraftContext = { commitSummary: string changeSummary: string patch: string + /** Workspace-linked GitHub issue number. Omitted entirely when none resolves. */ + linkedIssue?: number | null } export type GeneratedPullRequestFields = { diff --git a/src/shared/source-control-ai-action-variables.test.ts b/src/shared/source-control-ai-action-variables.test.ts new file mode 100644 index 000000000..94cf5be42 --- /dev/null +++ b/src/shared/source-control-ai-action-variables.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from 'vitest' +import type { CommitMessageDraftContext } from './commit-message-generation' +import { + formatLinkedIssueTemplateValue, + SOURCE_CONTROL_ACTION_VARIABLE_INFO, + SOURCE_CONTROL_ACTION_VARIABLES, + withLinkedIssueDraftContext +} from './source-control-ai-action-variables' +import { + renderSourceControlActionCommandTemplate, + SOURCE_CONTROL_LAUNCH_ACTION_IDS +} from './source-control-ai-actions' + +describe('source-control AI variable registry', () => { + it('documents every registered variable so chip hover cards cannot crash', () => { + // Why: the registry type already makes an undocumented chip a compile error; + // this keeps the guarantee falsifiable at runtime if that type ever loosens. + const documented = new Set(Object.keys(SOURCE_CONTROL_ACTION_VARIABLE_INFO)) + const undocumented = [...new Set(Object.values(SOURCE_CONTROL_ACTION_VARIABLES).flat())].filter( + (variable) => !documented.has(variable) + ) + + expect(undocumented).toEqual([]) + }) + + it('offers linkedIssue on commit message and pull request only', () => { + expect(SOURCE_CONTROL_ACTION_VARIABLES.commitMessage).toContain('linkedIssue') + expect(SOURCE_CONTROL_ACTION_VARIABLES.pullRequest).toContain('linkedIssue') + expect(SOURCE_CONTROL_ACTION_VARIABLES.branchName).not.toContain('linkedIssue') + for (const actionId of SOURCE_CONTROL_LAUNCH_ACTION_IDS) { + expect(SOURCE_CONTROL_ACTION_VARIABLES[actionId]).not.toContain('linkedIssue') + } + }) + + it('names GitHub and the empty case in the linkedIssue description', () => { + const info = SOURCE_CONTROL_ACTION_VARIABLE_INFO.linkedIssue + expect(info.description).toContain('GitHub') + expect(info.description).toContain('Empty') + expect(info.example).toBe('123') + }) +}) + +describe('formatLinkedIssueTemplateValue', () => { + it('renders positive integers as decimal strings', () => { + expect(formatLinkedIssueTemplateValue(123)).toBe('123') + expect(formatLinkedIssueTemplateValue(1)).toBe('1') + }) + + it('renders anything that is not a positive integer as an empty string', () => { + // Why: `Fixes #-7` / `Fixes #1e+21` are worse output than `Fixes #`, so corrupt + // metadata degrades to the unlinked rendering instead of a nonsense reference. + for (const value of [ + 0, + -7, + 12.9, + 1e21, + null, + undefined, + Number.NaN, + Number.POSITIVE_INFINITY + ]) { + expect(formatLinkedIssueTemplateValue(value)).toBe('') + } + }) + + it('expands both brace forms and never leaves the token literal', () => { + const template = 'Fixes #{linkedIssue} / {{ linkedIssue }}' + expect( + renderSourceControlActionCommandTemplate(template, { + linkedIssue: formatLinkedIssueTemplateValue(42) + }) + ).toBe('Fixes #42 / 42') + expect( + renderSourceControlActionCommandTemplate(template, { + linkedIssue: formatLinkedIssueTemplateValue(null) + }) + ).toBe('Fixes # / ') + }) +}) + +describe('withLinkedIssueDraftContext', () => { + it('attaches only positive integers and leaves the context untouched otherwise', () => { + const context: CommitMessageDraftContext = { + branch: 'main', + stagedSummary: 'M a.ts', + stagedPatch: 'diff' + } + + expect(withLinkedIssueDraftContext(context, 42)).toEqual({ ...context, linkedIssue: 42 }) + for (const value of [null, undefined, Number.NaN, 0, -7, 12.9]) { + expect(withLinkedIssueDraftContext(context, value)).toBe(context) + } + }) +}) diff --git a/src/shared/source-control-ai-action-variables.ts b/src/shared/source-control-ai-action-variables.ts new file mode 100644 index 000000000..0aab5cceb --- /dev/null +++ b/src/shared/source-control-ai-action-variables.ts @@ -0,0 +1,129 @@ +import type { SourceControlActionId } from './source-control-ai-actions' + +/** + * Registering a variable a hover card cannot describe is a compile error: the + * element type is keyed off `SOURCE_CONTROL_ACTION_VARIABLE_INFO`, so chips can + * never index a missing entry. + */ +export const SOURCE_CONTROL_ACTION_VARIABLES: Record< + SourceControlActionId, + SourceControlActionVariable[] +> = { + commitMessage: ['basePrompt', 'branch', 'stagedFiles', 'stagedPatch', 'linkedIssue'], + pullRequest: [ + 'basePrompt', + 'branch', + 'baseBranch', + 'currentTitle', + 'currentBody', + 'commitSummary', + 'changedFiles', + 'patch', + 'linkedIssue' + ], + branchName: ['basePrompt', 'firstPrompt', 'assistantMessage'], + fixCommitFailure: ['basePrompt'], + fixPushFailure: ['basePrompt'], + fixChecks: ['basePrompt'], + resolveConflicts: ['basePrompt'], + resolveComments: ['basePrompt'] +} + +export type SourceControlActionVariableInfo = { + description: string + example: string +} + +export const SOURCE_CONTROL_ACTION_VARIABLE_INFO = { + basePrompt: { + description: + 'Orca’s built-in prompt for this action, including the context Orca knows how to gather safely.', + example: + 'Commit messages include staged diff guidance; PR details include branch comparison guidance; fix actions include the failure summary.' + }, + branch: { + description: 'The current source-control branch name.', + example: 'feature/source-control-ai-recipes' + }, + stagedFiles: { + description: 'A newline-separated list of staged files for commit-message generation.', + example: 'M src/shared/source-control-ai.ts\nA src/shared/source-control-ai-actions.ts' + }, + stagedPatch: { + description: 'The staged git patch used for commit-message generation.', + example: 'diff --git a/src/app.ts b/src/app.ts\n+addActionRecipeDefaults()' + }, + baseBranch: { + description: 'The target branch selected in the Create PR composer.', + example: 'main' + }, + currentTitle: { + description: 'The PR title currently typed in the composer before generation starts.', + example: 'Improve Source Control AI customization' + }, + currentBody: { + description: 'The PR description currently typed in the composer before generation starts.', + example: 'Adds configurable agents and command templates for Source Control actions.' + }, + commitSummary: { + description: 'A newline-separated list of commits on the branch compared to the base.', + example: 'a1b2c3d Add action recipe defaults\nd4e5f6a Render command templates' + }, + changedFiles: { + description: 'A summary of files changed between the branch and the base branch.', + example: + 'src/shared/source-control-ai-actions.ts | 24 +++++\nsrc/main/text-generation.ts | 8 +-' + }, + patch: { + description: 'The branch diff against the base branch used for PR-details generation.', + example: 'diff --git a/src/app.ts b/src/app.ts\n+renderSourceControlActionCommandTemplate()' + }, + firstPrompt: { + description: 'The first user request that created the Orca workspace.', + example: 'Fix CI and commit the result' + }, + assistantMessage: { + description: 'The initial agent response, when Orca has one available.', + example: 'I will inspect the failing check, patch the issue, and run tests.' + }, + linkedIssue: { + description: + 'The GitHub issue number linked to this workspace. Empty when no GitHub issue is linked (including GitLab-linked workspaces). Prefer instructional templates: a bare "Fixes #{linkedIssue}" becomes "Fixes #" when unlinked.', + example: '123' + } +} satisfies Record + +export type SourceControlActionVariable = keyof typeof SOURCE_CONTROL_ACTION_VARIABLE_INFO + +/** + * Issue numbers are positive integers on every supported provider, so anything + * else (negative, zero, fractional, non-finite) is corrupt metadata rather than + * a renderable issue reference — `Fixes #-7` is worse output than `Fixes #`. + * The safe-integer bound also keeps the rendering in decimal notation: `String` + * switches to exponent form (`1e+21`) above it. + */ +export function isLinkedIssueNumber(linkedIssue: unknown): linkedIssue is number { + return typeof linkedIssue === 'number' && Number.isSafeInteger(linkedIssue) && linkedIssue > 0 +} + +/** + * Render the workspace-linked GitHub issue for template substitution. Anything + * that is not a positive integer becomes `''` so the token expands to nothing + * instead of leaking into the prompt. + */ +export function formatLinkedIssueTemplateValue(linkedIssue: number | null | undefined): string { + return isLinkedIssueNumber(linkedIssue) ? String(linkedIssue) : '' +} + +/** + * Attach a resolved issue number to a draft context. Returns the context untouched + * when nothing resolves, so unlinked workspaces keep their existing context shape. + * The `linkedIssue`-bearing constraint keeps the attach off contexts that do not + * declare the field (branch-name generation), where it would be silently unread. + */ +export function withLinkedIssueDraftContext( + context: T, + linkedIssue: number | null | undefined +): T { + return isLinkedIssueNumber(linkedIssue) ? { ...context, linkedIssue } : context +} diff --git a/src/shared/source-control-ai-actions.test.ts b/src/shared/source-control-ai-actions.test.ts index db1af7c9b..bb9cf45bf 100644 --- a/src/shared/source-control-ai-actions.test.ts +++ b/src/shared/source-control-ai-actions.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' +import { SOURCE_CONTROL_ACTION_VARIABLES } from './source-control-ai-action-variables' import { normalizeSourceControlAiActionDefaults, - SOURCE_CONTROL_ACTION_VARIABLES, SOURCE_CONTROL_LAUNCH_ACTION_IDS, SOURCE_CONTROL_LAUNCH_ACTION_LABELS, DEFAULT_SOURCE_CONTROL_ACTION_COMMAND_TEMPLATES, diff --git a/src/shared/source-control-ai-actions.ts b/src/shared/source-control-ai-actions.ts index 96edcf881..8b1a8acb6 100644 --- a/src/shared/source-control-ai-actions.ts +++ b/src/shared/source-control-ai-actions.ts @@ -2,6 +2,10 @@ import { isCustomAgentId, type CustomAgentId } from './commit-message-agent-spec import { isTuiAgent } from './tui-agent-config' import type { TuiAgent } from './types' +// Why: the variable registry lives in `./source-control-ai-action-variables` for max-lines +// headroom. It is deliberately not re-exported here — one import path per symbol keeps a +// grep of that module's consumers complete (the chip row is the one that must not diverge). + export type SourceControlTextActionId = 'commitMessage' | 'pullRequest' | 'branchName' export type SourceControlLaunchActionId = @@ -75,86 +79,6 @@ export const DEFAULT_SOURCE_CONTROL_ACTION_COMMAND_TEMPLATES: Record< resolveComments: '{basePrompt}' } -export const SOURCE_CONTROL_ACTION_VARIABLES: Record = { - commitMessage: ['basePrompt', 'branch', 'stagedFiles', 'stagedPatch'], - pullRequest: [ - 'basePrompt', - 'branch', - 'baseBranch', - 'currentTitle', - 'currentBody', - 'commitSummary', - 'changedFiles', - 'patch' - ], - branchName: ['basePrompt', 'firstPrompt', 'assistantMessage'], - fixCommitFailure: ['basePrompt'], - fixPushFailure: ['basePrompt'], - fixChecks: ['basePrompt'], - resolveConflicts: ['basePrompt'], - resolveComments: ['basePrompt'] -} - -export type SourceControlActionVariableInfo = { - description: string - example: string -} - -export const SOURCE_CONTROL_ACTION_VARIABLE_INFO: Record = - { - basePrompt: { - description: - 'Orca’s built-in prompt for this action, including the context Orca knows how to gather safely.', - example: - 'Commit messages include staged diff guidance; PR details include branch comparison guidance; fix actions include the failure summary.' - }, - branch: { - description: 'The current source-control branch name.', - example: 'feature/source-control-ai-recipes' - }, - stagedFiles: { - description: 'A newline-separated list of staged files for commit-message generation.', - example: 'M src/shared/source-control-ai.ts\nA src/shared/source-control-ai-actions.ts' - }, - stagedPatch: { - description: 'The staged git patch used for commit-message generation.', - example: 'diff --git a/src/app.ts b/src/app.ts\n+addActionRecipeDefaults()' - }, - baseBranch: { - description: 'The target branch selected in the Create PR composer.', - example: 'main' - }, - currentTitle: { - description: 'The PR title currently typed in the composer before generation starts.', - example: 'Improve Source Control AI customization' - }, - currentBody: { - description: 'The PR description currently typed in the composer before generation starts.', - example: 'Adds configurable agents and command templates for Source Control actions.' - }, - commitSummary: { - description: 'A newline-separated list of commits on the branch compared to the base.', - example: 'a1b2c3d Add action recipe defaults\nd4e5f6a Render command templates' - }, - changedFiles: { - description: 'A summary of files changed between the branch and the base branch.', - example: - 'src/shared/source-control-ai-actions.ts | 24 +++++\nsrc/main/text-generation.ts | 8 +-' - }, - patch: { - description: 'The branch diff against the base branch used for PR-details generation.', - example: 'diff --git a/src/app.ts b/src/app.ts\n+renderSourceControlActionCommandTemplate()' - }, - firstPrompt: { - description: 'The first user request that created the Orca workspace.', - example: 'Fix CI and commit the result' - }, - assistantMessage: { - description: 'The initial agent response, when Orca has one available.', - example: 'I will inspect the failing check, patch the issue, and run tests.' - } - } - const ACTION_ID_SET = new Set(SOURCE_CONTROL_ACTION_IDS) function isRecord(value: unknown): value is Record { diff --git a/tests/e2e/helpers/source-control-ai-generators.ts b/tests/e2e/helpers/source-control-ai-generators.ts index 67d2133ca..be3f1b432 100644 --- a/tests/e2e/helpers/source-control-ai-generators.ts +++ b/tests/e2e/helpers/source-control-ai-generators.ts @@ -27,6 +27,30 @@ async function setCustomGenerator(page: Page, scriptPath: string): Promise }, scriptPath) } +/** + * Writes a generator that echoes back whichever issue number reached the prompt, so the + * assertion covers the whole chain (renderer → IPC → worktree meta → template render → + * agent stdin) rather than any single hop. `emitPayload` lines run with a captured `issue` + * const in scope and must write the payload the caller's generation path expects. + */ +export function writeLinkedIssueEchoGenerator(scriptPath: string, emitPayload: string[]): void { + writeFileSync( + scriptPath, + [ + 'const chunks = []', + "process.stdin.on('data', (chunk) => chunks.push(chunk))", + "process.stdin.on('end', () => {", + " const prompt = Buffer.concat(chunks).toString('utf8')", + // Why: capture the whole line, not `\d*` — a `\d*` capture matches zero digits before + // an unexpanded `{linkedIssue}` and reports it as `empty`, hiding a literal token. + ' const match = prompt.match(/ORCA_E2E_ISSUE=([^\\r\\n]*)/)', + " const issue = match ? match[1] || 'empty' : 'missing'", + ...emitPayload, + '})' + ].join('\n') + ) +} + export async function installDelayedPrGenerator( page: Page, generatorScriptPath: string, diff --git a/tests/e2e/source-control-commit-message-ai.spec.ts b/tests/e2e/source-control-commit-message-ai.spec.ts index 60fd8c49d..f6201bc6a 100644 --- a/tests/e2e/source-control-commit-message-ai.spec.ts +++ b/tests/e2e/source-control-commit-message-ai.spec.ts @@ -3,6 +3,7 @@ import { rmSync, writeFileSync } from 'node:fs' import os from 'node:os' import path from 'node:path' import { test, expect } from './helpers/orca-app' +import { writeLinkedIssueEchoGenerator } from './helpers/source-control-ai-generators' import { waitForSessionReady } from './helpers/store' import { openSourceControlForWorktree } from './helpers/worktree-registration' @@ -41,6 +42,74 @@ function cleanupWorktree(repoPath: string, worktreePath: string, branchName: str } test.describe('Source Control AI commit messages', () => { + // Why: the unlinked case separates a real resolver from one that always returns a number, + // and — because the generator echoes the whole line — a literal `{linkedIssue}` reaches the + // assertion as `saw-issue:{linkedIssue}` instead of masquerading as the empty expansion. + for (const { label, linkedIssue, expected } of [ + { label: 'substitutes the workspace-linked issue into', linkedIssue: 4242, expected: '4242' }, + { + label: 'expands the issue token to nothing for an unlinked workspace in', + linkedIssue: null, + expected: 'empty' + } + ]) { + test(`${label} the commit-message recipe`, async ({ orcaPage, testRepoPath }) => { + const { branchName, worktreePath } = createWorktreeWithStagedChange(testRepoPath) + const generatorPath = path.join(os.tmpdir(), `${branchName}-linked-issue-generator.cjs`) + writeLinkedIssueEchoGenerator(generatorPath, [' process.stdout.write(`saw-issue:${issue}`)']) + + try { + await waitForSessionReady(orcaPage) + await openSourceControlForWorktree(orcaPage, testRepoPath, worktreePath) + + await orcaPage.evaluate( + async ({ generatorPath, linkedIssue }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const worktreeId = store.getState().activeWorktreeId + if (!worktreeId) { + throw new Error('No worktree was active after opening Source Control') + } + await window.api.worktrees.updateMeta({ worktreeId, updates: { linkedIssue } }) + const customAgentCommand = `node ${JSON.stringify(generatorPath)}` + await store.getState().updateSettings({ + activeRuntimeEnvironmentId: null, + sourceControlAi: { + enabled: true, + agentId: 'custom' as const, + selectedModelByAgent: {}, + selectedThinkingByModel: {}, + customAgentCommand, + instructionsByOperation: {}, + actions: { + commitMessage: { + agentId: 'custom' as const, + commandInputTemplate: 'ORCA_E2E_ISSUE={linkedIssue}\n\n{basePrompt}' + } + } + } + }) + }, + { generatorPath, linkedIssue } + ) + + const textarea = orcaPage.getByRole('textbox', { name: 'Commit message' }) + await expect(textarea).toBeVisible({ timeout: 10_000 }) + + const generate = orcaPage.getByRole('button', { name: 'Generate commit message with AI' }) + await expect(generate).toBeEnabled() + await generate.click() + + await expect(textarea).toHaveValue(`saw-issue:${expected}`, { timeout: 15_000 }) + } finally { + rmSync(generatorPath, { force: true }) + cleanupWorktree(testRepoPath, worktreePath, branchName) + } + }) + } + test('generates a commit message from staged changes through the Source Control UI', async ({ orcaPage, testRepoPath diff --git a/tests/e2e/source-control-pr-linked-issue-ai.spec.ts b/tests/e2e/source-control-pr-linked-issue-ai.spec.ts new file mode 100644 index 000000000..625d19d58 --- /dev/null +++ b/tests/e2e/source-control-pr-linked-issue-ai.spec.ts @@ -0,0 +1,96 @@ +import { rmSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { test, expect } from './helpers/orca-app' +import { + createBranchCommit, + openSourceControl, + seedCreatePrComposer +} from './helpers/source-control-ai-generation' +import { writeLinkedIssueEchoGenerator } from './helpers/source-control-ai-generators' +import { waitForSessionReady } from './helpers/store' + +// Why: the PR path reads the echoed issue from the generated title. +function writeLinkedIssuePrEchoGenerator(scriptPath: string, base: string): void { + writeLinkedIssueEchoGenerator(scriptPath, [ + ' process.stdout.write(JSON.stringify({', + ` base: ${JSON.stringify(base)},`, + ' title: `saw-issue:${issue}`,', + " body: 'linked-issue e2e body',", + ' draft: false', + ' }))' + ]) +} + +test.describe('Source Control AI pull request linkedIssue', () => { + // Why: the unlinked case separates a real resolver from one that always returns a number, + // and — because the generator echoes the whole line into the title — a literal + // `{linkedIssue}` reaches the assertion as `saw-issue:{linkedIssue}` instead of + // masquerading as the empty expansion. + for (const { label, linkedIssue, expected } of [ + { label: 'substitutes the workspace-linked issue into', linkedIssue: 4242, expected: '4242' }, + { + label: 'expands the issue token to nothing for an unlinked workspace in', + linkedIssue: null, + expected: 'empty' + } + ]) { + test(`${label} the pull-request recipe`, async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + const { prWorktreeId, prWorktreePath, primaryBranch } = await seedCreatePrComposer(orcaPage) + createBranchCommit(prWorktreePath) + + const generatorPath = path.join( + os.tmpdir(), + `e2e-pr-linked-issue-${Date.now()}-${Math.random().toString(16).slice(2)}.cjs` + ) + writeLinkedIssuePrEchoGenerator(generatorPath, primaryBranch) + + try { + await orcaPage.evaluate( + async ({ generatorPath, linkedIssue, worktreeId }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + await window.api.worktrees.updateMeta({ worktreeId, updates: { linkedIssue } }) + const customAgentCommand = `node ${JSON.stringify(generatorPath)}` + await store.getState().updateSettings({ + activeRuntimeEnvironmentId: null, + sourceControlAi: { + enabled: true, + agentId: 'custom' as const, + selectedModelByAgent: {}, + selectedThinkingByModel: {}, + customAgentCommand, + instructionsByOperation: {}, + actions: { + pullRequest: { + agentId: 'custom' as const, + commandInputTemplate: 'ORCA_E2E_ISSUE={linkedIssue}\n\n{basePrompt}' + } + } + } + }) + }, + { generatorPath, linkedIssue, worktreeId: prWorktreeId } + ) + + await openSourceControl(orcaPage, prWorktreeId) + + const title = orcaPage.getByRole('textbox', { name: 'Pull request title' }) + await expect(title).toBeVisible({ timeout: 10_000 }) + + const generate = orcaPage.getByRole('button', { + name: 'Generate pull request details with AI' + }) + await expect(generate).toBeEnabled() + await generate.click() + + await expect(title).toHaveValue(`saw-issue:${expected}`, { timeout: 15_000 }) + } finally { + rmSync(generatorPath, { force: true }) + } + }) + } +})