diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index 7aabea4adf1..b3465ac1617 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -2157,6 +2157,8 @@ describe('registerFilesystemHandlers', () => { }) 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' ) diff --git a/src/main/runtime/orca-runtime-git.test.ts b/src/main/runtime/orca-runtime-git.test.ts index 665d207a1f1..1103852ed8a 100644 --- a/src/main/runtime/orca-runtime-git.test.ts +++ b/src/main/runtime/orca-runtime-git.test.ts @@ -692,6 +692,30 @@ describe('RuntimeGitCommands', () => { ) }) + 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) diff --git a/src/main/runtime/orca-runtime-git.ts b/src/main/runtime/orca-runtime-git.ts index 670d59cdeaf..a2dd698444f 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -165,17 +165,20 @@ export type RuntimeGitCommandHost = { * 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 + getWorktreeLinkedIssue?(worktreeId: string): number | null | undefined } export class RuntimeGitCommands { constructor(private readonly host: RuntimeGitCommandHost) {} private linkedIssueForTarget(target: RuntimeGitTarget): number | null | undefined { - return this.host.getWorktreeLinkedIssue - ? this.host.getWorktreeLinkedIssue(target.worktree.id) - : target.worktree.linkedIssue + 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( diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index dddfd90600a..d3a9cb25785 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -7065,8 +7065,15 @@ export class OrcaRuntimeService { 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) => - this.store?.getWorktreeMeta?.(worktreeId)?.linkedIssue ?? null + 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/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts index 6e46d53451e..1af9d5d8bc9 100644 --- a/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts +++ b/src/renderer/src/components/right-sidebar/SourceControlTextGenerationDialog.test.ts @@ -110,9 +110,8 @@ describe('buildCommitMessageGenerationParams', () => { expect(markup).toContain('You are generating a single git commit message.') }) - // Why: the chips fall back to the synthetic `123` sample when no preview is supplied, - // which told an unlinked workspace it would render `Fixes #123` and then produced - // `Fixes #`. Workspace-scoped callers must preview their own value, empty included. + // 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: '' } @@ -161,8 +160,8 @@ describe('buildCommitMessageGenerationParams', () => { expect(markup).not.toContain('linkedIssue') }) - // Why: previews are chip-only. The recipe is saved repo- or globally scoped, so gating - // Save/Generate on the active workspace's empty `{linkedIssue}` would block a global write. + // 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 } @@ -172,16 +171,16 @@ describe('buildCommitMessageGenerationParams', () => { linkedIssue }) - // Why: `disabled=""` is the only observable gate here — the "Command input is empty." - // copy is click-driven `generationError` state, so static markup can never show it. + // 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: the negative control for the two cases above — without it they pass even if the - // buttons could never be disabled at all. + // 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=""') diff --git a/tests/e2e/helpers/source-control-ai-generators.ts b/tests/e2e/helpers/source-control-ai-generators.ts index 67d2133cac0..be3f1b43247 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 5d861c94fa5..f6201bc6a75 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' @@ -40,28 +41,6 @@ function cleanupWorktree(repoPath: string, worktreePath: string, branchName: str } } -/** - * 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. - */ -function writeLinkedIssueEchoGenerator(scriptPath: 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]*)/)', - " process.stdout.write(`saw-issue:${match ? match[1] || 'empty' : 'missing'}`)", - '})' - ].join('\n') - ) -} - 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 @@ -77,7 +56,7 @@ test.describe('Source Control AI commit messages', () => { 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) + writeLinkedIssueEchoGenerator(generatorPath, [' process.stdout.write(`saw-issue:${issue}`)']) try { await waitForSessionReady(orcaPage) diff --git a/tests/e2e/source-control-pr-linked-issue-ai.spec.ts b/tests/e2e/source-control-pr-linked-issue-ai.spec.ts index 82b276dfd2d..625d19d58b8 100644 --- a/tests/e2e/source-control-pr-linked-issue-ai.spec.ts +++ b/tests/e2e/source-control-pr-linked-issue-ai.spec.ts @@ -1,4 +1,4 @@ -import { rmSync, writeFileSync } from 'node:fs' +import { rmSync } from 'node:fs' import os from 'node:os' import path from 'node:path' import { test, expect } from './helpers/orca-app' @@ -7,34 +7,19 @@ import { openSourceControl, seedCreatePrComposer } from './helpers/source-control-ai-generation' +import { writeLinkedIssueEchoGenerator } from './helpers/source-control-ai-generators' import { waitForSessionReady } from './helpers/store' -/** - * Echoes back whichever issue number reached the PR prompt as the generated - * title, so the assertion covers renderer → IPC → meta → template → agent stdin - * for the pull-request path (the commit twin lives in source-control-commit-message-ai). - */ +// Why: the PR path reads the echoed issue from the generated title. function writeLinkedIssuePrEchoGenerator(scriptPath: string, base: 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'", - ' process.stdout.write(JSON.stringify({', - ` base: ${JSON.stringify(base)},`, - ' title: `saw-issue:${issue}`,', - " body: 'linked-issue e2e body',", - ' draft: false', - ' }))', - '})' - ].join('\n') - ) + 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', () => {