mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
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.
This commit is contained in:
@@ -2157,6 +2157,8 @@ describe('registerFilesystemHandlers', () => {
|
||||
})
|
||||
|
||||
expect(getWorktreeMeta).not.toHaveBeenCalled()
|
||||
// Why: without this the assertion below passes vacuously on an early return.
|
||||
expect(generateCommitMessageFromContextMock.mock.calls).toHaveLength(1)
|
||||
expect(generateCommitMessageFromContextMock.mock.calls[0]?.[0]).not.toHaveProperty(
|
||||
'linkedIssue'
|
||||
)
|
||||
|
||||
@@ -692,6 +692,30 @@ describe('RuntimeGitCommands', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps the cached issue number when live meta is unavailable rather than unlinked', async () => {
|
||||
const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-'))
|
||||
tempDirs.push(worktreePath)
|
||||
const context = { branch: 'main', stagedSummary: 'M\tREADME.md', stagedPatch: '+hello' }
|
||||
const params = { agentId: 'codex', model: 'gpt-5.4-mini' }
|
||||
mocks.resolveCommitMessageSettings.mockReturnValue({ ok: true, params })
|
||||
mocks.getStagedCommitContext.mockResolvedValue(context)
|
||||
mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'docs' })
|
||||
const commands = new RuntimeGitCommands({
|
||||
resolveRuntimeGitTarget: async () => ({ worktree: makeWorktree(worktreePath, 123) }),
|
||||
getRuntimeSettings: () => ({}) as GlobalSettings,
|
||||
// Why: what the host reports when its store is not initialized yet.
|
||||
getWorktreeLinkedIssue: () => undefined
|
||||
})
|
||||
|
||||
await commands.generateRuntimeCommitMessage('id:wt-1')
|
||||
|
||||
expect(mocks.generateCommitMessageFromContext).toHaveBeenCalledWith(
|
||||
{ ...context, linkedIssue: 123 },
|
||||
params,
|
||||
expect.objectContaining({ kind: 'local' })
|
||||
)
|
||||
})
|
||||
|
||||
it('reads pull-request linked issues from live meta too', async () => {
|
||||
const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-'))
|
||||
tempDirs.push(worktreePath)
|
||||
|
||||
@@ -165,17 +165,20 @@ export type RuntimeGitCommandHost = {
|
||||
* Live linked-issue read by worktree id. Resolved worktrees come from a
|
||||
* short-TTL cache, so link/unlink would otherwise lag generation; hosts that
|
||||
* implement this are authoritative, including the `null` unlinked answer.
|
||||
* Return `undefined` when metadata is unavailable (store not ready) so the
|
||||
* caller keeps the resolved worktree's cached value instead of reading it as
|
||||
* unlinked.
|
||||
*/
|
||||
getWorktreeLinkedIssue?(worktreeId: string): number | null
|
||||
getWorktreeLinkedIssue?(worktreeId: string): number | null | undefined
|
||||
}
|
||||
|
||||
export class RuntimeGitCommands {
|
||||
constructor(private readonly host: RuntimeGitCommandHost) {}
|
||||
|
||||
private linkedIssueForTarget(target: RuntimeGitTarget): number | null | undefined {
|
||||
return this.host.getWorktreeLinkedIssue
|
||||
? this.host.getWorktreeLinkedIssue(target.worktree.id)
|
||||
: target.worktree.linkedIssue
|
||||
const live = this.host.getWorktreeLinkedIssue?.(target.worktree.id)
|
||||
// Why: `undefined` means the host could not answer, not "unlinked".
|
||||
return live === undefined ? target.worktree.linkedIssue : live
|
||||
}
|
||||
|
||||
async getRuntimeGitStatus(
|
||||
|
||||
@@ -7065,8 +7065,15 @@ export class OrcaRuntimeService {
|
||||
getCommitMessageAgentEnvironment: () => this.commitMessageAgentEnv ?? undefined,
|
||||
// Why: resolved worktrees are cached for a second, so link/unlink would lag
|
||||
// generation; meta is keyed by the same id the resolver returns.
|
||||
getWorktreeLinkedIssue: (worktreeId) =>
|
||||
this.store?.getWorktreeMeta?.(worktreeId)?.linkedIssue ?? null
|
||||
getWorktreeLinkedIssue: (worktreeId) => {
|
||||
const store = this.store
|
||||
// Why: an unreadable store is "unknown", not "unlinked" — undefined keeps
|
||||
// the resolver's cached linkedIssue instead of suppressing {linkedIssue}.
|
||||
if (!store?.getWorktreeMeta) {
|
||||
return undefined
|
||||
}
|
||||
return store.getWorktreeMeta(worktreeId)?.linkedIssue ?? null
|
||||
}
|
||||
})
|
||||
|
||||
getRuntimeGitStatus: RuntimeGitCommands['getRuntimeGitStatus'] =
|
||||
|
||||
@@ -110,9 +110,8 @@ describe('buildCommitMessageGenerationParams', () => {
|
||||
expect(markup).toContain('You are generating a single git commit message.')
|
||||
})
|
||||
|
||||
// Why: the chips fall back to the synthetic `123` sample when no preview is supplied,
|
||||
// which told an unlinked workspace it would render `Fixes #123` and then produced
|
||||
// `Fixes #`. Workspace-scoped callers must preview their own value, empty included.
|
||||
// Why: with no preview the chips fall back to the synthetic `123`, promising an unlinked
|
||||
// workspace `Fixes #123` where it renders `Fixes #`.
|
||||
it.each([
|
||||
{ label: 'the linked issue', linkedIssue: 42, expected: '42' },
|
||||
{ label: 'an empty value when unlinked', linkedIssue: null, expected: '' }
|
||||
@@ -161,8 +160,8 @@ describe('buildCommitMessageGenerationParams', () => {
|
||||
expect(markup).not.toContain('linkedIssue')
|
||||
})
|
||||
|
||||
// Why: previews are chip-only. The recipe is saved repo- or globally scoped, so gating
|
||||
// Save/Generate on the active workspace's empty `{linkedIssue}` would block a global write.
|
||||
// Why: the recipe saves repo- or globally scoped, so gating on the active workspace's
|
||||
// empty `{linkedIssue}` would block a global write.
|
||||
it.each([
|
||||
{ label: 'a linked workspace', linkedIssue: 42 },
|
||||
{ label: 'an unlinked workspace', linkedIssue: null }
|
||||
@@ -172,16 +171,16 @@ describe('buildCommitMessageGenerationParams', () => {
|
||||
linkedIssue
|
||||
})
|
||||
|
||||
// Why: `disabled=""` is the only observable gate here — the "Command input is empty."
|
||||
// copy is click-driven `generationError` state, so static markup can never show it.
|
||||
// Why: the "Command input is empty." copy is click-driven state, so `disabled=""` is the
|
||||
// only gate static markup can observe.
|
||||
expect(markup).toContain('Save defaults')
|
||||
expect(markup).toContain('Generate commit message')
|
||||
expect(markup).not.toContain('disabled=""')
|
||||
})
|
||||
|
||||
it('still disables both actions for a template that renders empty for everyone', () => {
|
||||
// Why: the negative control for the two cases above — without it they pass even if the
|
||||
// buttons could never be disabled at all.
|
||||
// Why: negative control — without it the two cases above pass even if the buttons could
|
||||
// never disable at all.
|
||||
const markup = renderTextGenerationForm({ commandInputTemplate: ' ', linkedIssue: 42 })
|
||||
|
||||
expect(markup).toContain('disabled=""')
|
||||
|
||||
@@ -27,6 +27,30 @@ async function setCustomGenerator(page: Page, scriptPath: string): Promise<void>
|
||||
}, 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,
|
||||
|
||||
@@ -3,6 +3,7 @@ import { rmSync, writeFileSync } from 'node:fs'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { writeLinkedIssueEchoGenerator } from './helpers/source-control-ai-generators'
|
||||
import { waitForSessionReady } from './helpers/store'
|
||||
import { openSourceControlForWorktree } from './helpers/worktree-registration'
|
||||
|
||||
@@ -40,28 +41,6 @@ function cleanupWorktree(repoPath: string, worktreePath: string, branchName: str
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Echoes back whichever issue number reached the prompt, so the assertion covers the
|
||||
* whole chain (renderer → IPC → worktree meta → template render → agent stdin) rather
|
||||
* than any single hop.
|
||||
*/
|
||||
function writeLinkedIssueEchoGenerator(scriptPath: string): void {
|
||||
writeFileSync(
|
||||
scriptPath,
|
||||
[
|
||||
'const chunks = []',
|
||||
"process.stdin.on('data', (chunk) => chunks.push(chunk))",
|
||||
"process.stdin.on('end', () => {",
|
||||
" const prompt = Buffer.concat(chunks).toString('utf8')",
|
||||
// Why: capture the whole line, not `\d*` — a `\d*` capture matches zero digits before
|
||||
// an unexpanded `{linkedIssue}` and reports it as `empty`, hiding a literal token.
|
||||
' const match = prompt.match(/ORCA_E2E_ISSUE=([^\\r\\n]*)/)',
|
||||
" process.stdout.write(`saw-issue:${match ? match[1] || 'empty' : 'missing'}`)",
|
||||
'})'
|
||||
].join('\n')
|
||||
)
|
||||
}
|
||||
|
||||
test.describe('Source Control AI commit messages', () => {
|
||||
// Why: the unlinked case separates a real resolver from one that always returns a number,
|
||||
// and — because the generator echoes the whole line — a literal `{linkedIssue}` reaches the
|
||||
@@ -77,7 +56,7 @@ test.describe('Source Control AI commit messages', () => {
|
||||
test(`${label} the commit-message recipe`, async ({ orcaPage, testRepoPath }) => {
|
||||
const { branchName, worktreePath } = createWorktreeWithStagedChange(testRepoPath)
|
||||
const generatorPath = path.join(os.tmpdir(), `${branchName}-linked-issue-generator.cjs`)
|
||||
writeLinkedIssueEchoGenerator(generatorPath)
|
||||
writeLinkedIssueEchoGenerator(generatorPath, [' process.stdout.write(`saw-issue:${issue}`)'])
|
||||
|
||||
try {
|
||||
await waitForSessionReady(orcaPage)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { rmSync, writeFileSync } from 'node:fs'
|
||||
import { rmSync } from 'node:fs'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
@@ -7,34 +7,19 @@ import {
|
||||
openSourceControl,
|
||||
seedCreatePrComposer
|
||||
} from './helpers/source-control-ai-generation'
|
||||
import { writeLinkedIssueEchoGenerator } from './helpers/source-control-ai-generators'
|
||||
import { waitForSessionReady } from './helpers/store'
|
||||
|
||||
/**
|
||||
* Echoes back whichever issue number reached the PR prompt as the generated
|
||||
* title, so the assertion covers renderer → IPC → meta → template → agent stdin
|
||||
* for the pull-request path (the commit twin lives in source-control-commit-message-ai).
|
||||
*/
|
||||
// Why: the PR path reads the echoed issue from the generated title.
|
||||
function writeLinkedIssuePrEchoGenerator(scriptPath: string, base: string): void {
|
||||
writeFileSync(
|
||||
scriptPath,
|
||||
[
|
||||
'const chunks = []',
|
||||
"process.stdin.on('data', (chunk) => chunks.push(chunk))",
|
||||
"process.stdin.on('end', () => {",
|
||||
" const prompt = Buffer.concat(chunks).toString('utf8')",
|
||||
// Why: capture the whole line, not `\d*` — a `\d*` capture matches zero digits before
|
||||
// an unexpanded `{linkedIssue}` and reports it as `empty`, hiding a literal token.
|
||||
' const match = prompt.match(/ORCA_E2E_ISSUE=([^\\r\\n]*)/)',
|
||||
" const issue = match ? match[1] || 'empty' : 'missing'",
|
||||
' process.stdout.write(JSON.stringify({',
|
||||
` base: ${JSON.stringify(base)},`,
|
||||
' title: `saw-issue:${issue}`,',
|
||||
" body: 'linked-issue e2e body',",
|
||||
' draft: false',
|
||||
' }))',
|
||||
'})'
|
||||
].join('\n')
|
||||
)
|
||||
writeLinkedIssueEchoGenerator(scriptPath, [
|
||||
' process.stdout.write(JSON.stringify({',
|
||||
` base: ${JSON.stringify(base)},`,
|
||||
' title: `saw-issue:${issue}`,',
|
||||
" body: 'linked-issue e2e body',",
|
||||
' draft: false',
|
||||
' }))'
|
||||
])
|
||||
}
|
||||
|
||||
test.describe('Source Control AI pull request linkedIssue', () => {
|
||||
|
||||
Reference in New Issue
Block a user