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 <help@stably.ai>
This commit is contained in:
Jinjing
2026-07-25 13:30:30 -07:00
co-authored by Orca
parent 92cd6803be
commit d9d09f2eae
17 changed files with 565 additions and 148 deletions
+108 -1
View File
@@ -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,
@@ -2086,7 +2102,9 @@ describe('registerFilesystemHandlers', () => {
)
})
it('ignores a worktree id that does not own the requested worktree path', async () => {
// 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',
@@ -2141,6 +2159,95 @@ describe('registerFilesystemHandlers', () => {
)
})
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()
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',
+1 -1
View File
@@ -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 {
@@ -101,7 +102,6 @@ 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 { withLinkedIssueDraftContext } from '../../shared/source-control-ai-actions'
import { listMarkdownDocuments, markdownDocumentsFromRelativePaths } from './markdown-documents'
import { checkRgAvailable } from './rg-availability'
import {
@@ -41,7 +41,11 @@ describe('resolveSourceControlAiLinkedIssue', () => {
expect(store.getWorktreeMeta).toHaveBeenCalledWith(instanceId)
})
it('ignores an id whose path does not match the request', () => {
// 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(
@@ -65,7 +69,7 @@ describe('resolveSourceControlAiLinkedIssue', () => {
).toBe(5)
})
it('ignores an id whose repoId contradicts the request repoId', () => {
it('rejects an independently supplied id whose repoId contradicts the request repoId', () => {
const store = makeStore({ [LOCAL_ID]: { linkedIssue: 123 } })
expect(
@@ -78,6 +82,19 @@ describe('resolveSourceControlAiLinkedIssue', () => {
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 } })
@@ -124,14 +141,14 @@ describe('resolveSourceControlAiLinkedIssue', () => {
).toBeNull()
})
it('returns null for unparsable ids and unlinked or non-numeric meta', () => {
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]) {
for (const linkedIssue of [null, undefined, Number.NaN, 0, -7, 12.9]) {
expect(
resolveSourceControlAiLinkedIssue(makeStore({ [LOCAL_ID]: { linkedIssue } }), {
worktreeId: LOCAL_ID,
+16 -4
View File
@@ -1,5 +1,6 @@
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 = {
@@ -39,8 +40,17 @@ function matchesRequestPath(
*
* 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. A stale id after a workspace switch would
* otherwise pull another workspace's issue number into this commit message.
* (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:<uuid>` suffix of folder-repo
* workspace instances is part of the key, while validation uses the stripped path.
*/
@@ -59,7 +69,9 @@ export function resolveSourceControlAiLinkedIssue(
if (!parsed) {
return null
}
if (args.repoId && parsed.repoId !== args.repoId) {
// 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)) {
@@ -67,5 +79,5 @@ export function resolveSourceControlAiLinkedIssue(
}
const linkedIssue = store.getWorktreeMeta(args.worktreeId)?.linkedIssue
// Why: GitHub only in v1 — no `linkedGitLabIssue` dual-read.
return typeof linkedIssue === 'number' && Number.isFinite(linkedIssue) ? linkedIssue : null
return isLinkedIssueNumber(linkedIssue) ? linkedIssue : null
}
+38 -4
View File
@@ -61,7 +61,11 @@ vi.mock('../source-control/pull-request-template', () => ({
const tempDirs: string[] = []
function makeWorktree(path: string, linkedIssue: number | null = null): ResolvedRuntimeGitWorktree {
return {
// 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,
@@ -69,11 +73,12 @@ function makeWorktree(path: string, linkedIssue: number | null = null): Resolved
git: {
path,
branch: 'main',
bare: false,
detached: false,
isBare: false,
isMainWorktree: false,
head: 'a'.repeat(40)
}
} as unknown as ResolvedRuntimeGitWorktree
} satisfies Partial<ResolvedRuntimeGitWorktree>
return worktree as unknown as ResolvedRuntimeGitWorktree
}
function makeCommands(worktreePath: string): RuntimeGitCommands {
@@ -701,4 +706,33 @@ describe('RuntimeGitCommands', () => {
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'
)
})
})
+1 -1
View File
@@ -23,8 +23,8 @@ 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 { withLinkedIssueDraftContext } from '../../shared/source-control-ai-actions'
import type { GitProviderStatusOptions } from '../providers/types'
import { getRemoteCommitUrl, getRemoteFileUrl } from '../git/repo'
import {
@@ -1970,17 +1970,41 @@ describe('linkedIssue template substitution', () => {
}
})
// 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: 42 },
{ agentId: 'custom', model: '', customAgentCommand: 'agent' },
{ ...COMMIT_CONTEXT, linkedIssue: BUILT_IN_PROMPT_SENTINEL_ISSUE },
builtInPromptParams,
capturingTarget((value) => {
prompt = value
})
)
expect(prompt).not.toContain('42')
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')
})
@@ -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 })
}}
@@ -38,6 +38,11 @@ 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, '&quot;')
}
describe('buildCommitMessageGenerationParams', () => {
it('defaults saved text-generation recipes to the global target when repo and global are available', () => {
expect(
@@ -79,6 +84,57 @@ describe('buildCommitMessageGenerationParams', () => {
expect(markup).toContain('You are generating a single git commit message.')
})
// Why: the chips fall back to the synthetic `123` sample when no preview is supplied,
// which told an unlinked workspace it would render `Fixes #123` and then produced
// `Fixes #`. Workspace-scoped callers must preview their own value, empty included.
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('leaves the issue preview to the synthetic sample 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')
})
it('preserves the resolved model and thinking level for the selected agent', () => {
expect(
buildCommitMessageGenerationParams({
@@ -31,6 +31,8 @@ type SourceControlTextGenerationBaseDialogProps = {
settings: GlobalSettings | null
repo?: Pick<Repo, 'id' | 'sourceControlAi'> | 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}
@@ -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<Repo, 'id' | 'sourceControlAi'> | 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,7 +118,21 @@ export function SourceControlTextGenerationDialogForm({
settings,
customAgentCommand: baseParams?.customAgentCommand
})
const paramsPlanResult = params ? planSourceControlTextGeneration(actionId, params) : null
// Why: the workspace's own issue, not the synthetic `123`, so an unlinked workspace
// previews what it will actually generate.
const variablePreviews = useMemo(() => {
const previews: Record<string, string> = {}
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, variablePreviews)
: null
const canRunGeneration = Boolean(params && paramsPlanResult?.ok)
const saving = savingTargetKey !== null
const defaultsAlreadySaved = Boolean(
@@ -287,7 +305,7 @@ export function SourceControlTextGenerationDialogForm({
/>
<SourceControlActionVariableChips
actionId={actionId}
variablePreviews={basePromptPreview ? { basePrompt: basePromptPreview } : undefined}
variablePreviews={variablePreviews}
onInsert={(variable) => {
const separator =
commandTemplate.endsWith('\n') || commandTemplate.length === 0 ? '' : ' '
@@ -64,6 +64,28 @@ describe('planSourceControlCommitMessageGeneration', () => {
}
})
it('prefers caller-supplied variable values over the synthetic sample', () => {
for (const [linkedIssue, expected] of [
['7', 'echo Fixes #7'],
// Why: an unlinked workspace must preview the empty expansion it will really send,
// not the synthetic `123`.
['', 'echo Fixes #']
]) {
const result = planSourceControlTextGeneration(
'commitMessage',
{
agentId: 'custom',
model: '',
customAgentCommand: 'echo {prompt}',
commandInputTemplate: 'Fixes #{linkedIssue}'
},
{ linkedIssue }
)
expect(result.ok && result.commandLabel).toBe(expected)
}
})
it('leaves linkedIssue literal in branch-name plan previews', () => {
const result = planSourceControlTextGeneration('branchName', {
agentId: 'custom',
@@ -50,16 +50,23 @@ const SYNTHETIC_BASE_PROMPTS: Record<SourceControlTextActionId, string> = {
branchName: 'Generate a git branch name for a synthetic task.'
}
/**
* `variableOverrides` lets a workspace-scoped caller (the in-app generation dialog)
* preview real values where it has them, so an unlinked workspace does not preview
* `Fixes #123` and then generate `Fixes #`. Settings dry-runs pass nothing and stay
* fully synthetic, since no workspace is in scope there.
*/
export function planSourceControlTextGeneration(
actionId: SourceControlTextActionId,
params: ResolvedSourceControlAiGenerationParams
params: ResolvedSourceControlAiGenerationParams,
variableOverrides?: Record<string, string>
): SourceControlGenerationPlanResult {
const prompt =
params.commandInputTemplate !== undefined
? renderSourceControlActionCommandTemplate(
params.commandInputTemplate,
SYNTHETIC_TEXT_GENERATION_CONTEXT[actionId]
)
? renderSourceControlActionCommandTemplate(params.commandInputTemplate, {
...SYNTHETIC_TEXT_GENERATION_CONTEXT[actionId],
...variableOverrides
})
: SYNTHETIC_BASE_PROMPTS[actionId]
if (!prompt.trim()) {
return {
@@ -0,0 +1,120 @@
import type { SourceControlActionId } from './source-control-ai-actions'
export const SOURCE_CONTROL_ACTION_VARIABLES: Record<SourceControlActionId, string[]> = {
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: Record<string, SourceControlActionVariableInfo> =
{
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'
}
}
/**
* 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<T extends { linkedIssue?: number | null }>(
context: T,
linkedIssue: number | null | undefined
): T {
return isLinkedIssueNumber(linkedIssue) ? { ...context, linkedIssue } : context
}
+27 -14
View File
@@ -1,4 +1,5 @@
import { describe, expect, it } from 'vitest'
import type { CommitMessageDraftContext } from './commit-message-generation'
import {
formatLinkedIssueTemplateValue,
normalizeSourceControlAiActionDefaults,
@@ -200,18 +201,26 @@ describe('source-control AI variable registry', () => {
})
describe('formatLinkedIssueTemplateValue', () => {
it('renders finite numbers as decimal strings', () => {
it('renders positive integers as decimal strings', () => {
expect(formatLinkedIssueTemplateValue(123)).toBe('123')
expect(formatLinkedIssueTemplateValue(0)).toBe('0')
expect(formatLinkedIssueTemplateValue(-7)).toBe('-7')
expect(formatLinkedIssueTemplateValue(12.9)).toBe('12')
expect(formatLinkedIssueTemplateValue(1)).toBe('1')
})
it('renders missing and non-finite values as an empty string', () => {
expect(formatLinkedIssueTemplateValue(null)).toBe('')
expect(formatLinkedIssueTemplateValue(undefined)).toBe('')
expect(formatLinkedIssueTemplateValue(Number.NaN)).toBe('')
expect(formatLinkedIssueTemplateValue(Number.POSITIVE_INFINITY)).toBe('')
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', () => {
@@ -230,12 +239,16 @@ describe('formatLinkedIssueTemplateValue', () => {
})
describe('withLinkedIssueDraftContext', () => {
it('attaches only finite numbers and leaves the context untouched otherwise', () => {
const context = { branch: 'main', stagedSummary: 'M a.ts', stagedPatch: 'diff' }
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 })
expect(withLinkedIssueDraftContext(context, null)).toBe(context)
expect(withLinkedIssueDraftContext(context, undefined)).toBe(context)
expect(withLinkedIssueDraftContext(context, Number.NaN)).toBe(context)
for (const value of [null, undefined, Number.NaN, 0, -7, 12.9]) {
expect(withLinkedIssueDraftContext(context, value)).toBe(context)
}
})
})
+11 -109
View File
@@ -2,6 +2,17 @@ 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 its own module for max-lines headroom;
// re-exported here so every existing chip/render consumer keeps one import site.
export {
SOURCE_CONTROL_ACTION_VARIABLES,
SOURCE_CONTROL_ACTION_VARIABLE_INFO,
formatLinkedIssueTemplateValue,
isLinkedIssueNumber,
withLinkedIssueDraftContext,
type SourceControlActionVariableInfo
} from './source-control-ai-action-variables'
export type SourceControlTextActionId = 'commitMessage' | 'pullRequest' | 'branchName'
export type SourceControlLaunchActionId =
@@ -75,115 +86,6 @@ export const DEFAULT_SOURCE_CONTROL_ACTION_COMMAND_TEMPLATES: Record<
resolveComments: '{basePrompt}'
}
export const SOURCE_CONTROL_ACTION_VARIABLES: Record<SourceControlActionId, string[]> = {
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: Record<string, SourceControlActionVariableInfo> =
{
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'
}
}
/**
* Render the workspace-linked GitHub issue for template substitution. Non-numbers
* become `''` so the token expands to nothing instead of leaking into the prompt.
*/
export function formatLinkedIssueTemplateValue(linkedIssue: number | null | undefined): string {
return typeof linkedIssue === 'number' && Number.isFinite(linkedIssue)
? String(Math.trunc(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.
*/
export function withLinkedIssueDraftContext<T extends object>(
context: T,
linkedIssue: number | null | undefined
): T {
return typeof linkedIssue === 'number' && Number.isFinite(linkedIssue)
? { ...context, linkedIssue }
: context
}
const ACTION_ID_SET = new Set<string>(SOURCE_CONTROL_ACTION_IDS)
function isRecord(value: unknown): value is Record<string, unknown> {
@@ -40,7 +40,86 @@ function cleanupWorktree(repoPath: string, worktreePath: string, branchName: str
}
}
/**
* Echoes back whichever issue number reached the prompt, so the assertion covers the
* whole chain (renderer → IPC → worktree meta → template render → agent stdin) rather
* than any single hop.
*/
function writeLinkedIssueEchoGenerator(scriptPath: string): void {
writeFileSync(
scriptPath,
[
'const chunks = []',
"process.stdin.on('data', (chunk) => chunks.push(chunk))",
"process.stdin.on('end', () => {",
" const prompt = Buffer.concat(chunks).toString('utf8')",
' const match = prompt.match(/ORCA_E2E_ISSUE=(\\d*)/)',
" process.stdout.write(`saw-issue:${match ? match[1] || 'empty' : 'missing'}`)",
'})'
].join('\n')
)
}
test.describe('Source Control AI commit messages', () => {
test('substitutes the workspace-linked issue into 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)
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: 4242 }
)
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:4242', { 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