Files
orca/src/shared/pull-request-generation.ts
T
JinjingandOrca ca70be8318 Add {linkedIssue} template variable for commit and PR generation (#10640)
* 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 <help@stably.ai>

* 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 <help@stably.ai>
2026-07-25 19:31:44 -07:00

186 lines
6.1 KiB
TypeScript

import { truncateDiffForPrompt } from './commit-message-prompt'
import { assertJsonTextStructureWithinLimits } from './json-text-structure-limit'
export const GENERATED_PULL_REQUEST_JSON_STRUCTURE_LIMITS = {
structuralTokens: 64,
nestingDepth: 8
} as const
export type PullRequestDraftContext = {
branch: string | null
base: string
branchChangedByPreparation: boolean
currentTitle: string
currentBody: string
currentDraft: boolean
commitSummary: string
changeSummary: string
patch: string
/** Workspace-linked GitHub issue number. Omitted entirely when none resolves. */
linkedIssue?: number | null
}
export type GeneratedPullRequestFields = {
base: string
title: string
body: string
draft: boolean
}
function limitSection(value: string, maxChars: number): string {
if (value.length <= maxChars) {
return value
}
const omitted = value.length - maxChars
return `${value.slice(0, maxChars)}\n\n[truncated: ${omitted} characters omitted]`
}
export function buildPullRequestFieldsPrompt(
context: PullRequestDraftContext,
customPrompt: string
): string {
const base = [
'You are generating pull request details.',
'Return ONLY compact JSON with this exact shape:',
'{"base":"branch-name","title":"short title","body":"markdown description","draft":false}',
'',
'Rules:',
'- Use the branch diff and commits below as source of truth.',
'- Keep the base branch as the current base unless the diff clearly targets a different branch.',
'- Title: concise, specific, no trailing period.',
'- Body: useful Markdown summary for reviewers. Include testing notes only when evidence exists.',
'- If Current description contains a pull request or merge request template, preserve its headings, required sections, and checklists while filling relevant sections from the branch changes.',
'- Leave genuinely unknown template items as TODO or unchecked instead of deleting them.',
'- draft: true only when the changes clearly look unfinished, WIP, or unsafe to review.',
'- Do not include labels, reviewers, code fences, prose, or any keys beyond base/title/body/draft.',
'',
`Head branch: ${context.branch ?? '(detached)'}`,
`Current base: ${context.base}`,
`Current title: ${context.currentTitle || '(empty)'}`,
`Current description: ${context.currentBody || '(empty)'}`,
`Current draft: ${context.currentDraft ? 'true' : 'false'}`,
'',
'Commits:',
limitSection(context.commitSummary || '(none)', 8_000),
'',
'Changed files:',
limitSection(context.changeSummary || '(none)', 8_000),
'',
'Patch:',
'```diff',
truncateDiffForPrompt(context.patch),
'```'
].join('\n')
const trimmedPrompt = customPrompt.trim()
if (!trimmedPrompt) {
return [
base,
'',
'Final output requirement:',
'Return compact JSON only with keys base, title, body, and draft. No prose or code fences.'
].join('\n')
}
return [
base,
'',
'Additional user prompt:',
limitSection(trimmedPrompt, 4_000),
'',
'Final output requirement:',
'Return compact JSON only with keys base, title, body, and draft. No prose or code fences.'
].join('\n')
}
function stripJsonFence(raw: string): string {
let text = raw.trim()
const fencedBody = getJsonFenceBody(text)
if (fencedBody !== null) {
text = fencedBody.trim()
}
const start = text.indexOf('{')
const end = text.lastIndexOf('}')
if (start !== -1 && end > start) {
return text.slice(start, end + 1)
}
return text
}
function getJsonFenceBody(text: string): string | null {
let bodyStart = getLineBreakEnd(text, 3)
if (bodyStart === null && startsWithAsciiIgnoreCase(text, '```json', 0)) {
bodyStart = getLineBreakEnd(text, 7)
}
if (bodyStart === null || !text.endsWith('```')) {
return null
}
const closeStart = text.length - 3
const bodyEnd = getBodyEndBeforeClosingFence(text, closeStart)
return bodyEnd === null ? null : text.slice(bodyStart, bodyEnd)
}
function getLineBreakEnd(text: string, index: number): number | null {
const code = text.charCodeAt(index)
if (code === 10) {
return index + 1
}
if (code === 13) {
return text.charCodeAt(index + 1) === 10 ? index + 2 : index + 1
}
return null
}
function getBodyEndBeforeClosingFence(text: string, closeStart: number): number | null {
const previousCode = text.charCodeAt(closeStart - 1)
if (previousCode === 10) {
return text.charCodeAt(closeStart - 2) === 13 ? closeStart - 2 : closeStart - 1
}
if (previousCode === 13) {
return closeStart - 1
}
return null
}
function startsWithAsciiIgnoreCase(value: string, search: string, startIndex: number): boolean {
if (startIndex < 0 || startIndex + search.length > value.length) {
return false
}
for (let index = 0; index < search.length; index++) {
const code = value.charCodeAt(startIndex + index)
const normalizedCode = code >= 65 && code <= 90 ? code + 32 : code
if (normalizedCode !== search.charCodeAt(index)) {
return false
}
}
return true
}
export function parseGeneratedPullRequestFields(
raw: string,
fallback: Pick<PullRequestDraftContext, 'base' | 'currentTitle' | 'currentBody' | 'currentDraft'>
): GeneratedPullRequestFields {
const content = stripJsonFence(raw)
assertJsonTextStructureWithinLimits(content, GENERATED_PULL_REQUEST_JSON_STRUCTURE_LIMITS)
const parsed = JSON.parse(content) as unknown
if (!parsed || typeof parsed !== 'object') {
throw new Error('Expected a JSON object.')
}
const record = parsed as Record<string, unknown>
const base = typeof record.base === 'string' ? record.base.trim() : fallback.base
const title =
typeof record.title === 'string' && record.title.trim()
? record.title.trim().replace(/[.]+$/g, '')
: fallback.currentTitle.trim()
const body =
typeof record.body === 'string' ? record.body.replace(/\s+$/g, '') : fallback.currentBody
const draft = typeof record.draft === 'boolean' ? record.draft : fallback.currentDraft
return {
base: base || fallback.base,
title: title || 'Update project files',
body,
draft
}
}