From ca70be831861c4697da67b747fffe4c5efa824a0 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:31:44 -0700 Subject: [PATCH] Add {linkedIssue} template variable for commit and PR generation (#10640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(source-control-ai): add {linkedIssue} recipe variable for commit and PR prompts Custom commit-message and pull-request recipes can now reference the GitHub issue linked to the workspace, so a template like "Fixes #{linkedIssue}" lands the closing trailer without the user retyping the number. - register `linkedIssue` on the commitMessage and pullRequest actions only, with the VARIABLE_INFO entry the chip hover card requires - substitute unconditionally via `formatLinkedIssueTemplateValue` (empty string when nothing resolves) so the token never survives into a prompt; enrich the draft context conditionally via `withLinkedIssueDraftContext` so unlinked workspaces keep their existing context shape - attach at the 7 call boundaries (runtime commit x2, runtime PR shared, IPC commit x2, IPC PR x2); the pure git gather stays pure - validate the renderer-supplied worktreeId against the request path and repoId before any meta read, comparing SSH paths as raw strings so a Windows host cannot rewrite a remote POSIX path - built-in prompts are unchanged; no GitLab dual-read and no default trailer * fix(source-control-ai): resolve {linkedIssue} adversarial review findings Addresses 13 of the 14 findings from the {linkedIssue} code review (6 minor, 8 nit, 0 critical, 0 major); Issue 5 (GitLab provider naming) is deferred to design Open Question 3 as product expansion. Behavior: - Dialog previews the workspace's real linked issue instead of the synthetic 123, in both the chip hover card and the plan preview, so an unlinked workspace previews the `Fixes #` it will actually generate. Settings dry-runs stay fully synthetic. - Reject non-positive, fractional and unsafe-integer issue numbers at the IPC resolver via a shared isLinkedIssueNumber predicate, so corrupt meta never reaches a draft context (previously -7 rendered `Fixes #-7` and 1e21 rendered `Fixes #1e+21`). - Fail closed on an empty-string repoId instead of skipping the cross-check. Structure: - Split the variable registry into source-control-ai-action-variables.ts and re-export it, restoring max-lines headroom with no consumer churn and no lint disable. - Constrain withLinkedIssueDraftContext to contexts declaring linkedIssue. - Move the misplaced shared imports into their import group. Docs and tests: - Document that the IPC id/path validator guards relay/CLI/future callers, not the renderer (whose path is id-derived), and rename the three tests that read as proof of a protection that cannot fire. - Add PR-side coverage that was missing: three git:generatePullRequestFields handler tests, a built-in PR prompt no-leak guard, and the runtime PR unlinked case. - Replace the coincidental '42' assertion with a fixture-unique sentinel. - Type the runtime worktree fixture with satisfies, which surfaced and fixed pre-existing drift in its git sub-object. - Add an e2e case covering the preload -> main -> meta -> template chain. Co-authored-by: Orca * fix(source-control-ai): resolve {linkedIssue} adversarial re-review findings Addresses all 8 findings from the {linkedIssue} code re-review (2 minor, 6 nit, 0 critical, 0 major); none deferred. Behavior: - Revert the variableOverrides parameter on planSourceControlTextGeneration. Its result is a Save/Generate gate, not a preview, and the recipe it validates is saved repo- or globally scoped -- so rendering it against the active workspace disabled both buttons with "Command input is empty." for a {linkedIssue}-only template on any unlinked workspace, blocking a global settings write. Validation is synthetic again; chip previews are unchanged. - Make the chip hover card additive instead of either/or. A supplied preview now appends a "This workspace" sample below the description and Example rather than replacing them, so the GitLab-empty and dangling `Fixes #` warning survives on the two dialogs where recipes are actually authored. basePrompt keeps its preview-only shape, where the preview is the content. Structure: - Drop the registry re-export from source-control-ai-actions.ts and move the last two consumers onto source-control-ai-action-variables, so one import path per symbol keeps a grep of the registry's consumers complete. - Split the registry/helper suites into source-control-ai-action-variables.test.ts so each test file mirrors its module. Tests: - Cover the Save/Generate gate at the canRunGeneration level for a bare {linkedIssue} recipe on linked and unlinked workspaces, with a negative control proving the buttons can still be disabled. - Cover the chip hover card directly; the dialog tests mock it away. - Guard the PR mismatched-id test with toHaveLength(1) so it cannot pass vacuously on an unrelated early return. - Add an unlinked-workspace e2e case (saw-issue:empty), which is what distinguishes a real resolver from one that always returns a number. Spec now runs green: 3 passed. - Rename the dialog test that claimed a synthetic-fallback assertion it did not make, and route its renders through one shared helper. Docs are worktree-local (.gitignore:84 ignores docs/**): the design doc's plan-preview and chip-surface claims, the manual QA rows, and both reviews' statements about pre-existing PR-handler tests are corrected there. * fix(source-control-ai): make the {linkedIssue} e2e guard and dialog test falsifiable The e2e unlinked case extracted the echoed issue with `ORCA_E2E_ISSUE=(\d*)`, which matches zero digits in front of an unexpanded `{linkedIssue}` and reported it as `empty` — so the case that exists to catch a literal token surviving into a prompt passed on exactly that regression. Capture the whole line instead: a literal now arrives as `saw-issue:{linkedIssue}` and fails, verified by dropping the substitution key for unlinked contexts and watching the case go red. Also drop the inert `not.toContain('Command input is empty.')` assertion — that copy is click-driven `generationError` state and this suite renders statically, so it could never fail; the claim it reached for is carried by the plan test. Rename two plan tests off the "plan preview" framing the design now rejects. Local review artifacts (design doc, implementation notes, final review) were swept to match the tree in the same pass; they are gitignored here. * Resolve {linkedIssue} from live metadata, not cache Resolved worktrees are cached for a second, causing commit and PR generation to use stale linked-issue state. Hosts now implement getWorktreeLinkedIssue to provide fresh issue metadata by worktree id, with proper fallback for unlinked workspaces. Updates both commit message and PR field generation paths; includes integration and e2e coverage. * Keep cached linkedIssue when metadata is unavailable Return undefined from getWorktreeLinkedIssue when live metadata cannot be read (store not ready), distinguishing it from null (unlinked). The caller now falls back to the cached worktree value instead of treating unavailable as unlinked. Also extract the linked-issue echo generator as a shared e2e test helper. --------- Co-authored-by: Orca --- src/main/ipc/filesystem.test.ts | 230 +++++++++++++++ src/main/ipc/filesystem.ts | 22 ++ .../source-control-ai-linked-issue.test.ts | 173 ++++++++++++ .../ipc/source-control-ai-linked-issue.ts | 83 ++++++ src/main/runtime/orca-runtime-git.test.ts | 262 +++++++++++++++++- src/main/runtime/orca-runtime-git.ts | 20 ++ ...linked-issue-live-meta.integration.test.ts | 141 ++++++++++ src/main/runtime/orca-runtime.ts | 13 +- .../commit-message-text-generation.test.ts | 162 +++++++++++ .../commit-message-text-generation.ts | 9 +- src/preload/api-types.ts | 4 + src/preload/index.ts | 2 + .../right-sidebar/SourceControl.tsx | 2 + .../SourceControlTextGenerationDialog.test.ts | 107 +++++++ .../SourceControlTextGenerationDialog.tsx | 4 + .../SourceControlTextGenerationDialogForm.tsx | 18 +- .../SourceControlActionVariableChips.test.tsx | 37 +++ .../SourceControlActionVariableChips.tsx | 89 +++--- src/renderer/src/i18n/locales/en.json | 3 +- src/renderer/src/i18n/locales/es.json | 3 +- src/renderer/src/i18n/locales/ja.json | 3 +- src/renderer/src/i18n/locales/ko.json | 3 +- src/renderer/src/i18n/locales/zh.json | 3 +- .../source-control-generation-plan.test.ts | 38 +++ .../src/lib/source-control-generation-plan.ts | 12 +- .../src/runtime/runtime-git-client.test.ts | 43 +++ .../src/runtime/runtime-git-client.ts | 4 + src/shared/commit-message-generation.ts | 2 + src/shared/pull-request-generation.ts | 2 + ...source-control-ai-action-variables.test.ts | 94 +++++++ .../source-control-ai-action-variables.ts | 129 +++++++++ src/shared/source-control-ai-actions.test.ts | 2 +- src/shared/source-control-ai-actions.ts | 84 +----- .../helpers/source-control-ai-generators.ts | 24 ++ .../source-control-commit-message-ai.spec.ts | 69 +++++ .../source-control-pr-linked-issue-ai.spec.ts | 96 +++++++ 36 files changed, 1859 insertions(+), 133 deletions(-) create mode 100644 src/main/ipc/source-control-ai-linked-issue.test.ts create mode 100644 src/main/ipc/source-control-ai-linked-issue.ts create mode 100644 src/main/runtime/orca-runtime-linked-issue-live-meta.integration.test.ts create mode 100644 src/shared/source-control-ai-action-variables.test.ts create mode 100644 src/shared/source-control-ai-action-variables.ts create mode 100644 tests/e2e/source-control-pr-linked-issue-ai.spec.ts 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 }) + } + }) + } +})