Merge branch 'main' into brennanb2025/relay-completion-unverifiable-r1

This commit is contained in:
Brennan Benson
2026-08-28 14:20:27 -07:00
42 changed files with 1137 additions and 165 deletions
+4 -4
View File
@@ -1,5 +1,5 @@
<svg xmlns="http://www.w3.org/2000/svg" width="106" height="20" role="img" aria-label="downloads: 31m">
<title>downloads: 31m</title>
<svg xmlns="http://www.w3.org/2000/svg" width="106" height="20" role="img" aria-label="downloads: 32m">
<title>downloads: 32m</title>
<linearGradient id="s" x2="0" y2="100%">
<stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
<stop offset="1" stop-opacity=".1"/>
@@ -15,7 +15,7 @@
<g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" text-rendering="geometricPrecision" font-size="11">
<text x="37" y="15" fill="#010101" fill-opacity=".3">downloads</text>
<text x="37" y="14">downloads</text>
<text x="90" y="15" fill="#010101" fill-opacity=".3">31m</text>
<text x="90" y="14">31m</text>
<text x="90" y="15" fill="#010101" fill-opacity=".3">32m</text>
<text x="90" y="14">32m</text>
</g>
</svg>

Before

Width:  |  Height:  |  Size: 935 B

After

Width:  |  Height:  |  Size: 935 B

+44 -4
View File
@@ -1,11 +1,20 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { readFileSync } from 'node:fs'
import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
const { constructorArgsMock, callMock, getCliStatusMock } = vi.hoisted(() => ({
const {
applyAgentStatusHooksEnabledMock,
constructorArgsMock,
callMock,
getCliStatusMock,
testUserDataPathRef
} = vi.hoisted(() => ({
applyAgentStatusHooksEnabledMock: vi.fn(async () => []),
constructorArgsMock: vi.fn(),
callMock: vi.fn(),
getCliStatusMock: vi.fn()
getCliStatusMock: vi.fn(),
testUserDataPathRef: { current: '' }
}))
// Why: `main` reaches RuntimeClient through `await import('./runtime-client.js')`
@@ -21,6 +30,20 @@ vi.mock('./runtime/environments', async (importOriginal) => {
}
})
// Why: this suite runs the REAL `main()`, and `agent hooks off` below reaches the production
// handler, which calls removeManagedAgentHooks() against the developer's OWN ~/.claude and
// ~/.cursor — a green test run silently deleted every Orca-managed hook on the machine, so agent
// status stopped reporting until the next Orca restart (STA-5679). The byte-for-byte equivalence
// twin already refuses these tokens for exactly this reason
// (config/scripts/cli-runtime-client-deferral-equivalence.mjs); this is the same guard for vitest.
// Stubbed, not dropped: the row is the only case that reads ctx.client, so it carries the
// null-vs-undefined coverage the rest of the table cannot.
vi.mock('../main/agent-hooks/managed-agent-hook-controls', () => ({
applyAgentStatusHooksEnabled: applyAgentStatusHooksEnabledMock,
getManagedAgentHookStatuses: vi.fn(() => []),
prepareManagedCodexHomeBeforeShellLaunch: vi.fn(async () => {})
}))
vi.mock('./runtime-client', () => {
class RuntimeClient {
call = callMock
@@ -31,7 +54,7 @@ vi.mock('./runtime-client', () => {
constructorArgsMock(...args)
}
}
return { RuntimeClient, getDefaultUserDataPath: () => '/tmp/orca-user-data' }
return { RuntimeClient, getDefaultUserDataPath: () => testUserDataPathRef.current }
})
import { main } from './index'
@@ -44,6 +67,8 @@ describe('RuntimeClient module-graph deferral', () => {
let errorSpy: ReturnType<typeof vi.spyOn>
beforeEach(() => {
testUserDataPathRef.current = mkdtempSync(join(tmpdir(), 'orca-runtime-deferral-userdata-'))
applyAgentStatusHooksEnabledMock.mockClear()
constructorArgsMock.mockClear()
callMock.mockReset()
getCliStatusMock.mockReset()
@@ -55,6 +80,7 @@ describe('RuntimeClient module-graph deferral', () => {
logSpy.mockRestore()
errorSpy.mockRestore()
vi.unstubAllEnvs()
rmSync(testUserDataPathRef.current, { recursive: true, force: true })
process.exitCode = 0
})
@@ -132,6 +158,20 @@ describe('RuntimeClient module-graph deferral', () => {
expect(call[2], `${argv.join(' ')} pairing code`).toBeNull()
expect(call[3], `${argv.join(' ')} environment`).toBeNull()
}
if (argv.join(' ') === 'agent hooks off') {
expect(
applyAgentStatusHooksEnabledMock,
`${argv.join(' ')} hook application`
).toHaveBeenCalledExactlyOnceWith(false, {
agentCmdOverrides: {},
disabledTuiAgents: []
})
} else {
expect(
applyAgentStatusHooksEnabledMock,
`${argv.join(' ')} hook application`
).not.toHaveBeenCalled()
}
}
)
@@ -45,6 +45,8 @@ import {
applyAgentStatusHooksEnabled,
installManagedAgentHooks,
removeManagedAgentHooksAsync,
resolveStartupManagedHookAction,
shouldInstallStartupManagedAgentHook,
shouldContinueManagedHookStartup
} from './managed-agent-hook-controls'
@@ -273,3 +275,68 @@ describe('managed agent hook controls', () => {
expect(results).toEqual([expect.objectContaining({ agent: 'codex', state: 'not_installed' })])
})
})
describe('startup managed hook reconciliation (STA-5679)', () => {
beforeEach(() => {
vi.clearAllMocks()
})
it('skips instead of removing when this profile has the off switch set', () => {
// Why this matters: the hook files are user-global. Startup removal here deleted the hooks that
// every other Orca instance depends on, and Cursor then reads as idle with no status at all.
expect(resolveStartupManagedHookAction({ agentStatusHooksEnabled: false })).toBe('skip')
})
it('installs when hooks are enabled or the setting is unset', () => {
expect(resolveStartupManagedHookAction({ agentStatusHooksEnabled: true })).toBe('install')
expect(resolveStartupManagedHookAction({})).toBe('install')
expect(resolveStartupManagedHookAction(null)).toBe('install')
})
it('only allows startup installs for globally enabled and agent-enabled hooks', () => {
expect(shouldInstallStartupManagedAgentHook({ agentStatusHooksEnabled: false }, 'codex')).toBe(
false
)
expect(
shouldInstallStartupManagedAgentHook(
{ agentStatusHooksEnabled: true, disabledTuiAgents: ['codex'] },
'codex'
)
).toBe(false)
expect(
shouldInstallStartupManagedAgentHook(
{ agentStatusHooksEnabled: true, disabledTuiAgents: ['claude'] },
'codex'
)
).toBe(true)
})
it('does not remove disabled agents during startup install reconciliation', async () => {
const settings = {
agentStatusHooksEnabled: true,
agentCmdOverrides: {},
disabledTuiAgents: ['claude' as const]
}
mocks.detect.mockResolvedValue({ codex: { state: 'found' } })
await installManagedAgentHooks(settings, {
shouldContinue: (agent) => shouldContinueManagedHookStartup(false, settings, agent)
})
expect(mocks.removeClaude).not.toHaveBeenCalled()
expect(mocks.installClaude).not.toHaveBeenCalled()
expect(mocks.installCodex).toHaveBeenCalledTimes(1)
})
it('still removes through the explicit Settings toggle', async () => {
// Anchors the assertion above: the removers really are wired, so 'skip' is a behavioral
// difference rather than a vacuous constant.
mocks.removeClaude.mockResolvedValue(status('claude', 'not_installed'))
mocks.removeCodex.mockResolvedValue(status('codex', 'not_installed'))
await applyAgentStatusHooksEnabled(false, { agentStatusHooksEnabled: false })
expect(mocks.removeClaude).toHaveBeenCalledTimes(1)
expect(mocks.removeCodex).toHaveBeenCalledTimes(1)
})
})
@@ -41,6 +41,29 @@ export function isAgentStatusHooksEnabled(
return settings?.agentStatusHooksEnabled !== false
}
export type StartupManagedHookAction = 'install' | 'skip'
// Why never 'remove': this reads THIS instance's settings, but the managed hook files are
// user-global (~/.claude/settings.json, ~/.cursor/hooks.json). A second Orca profile with the off
// switch set would delete the hooks every other instance depends on, and Cursor — the one agent
// with no title-derived status fallback — then goes silently idle (STA-5679). Honoring the off
// switch only requires skipping the install; explicit removal stays on the Settings toggle.
export function resolveStartupManagedHookAction(
settings: ManagedHookSettings
): StartupManagedHookAction {
return isAgentStatusHooksEnabled(settings) ? 'install' : 'skip'
}
export function shouldInstallStartupManagedAgentHook(
settings: ManagedHookSettings,
agent: AgentHookTarget
): boolean {
return (
resolveStartupManagedHookAction(settings) === 'install' &&
!normalizeDisabledTuiAgents(settings?.disabledTuiAgents).includes(agent)
)
}
export function shouldContinueManagedHookStartup(
isQuitting: boolean,
settings: ManagedHookSettings,
+16 -2
View File
@@ -26,7 +26,7 @@ vi.mock('./github-api-repository', async (importOriginal) =>
)
)
import { updatePRState, _resetOwnerRepoCache } from './client'
import { markPRReadyForReview, updatePRState, _resetOwnerRepoCache } from './client'
import { resetOriginRepositoryCache } from './client-test-harness'
const {
@@ -39,7 +39,7 @@ const {
releaseMock
} = clientMocks
describe('updatePRState', () => {
describe('pull request state mutations', () => {
beforeEach(() => {
resetOriginRepositoryCache()
ghExecFileAsyncMock.mockReset()
@@ -103,4 +103,18 @@ describe('updatePRState', () => {
{ host: 'github.com' }
)
})
it('marks pull requests ready for review through gh', async () => {
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' })
await expect(markPRReadyForReview('/repo-root', 3977)).resolves.toEqual({ ok: true })
expect(ghExecFileAsyncMock).toHaveBeenCalledWith(
['pr', 'ready', '3977', '--repo', 'stablyai/orca'],
{ cwd: '/repo-root', host: 'github.com' }
)
expect(acquireMock).toHaveBeenCalledTimes(1)
expect(releaseMock).toHaveBeenCalledTimes(1)
})
})
+1
View File
@@ -33,6 +33,7 @@ export { setPRAutoMerge } from './client/merge/pr-auto-merge'
export { setPRCommentReaction } from './client/update/pr-comment-reaction'
export { setPRFileViewed } from './client/update/pr-file-viewed'
export { updatePRDetails, updatePRTitle } from './client/update/pr-details'
export { markPRReadyForReview } from './client/update/pr-ready'
export { updatePRState } from './client/update/pr-state'
export type { GitHubPRBranchLookupOptions } from './client/lookup/pull-request-lookup-data'
export type { MainWorkItem } from './client/map/work-item-field-coercion'
+41
View File
@@ -0,0 +1,41 @@
import {
acquire,
classifyPullRequestUpdateError,
ghExecFileAsync,
release,
type LocalGitExecOptions
} from '../../gh-utils'
import { resolveGitHubRepoExecution, type GitHubApiRepository } from '../../github-api-repository'
export async function markPRReadyForReview(
repoPath: string,
prNumber: number,
connectionId?: string | null,
prRepo?: GitHubApiRepository | null,
localGitOptions: LocalGitExecOptions = {}
): Promise<{ ok: true } | { ok: false; error: string }> {
const { ownerRepo, ghOptions } = await resolveGitHubRepoExecution(
repoPath,
prRepo,
connectionId,
localGitOptions
)
if (!ownerRepo) {
return { ok: false, error: 'Could not resolve GitHub owner/repo for this repository' }
}
await acquire()
try {
await ghExecFileAsync(
['pr', 'ready', String(prNumber), '--repo', `${ownerRepo.owner}/${ownerRepo.repo}`],
ghOptions
)
return { ok: true }
} catch (err) {
const message =
err instanceof Error ? err.message : typeof err === 'string' ? err : 'Unknown error'
return { ok: false, error: classifyPullRequestUpdateError(message).message }
} finally {
release()
}
}
@@ -54,6 +54,23 @@ import {
updateMRReviewers
} from './client'
import { resetGitLabMrMocks } from './client-mr-test-harness'
import { stripGitLabDraftTitlePrefix } from './merge-request-draft-title'
describe('stripGitLabDraftTitlePrefix', () => {
it.each([
['Draft: Ship it', 'Ship it'],
['wip: Ship it', 'Ship it'],
['[Draft] Ship it', 'Ship it'],
['(WIP)Ship it', 'Ship it'],
['Draft - Ship it', 'Ship it']
])('strips a GitLab draft marker from %s', (title, expected) => {
expect(stripGitLabDraftTitlePrefix(title)).toBe(expected)
})
it('leaves markerless titles unchanged', () => {
expect(stripGitLabDraftTitlePrefix('Ship it')).toBeNull()
})
})
describe('gitlab client — MR operations', () => {
beforeEach(() => {
@@ -210,6 +227,58 @@ describe('gitlab client — MR operations', () => {
{}
)
})
it('fetches the current title before marking a merge request ready', async () => {
glabExecFileAsyncMock
.mockResolvedValueOnce({ stdout: JSON.stringify({ title: 'Draft: Fresh title' }) })
.mockResolvedValueOnce({ stdout: '{}' })
await expect(
updateMR('/repo', 12, { readyForReview: true }, 'upstream', 'conn-1')
).resolves.toEqual({ ok: true })
expect(glabExecFileAsyncMock).toHaveBeenNthCalledWith(
1,
['api', '--hostname', 'git.internal', 'projects/g%2Fp/merge_requests/12'],
{}
)
expect(glabExecFileAsyncMock).toHaveBeenNthCalledWith(
2,
[
'api',
'--hostname',
'git.internal',
'-X',
'PUT',
'projects/g%2Fp/merge_requests/12',
'-f',
'title=Fresh title'
],
{}
)
})
it('treats a freshly markerless title as already ready', async () => {
glabExecFileAsyncMock.mockResolvedValueOnce({
stdout: JSON.stringify({ title: 'Fresh title' })
})
await expect(
updateMR('/repo', 12, { readyForReview: true }, 'upstream', 'conn-1')
).resolves.toEqual({ ok: true })
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(1)
})
it('rejects a draft marker that would leave an empty title', async () => {
glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: JSON.stringify({ title: 'Draft:' }) })
await expect(
updateMR('/repo', 12, { readyForReview: true }, 'upstream', 'conn-1')
).resolves.toEqual({ ok: false, error: 'Title is required' })
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(1)
})
})
describe('resolveMRDiscussion', () => {
@@ -0,0 +1,7 @@
const GITLAB_DRAFT_TITLE_PREFIX =
/^(?:(?:draft|wip)\s*:\s*|(?:draft|wip)\s+-\s+|\[(?:draft|wip)\]\s*|\((?:draft|wip)\)\s*)/i
export function stripGitLabDraftTitlePrefix(title: string): string | null {
const readyTitle = title.replace(GITLAB_DRAFT_TITLE_PREFIX, '')
return readyTitle === title ? null : readyTitle
}
+53 -28
View File
@@ -1,4 +1,5 @@
import type { IssueSourcePreference } from '../../shared/repo-types'
import type { GitLabMRUpdate } from '../../shared/gitlab-types'
import {
acquire,
classifyGlabError,
@@ -10,17 +11,13 @@ import {
type ProjectRef
} from './gl-utils'
import { encodedProject } from './project-path-encoding'
import { stripGitLabDraftTitlePrefix } from './merge-request-draft-title'
import { withProjectRef } from './merge-request-project-resolution'
export async function updateMR(
repoPath: string,
iid: number,
updates: {
title?: string
body?: string
addLabels?: string[]
removeLabels?: string[]
},
updates: GitLabMRUpdate,
preference?: IssueSourcePreference,
connectionId?: string | null,
projectRef?: ProjectRef | null,
@@ -32,38 +29,66 @@ export async function updateMR(
connectionId,
projectRef,
async (projectRef) => {
const fields: string[] = []
const title = updates.title?.trim()
if (updates.title !== undefined) {
if (!title) {
return { ok: false, error: 'Title is required' }
}
fields.push(`title=${title}`)
}
if (updates.body !== undefined) {
fields.push(`description=${updates.body}`)
}
const addLabels = (updates.addLabels ?? []).filter((label) => label.trim().length > 0)
const removeLabels = (updates.removeLabels ?? []).filter((label) => label.trim().length > 0)
if (addLabels.length > 0) {
fields.push(`add_labels=${addLabels.join(',')}`)
}
if (removeLabels.length > 0) {
fields.push(`remove_labels=${removeLabels.join(',')}`)
}
if (fields.length === 0) {
if (
!updates.readyForReview &&
updates.title === undefined &&
updates.body === undefined &&
!(updates.addLabels ?? []).some((label) => label.trim()) &&
!(updates.removeLabels ?? []).some((label) => label.trim())
) {
return { ok: true }
}
await acquire()
try {
if (updates.readyForReview && updates.title !== undefined) {
return { ok: false, error: 'Cannot update the title while marking a merge request ready' }
}
const endpoint = `projects/${encodedProject(projectRef.path)}/merge_requests/${iid}`
let title = updates.title?.trim()
if (updates.readyForReview) {
const response = await glabExecFileAsync(
['api', ...glabHostnameArgs(projectRef, connectionId), endpoint],
glabRepoExecOptions(repoPath, connectionId, localGitOptions)
)
const currentTitle = (JSON.parse(response.stdout) as { title?: unknown }).title
if (typeof currentTitle !== 'string') {
return { ok: false, error: 'Could not read the current merge request title' }
}
title = stripGitLabDraftTitlePrefix(currentTitle) ?? undefined
}
const fields: string[] = []
if (title !== undefined) {
if (!title.trim()) {
return { ok: false, error: 'Title is required' }
}
fields.push(`title=${title.trim()}`)
} else if (updates.title !== undefined) {
return { ok: false, error: 'Title is required' }
}
if (updates.body !== undefined) {
fields.push(`description=${updates.body}`)
}
const addLabels = (updates.addLabels ?? []).filter((label) => label.trim().length > 0)
const removeLabels = (updates.removeLabels ?? []).filter((label) => label.trim().length > 0)
if (addLabels.length > 0) {
fields.push(`add_labels=${addLabels.join(',')}`)
}
if (removeLabels.length > 0) {
fields.push(`remove_labels=${removeLabels.join(',')}`)
}
if (fields.length === 0) {
return { ok: true }
}
await glabExecFileAsync(
[
'api',
...glabHostnameArgs(projectRef, connectionId),
'-X',
'PUT',
`projects/${encodedProject(projectRef.path)}/merge_requests/${iid}`,
endpoint,
...fields.flatMap((field) => ['-f', field])
],
glabRepoExecOptions(repoPath, connectionId, localGitOptions)
+35 -31
View File
@@ -89,10 +89,11 @@ import {
sweepRestoredSubagentsWithoutLiveAgent
} from './agent-hooks/restored-subagent-liveness-sweep'
import {
applyAgentStatusHooksEnabled,
installManagedAgentHooks,
isAgentStatusHooksEnabled,
removeManagedAgentHooks,
removeManagedAgentHooksAsync,
resolveStartupManagedHookAction,
shouldInstallStartupManagedAgentHook,
shouldContinueManagedHookStartup
} from './agent-hooks/managed-agent-hook-controls'
import { initCohortClassifier } from './telemetry/cohort-classifier'
@@ -3089,37 +3090,40 @@ void app.whenReady().then(async () => {
// ordered before managed-hook reconciliation — an incapable host must re-arm
// and complete the legacy real-home sweep first — but awaiting it inline
// stalled app init behind that session, so chain instead of blocking.
const realHomeCodexHookState = codexRuntimeHome.isHostSystemDefaultRealHomeSelected()
? ensureRealHomeCodexHookState({
hooksEnabled: isAgentStatusHooksEnabled(store.getSettings()),
userDataPath: app.getPath('userData')
}).catch((error: unknown) => {
console.warn('[codex-real-home-hooks] startup ensure failed:', error)
})
: Promise.resolve()
if (shouldInstallManagedHooks(is.dev)) {
// Why: check the persisted off switch before any auto-install so removed hooks don't silently reappear on launch.
if (isAgentStatusHooksEnabled(store.getSettings())) {
const managedHookStore = store
void realHomeCodexHookState
.then(() =>
applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), {
shouldHydrateShellPath: app.isPackaged,
onInstallError: recordManagedHookInstallFailure,
shouldContinue: (agent) => {
const settings = managedHookStore.getSettings()
return shouldContinueManagedHookStartup(isQuitting, settings, agent)
}
})
)
.catch((error: unknown) => {
console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error)
const startupManagedHookSettings = store.getSettings()
const shouldReconcileStartupManagedHooks =
shouldInstallManagedHooks(is.dev) &&
resolveStartupManagedHookAction(startupManagedHookSettings) === 'install'
const realHomeCodexHookState =
shouldReconcileStartupManagedHooks &&
shouldInstallStartupManagedAgentHook(startupManagedHookSettings, 'codex') &&
codexRuntimeHome.isHostSystemDefaultRealHomeSelected()
? ensureRealHomeCodexHookState({
hooksEnabled: true,
userDataPath: app.getPath('userData')
}).catch((error: unknown) => {
console.warn('[codex-real-home-hooks] startup ensure failed:', error)
})
} else {
void removeManagedAgentHooks().catch((error: unknown) => {
console.warn('[agent-hooks] failed to remove managed hooks on startup:', error)
: Promise.resolve()
// Why skip rather than remove when the off switch is set: the hook files are user-global but this
// decision reads only THIS profile's settings, so removing here deletes the hooks every other Orca
// instance depends on (STA-5679). Skipping already keeps removed hooks from reappearing on launch.
if (shouldReconcileStartupManagedHooks) {
const managedHookStore = store
void realHomeCodexHookState
.then(() =>
installManagedAgentHooks(managedHookStore.getSettings(), {
shouldHydrateShellPath: app.isPackaged,
onInstallError: recordManagedHookInstallFailure,
shouldContinue: (agent) => {
const settings = managedHookStore.getSettings()
return shouldContinueManagedHookStartup(isQuitting, settings, agent)
}
})
)
.catch((error: unknown) => {
console.warn('[agent-hooks] failed to reconcile managed hooks on startup:', error)
})
}
}
// Why: process-gone metrics only see survivors; retain a recent whole-app
// snapshot for comparison in crash reports.
@@ -46,6 +46,7 @@ const EXPECTED_GITHUB_IPC_CHANNELS = [
'gh:mergePR',
'gh:setPRAutoMerge',
'gh:updatePRState',
'gh:markPRReadyForReview',
'gh:rerunPRChecks',
'gh:requestPRReviewers',
'gh:removePRReviewers',
+1
View File
@@ -29,6 +29,7 @@ const CLIENT_EXPORTS = [
'mergePR',
'setPRAutoMerge',
'updatePRState',
'markPRReadyForReview',
'rerunPRChecks',
'requestPRReviewers',
'removePRReviewers',
@@ -2,6 +2,7 @@ import { ipcMain } from 'electron'
import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types'
import type { GitHubPullRequestStateUpdate } from '../../shared/issue-mutation-types'
import {
markPRReadyForReview,
mergePR,
removePRReviewers,
requestPRReviewers,
@@ -133,6 +134,35 @@ export function registerGitHubPRMutationHandlers(store: Store): void {
}
)
ipcMain.handle(
'gh:markPRReadyForReview',
async (
event,
args: GitHubRepoScopedArgs & {
prNumber: number
prRepo?: GitHubOwnerRepo | null
}
) => {
const repo = assertRegisteredGitHubRepo(args, store)
if (
typeof args.prNumber !== 'number' ||
!Number.isInteger(args.prNumber) ||
args.prNumber < 1
) {
return { ok: false, error: 'Invalid pull request number' }
}
const result = await markPRReadyForReview(
repo.path,
args.prNumber,
getGitHubRepoConnectionId(repo),
args.prRepo ?? null,
...getGitHubLocalGitOptionArgs(store, repo)
)
broadcastSuccessfulPRMutation(result.ok, repo.path, repo.id, args.prNumber, event.sender.id)
return result
}
)
ipcMain.handle(
'gh:rerunPRChecks',
async (
@@ -41,6 +41,7 @@ const {
mergePR: mergePRMock,
setPRAutoMerge: setPRAutoMergeMock,
updatePRState: updatePRStateMock,
markPRReadyForReview: markPRReadyForReviewMock,
rerunPRChecks: rerunPRChecksMock,
requestPRReviewers: requestPRReviewersMock,
removePRReviewers: removePRReviewersMock
@@ -246,6 +247,7 @@ describe('registerGitHubHandlers', () => {
mergePRMock.mockResolvedValue({ ok: true })
setPRAutoMergeMock.mockResolvedValue({ ok: true })
updatePRStateMock.mockResolvedValue({ ok: true })
markPRReadyForReviewMock.mockResolvedValue({ ok: true })
rerunPRChecksMock.mockResolvedValue({ ok: true, count: 1 })
requestPRReviewersMock.mockResolvedValue({ ok: true })
removePRReviewersMock.mockResolvedValue({ ok: true })
@@ -381,6 +383,14 @@ describe('registerGitHubHandlers', () => {
prRepo
}
)
await handlers['gh:markPRReadyForReview'](
{ sender: { id: 1 } },
{
repoPath: '/workspace/repo',
prNumber: 42,
prRepo
}
)
await handlers['gh:rerunPRChecks'](null, {
repoPath: '/workspace/repo',
prNumber: 42,
@@ -536,6 +546,13 @@ describe('registerGitHubHandlers', () => {
prRepo,
localGitOptions
)
expect(markPRReadyForReviewMock).toHaveBeenCalledWith(
'/workspace/repo',
42,
null,
prRepo,
localGitOptions
)
expect(rerunPRChecksMock).toHaveBeenCalledWith(
'/workspace/repo',
42,
+18 -1
View File
@@ -293,6 +293,7 @@ import type { ListWorkItemsResult } from '../../shared/github/work-item-types'
import type {
GitLabIssueUpdate,
GitLabMRInlineCommentInput,
GitLabMRUpdate,
GitLabProjectRef,
GitLabWorkItem,
MRListState
@@ -821,6 +822,7 @@ import {
updatePRTitle,
updatePRDetails,
mergePR,
markPRReadyForReview,
setPRAutoMerge,
updatePRState,
requestPRReviewers,
@@ -23347,7 +23349,7 @@ export class OrcaRuntimeService {
async updateGitLabRepoMR(
repoSelector: string,
iid: number,
updates: { title?: string; body?: string; addLabels?: string[]; removeLabels?: string[] },
updates: GitLabMRUpdate,
projectRef?: GitLabProjectRef | null
): Promise<Awaited<ReturnType<typeof updateGitLabMR>>> {
const repo = await this.resolveRepoSelector(repoSelector)
@@ -23663,6 +23665,21 @@ export class OrcaRuntimeService {
)
}
async markRepoPRReadyForReview(
repoSelector: string,
prNumber: number,
prRepo?: GitHubOwnerRepo | null
): Promise<Awaited<ReturnType<typeof markPRReadyForReview>>> {
const repo = await this.resolveRepoSelector(repoSelector)
return markPRReadyForReview(
repo.path,
prNumber,
repo.connectionId ?? null,
prRepo ?? null,
...this.getLocalGitExecutionOptionArgs(repo)
)
}
async updateRepoPRState(
repoSelector: string,
prNumber: number,
@@ -39,6 +39,11 @@ const UpdatePrState = RepoSelector.extend({
})
})
const MarkPrReadyForReview = RepoSelector.extend({
prNumber: z.number().int().positive(),
prRepo: SlugRepo.nullable().optional()
})
const RequestPrReviewers = RepoSelector.extend({
prNumber: z.number().int().positive(),
prRepo: SlugRepo.nullable().optional(),
@@ -113,6 +118,12 @@ export const GITHUB_PULL_REQUEST_UPDATE_METHODS: RpcMethod[] = [
handler: async (params, { runtime }) =>
runtime.updateRepoPRState(params.repo, params.prNumber, params.updates, params.prRepo ?? null)
}),
defineMethod({
name: 'github.markPRReadyForReview',
params: MarkPrReadyForReview,
handler: async (params, { runtime }) =>
runtime.markRepoPRReadyForReview(params.repo, params.prNumber, params.prRepo ?? null)
}),
defineMethod({
name: 'github.requestPRReviewers',
params: RequestPrReviewers,
@@ -454,6 +454,28 @@ describe('github RPC methods', () => {
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it('marks PRs ready for review on the runtime server', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
markRepoPRReadyForReview: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: GITHUB_METHODS })
const response = await dispatcher.dispatch(
makeRequest('github.markPRReadyForReview', {
repo: 'repo-1',
prNumber: 7,
prRepo: { owner: 'acme', repo: 'widgets' }
})
)
expect(runtime.markRepoPRReadyForReview).toHaveBeenCalledWith('repo-1', 7, {
owner: 'acme',
repo: 'widgets'
})
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it('routes PR reviewer mutations on the runtime server', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
@@ -281,6 +281,30 @@ describe('gitlab RPC methods', () => {
)
})
it('accepts the negotiated ready-for-review update field', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
updateGitLabRepoMR: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: GITLAB_METHODS })
const response = await dispatcher.dispatch(
makeRequest('gitlab.updateMR', {
repo: 'id:repo-1',
iid: 8,
updates: { readyForReview: true }
})
)
expect(runtime.updateGitLabRepoMR).toHaveBeenCalledWith(
'id:repo-1',
8,
{ readyForReview: true },
undefined
)
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it('normalizes GitLab issue list arguments to match desktop preload behavior', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
+2 -1
View File
@@ -73,7 +73,8 @@ const UpdateMr = RepoSelector.extend({
title: z.string().optional(),
body: z.string().optional(),
addLabels: z.array(z.string()).optional(),
removeLabels: z.array(z.string()).optional()
removeLabels: z.array(z.string()).optional(),
readyForReview: z.literal(true).optional()
}),
projectRef: GitLabProjectRef
})
@@ -155,6 +155,12 @@ export type GithubPullRequestApi = {
prRepo?: GitHubOwnerRepo | null
}
) => Promise<{ ok: true } | { ok: false; error: string }>
markPRReadyForReview: (
args: GitHubRepoSelectorArgs & {
prNumber: number
prRepo?: GitHubOwnerRepo | null
}
) => Promise<{ ok: true } | { ok: false; error: string }>
requestPRReviewers: (
args: GitHubRepoSelectorArgs & {
prNumber: number
+9
View File
@@ -1668,6 +1668,15 @@ const api = {
}): Promise<{ ok: true } | { ok: false; error: string }> =>
ipcRenderer.invoke('gh:updatePRState', args),
markPRReadyForReview: (args: {
repoPath: string
repoId?: string
sourceContext?: TaskSourceContext | null
prNumber: number
prRepo?: GitHubOwnerRepo | null
}): Promise<{ ok: true } | { ok: false; error: string }> =>
ipcRenderer.invoke('gh:markPRReadyForReview', args),
requestPRReviewers: (args: {
repoPath: string
repoId?: string
@@ -97,7 +97,7 @@ describe('ArtifactPublishButton', () => {
await user.click(screen.getByRole('button', { name: 'Share as artifact' }))
expect(mocks.publish).not.toHaveBeenCalled()
await user.click(await screen.findByRole('button', { name: 'Share public link' }))
await user.click(await screen.findByRole('button', { name: 'Generate link' }))
await waitFor(() => expect(mocks.publish).toHaveBeenCalledWith(createRequest))
expect(screen.getByText('https://example.com')).toBeInTheDocument()
expect(screen.getByRole('button', { name: 'Update shared content' })).toBeInTheDocument()
@@ -108,7 +108,7 @@ describe('ArtifactPublishButton', () => {
mocks.state.orcaProfileAuthStatus = { configured: true, state: 'local' }
render(<ArtifactPublishButton sourceKey="/repo/report.md" createRequest={vi.fn()} />)
expect(await screen.findByRole('button', { name: 'Share public link' })).toBeDisabled()
expect(await screen.findByRole('button', { name: 'Generate link' })).toBeDisabled()
await user.click(screen.getByRole('button', { name: 'Sign in' }))
expect(mocks.connect).toHaveBeenCalledOnce()
@@ -121,7 +121,7 @@ describe('ArtifactPublishButton', () => {
render(<ArtifactPublishButton sourceKey="/repo/report.md" createRequest={vi.fn()} />)
await user.click(screen.getByRole('button', { name: 'Share as artifact' }))
expect(await screen.findByRole('button', { name: 'Share public link' })).toBeDisabled()
expect(await screen.findByRole('button', { name: 'Generate link' })).toBeDisabled()
await user.click(screen.getByRole('button', { name: 'Open Artifacts settings' }))
expect(mocks.openSettingsTarget).toHaveBeenCalledWith({ pane: 'artifacts', repoId: null })
@@ -284,7 +284,7 @@ export function ArtifactPublishButton({
? translate('auto.components.artifacts.ArtifactPublishButton.sharing', 'Sharing…')
: translate(
'auto.components.artifacts.ArtifactPublishButton.sharePublicLink',
'Share public link'
'Generate link'
)}
</Button>
)}
@@ -0,0 +1,75 @@
import type { ReactNode } from 'react'
import { renderToStaticMarkup } from 'react-dom/server'
import { describe, expect, it, vi } from 'vitest'
import type { Repo } from '../../../../shared/repo-types'
import type { Worktree } from '../../../../shared/worktree/types'
import HostedReviewActions from './HostedReviewActions'
import type { HostedReviewActionInfo } from './use-hosted-review-actions'
const actionMocks = vi.hoisted(() => ({
handleMarkReadyForReview: vi.fn(),
handleCloseReview: vi.fn()
}))
vi.mock('@/store', () => ({ useAppStore: () => false }))
vi.mock('./use-hosted-review-actions', () => ({
useHostedReviewActions: () => ({
merging: false,
readying: false,
stateUpdating: null,
actionError: null,
handleMerge: vi.fn(),
handleAutoMerge: vi.fn(),
handleMarkReadyForReview: actionMocks.handleMarkReadyForReview,
handleCloseReview: actionMocks.handleCloseReview,
handleReopenReview: vi.fn()
})
}))
vi.mock('@/components/ui/dropdown-menu', () => ({
DropdownMenu: ({ children }: { children: ReactNode }) => <>{children}</>,
DropdownMenuTrigger: ({ children }: { children: ReactNode }) => <>{children}</>,
DropdownMenuContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DropdownMenuItem: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DropdownMenuSeparator: () => <hr />
}))
const repo = { id: 'repo-1', path: '/repo' } as Repo
const worktree = { id: 'worktree-1' } as Worktree
function renderDraft(provider: HostedReviewActionInfo['provider']): string {
return renderToStaticMarkup(
<HostedReviewActions
review={{
provider,
number: 42,
state: 'draft',
status: 'success',
mergeable: 'UNKNOWN'
}}
repo={repo}
worktree={worktree}
onRefreshReview={vi.fn().mockResolvedValue(undefined)}
/>
)
}
describe('HostedReviewActions draft state', () => {
it.each([
['github', 'PR'],
['gitlab', 'MR']
] as const)('renders Ready as primary and Close as secondary for %s', (provider, shortLabel) => {
const markup = renderDraft(provider)
expect(markup).toContain('Mark ready for review')
expect(markup).toContain(`Close ${shortLabel}`)
expect(markup).not.toContain('Merge')
expect(markup).not.toContain('auto-merge')
})
it.each(['azure-devops', 'gitea'] as const)(
'does not misroute unsupported %s drafts through GitHub',
(provider) => {
expect(renderDraft(provider)).toBe('')
}
)
})
@@ -21,6 +21,7 @@ import { getDeleteStateForWorktreeHost } from '../sidebar/worktree-delete-state-
import { presentGitLabMRMergeState } from './gitlab-mr-merge-state'
import {
ClosedReviewActions,
DraftReviewActions,
HostedReviewActionError,
MergedReviewActions
} from './HostedReviewStateActions'
@@ -123,10 +124,12 @@ export default function HostedReviewActions({
)
const {
merging,
readying,
stateUpdating,
actionError,
handleMerge,
handleAutoMerge,
handleMarkReadyForReview,
handleCloseReview,
handleReopenReview
} = useHostedReviewActions({
@@ -155,6 +158,21 @@ export default function HostedReviewActions({
runWorktreeDelete(worktree.id, worktree.hostId ? { expectedHostId: worktree.hostId } : {})
}, [worktree.hostId, worktree.id])
if (review.state === 'draft' && (review.provider === 'github' || review.provider === 'gitlab')) {
return (
<DraftReviewActions
shortLabel={shortLabel}
reviewLabel={reviewLabel}
isGitLab={isGitLab}
readying={readying}
stateUpdating={stateUpdating}
actionError={actionError}
onMarkReadyForReview={() => void handleMarkReadyForReview()}
onCloseReview={() => void handleCloseReview()}
/>
)
}
if (review.state === 'open') {
return (
<div className="space-y-1.5">
@@ -1,6 +1,24 @@
import { CircleDot, LoaderCircle, Trash2 } from 'lucide-react'
import {
ChevronDown,
CircleDot,
GitMerge,
GitPullRequestArrow,
GitPullRequestClosed,
LoaderCircle,
Trash2
} from 'lucide-react'
import { Button } from '@/components/ui/button'
import { translate } from '@/i18n/i18n'
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuTrigger
} from '@/components/ui/dropdown-menu'
import {
RIGHT_SIDEBAR_PRIMARY_BUTTON_LABEL_CLASS,
RIGHT_SIDEBAR_SPLIT_ACTION_ROW_CLASS
} from './right-sidebar-primary-action-layout'
export function HostedReviewActionError({
message
@@ -10,6 +28,91 @@ export function HostedReviewActionError({
return message ? <div className="text-[10px] text-rose-500 break-words">{message}</div> : null
}
export function DraftReviewActions({
shortLabel,
reviewLabel,
isGitLab,
readying,
stateUpdating,
actionError,
onMarkReadyForReview,
onCloseReview
}: {
shortLabel: string
reviewLabel: string
isGitLab: boolean
readying: boolean
stateUpdating: 'open' | 'closed' | null
actionError: string | null
onMarkReadyForReview: () => void
onCloseReview: () => void
}): React.JSX.Element {
const disabled = readying || stateUpdating !== null
const ReadyIcon = isGitLab ? GitMerge : GitPullRequestArrow
return (
<div className="space-y-1.5">
<div className={RIGHT_SIDEBAR_SPLIT_ACTION_ROW_CLASS}>
<Button
type="button"
size="xs"
className="min-w-0 rounded-r-none px-3 text-[11px] disabled:cursor-not-allowed"
onClick={onMarkReadyForReview}
disabled={disabled}
>
{readying ? (
<LoaderCircle className="size-3.5 animate-spin" />
) : (
<ReadyIcon className="size-3.5" />
)}
<span className={RIGHT_SIDEBAR_PRIMARY_BUTTON_LABEL_CLASS}>
{readying
? translate(
'auto.components.right.sidebar.HostedReviewActions.markingReady',
'Marking ready...'
)
: translate(
'auto.components.right.sidebar.HostedReviewActions.markReady',
'Mark ready for review'
)}
</span>
</Button>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
type="button"
size="xs"
className="shrink-0 rounded-l-none border-l border-primary-foreground/20 px-1.5 disabled:cursor-not-allowed"
disabled={disabled}
aria-label={translate(
'auto.components.right.sidebar.HostedReviewActions.draftMoreActions',
'More {{value0}} actions',
{ value0: reviewLabel }
)}
>
{stateUpdating === 'closed' ? (
<LoaderCircle className="size-3.5 animate-spin" />
) : (
<ChevronDown className="size-3.5" />
)}
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="w-52">
<DropdownMenuItem variant="destructive" onSelect={onCloseReview}>
<GitPullRequestClosed className="size-3.5" />
{translate(
'auto.components.right.sidebar.HostedReviewActions.closeDraft',
'Close'
)}{' '}
{shortLabel}
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
</div>
<HostedReviewActionError message={actionError} />
</div>
)
}
export function ClosedReviewActions({
shortLabel,
stateUpdating,
@@ -1,7 +1,15 @@
import type { GitHubPRMergeMethod, PRInfo } from '../../../../shared/github/pull-request-types'
import type { Repo } from '../../../../shared/repo-types'
import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared/execution-host'
import { callRuntimeRpc, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client'
import {
GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY,
GITHUB_MARK_PR_READY_UPDATE_REQUIRED_MESSAGE
} from '../../../../shared/protocol-version'
import {
assertRuntimeEnvironmentCapability,
callRuntimeRpc,
type RuntimeClientTarget
} from '@/runtime/runtime-rpc-client'
type GitHubPRRepo = PRInfo['prRepo']
@@ -104,3 +112,34 @@ export async function updateGitHubHostedReviewState(args: {
updates: { state: args.nextState }
})
}
export async function markGitHubHostedReviewReadyForReview(args: {
repo: Repo
prNumber: number
prRepo?: GitHubPRRepo | null
}): Promise<Awaited<ReturnType<typeof window.api.gh.markPRReadyForReview>>> {
const target = getGitHubActionTarget(args.repo)
if (target.kind === 'environment') {
await assertRuntimeEnvironmentCapability(
target.environmentId,
GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY,
GITHUB_MARK_PR_READY_UPDATE_REQUIRED_MESSAGE
)
return callRuntimeRpc<Awaited<ReturnType<typeof window.api.gh.markPRReadyForReview>>>(
target,
'github.markPRReadyForReview',
{
repo: args.repo.id,
prNumber: args.prNumber,
prRepo: args.prRepo ?? null
},
{ timeoutMs: 30_000 }
)
}
return window.api.gh.markPRReadyForReview({
repoPath: args.repo.path,
repoId: args.repo.id,
prNumber: args.prNumber,
prRepo: args.prRepo ?? null
})
}
@@ -0,0 +1,37 @@
import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared/execution-host'
import {
GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY,
GITLAB_READY_FOR_REVIEW_UPDATE_REQUIRED_MESSAGE
} from '../../../../shared/protocol-version'
import type { Repo } from '../../../../shared/repo-types'
import { assertRuntimeEnvironmentCapability, callRuntimeRpc } from '@/runtime/runtime-rpc-client'
export async function markGitLabHostedReviewReadyForReview(args: {
repo: Repo
mrNumber: number
}): Promise<Awaited<ReturnType<typeof window.api.gl.updateMR>>> {
const host = parseExecutionHostId(getRepoExecutionHostId(args.repo))
if (host?.kind === 'runtime') {
await assertRuntimeEnvironmentCapability(
host.environmentId,
GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY,
GITLAB_READY_FOR_REVIEW_UPDATE_REQUIRED_MESSAGE
)
return callRuntimeRpc<Awaited<ReturnType<typeof window.api.gl.updateMR>>>(
{ kind: 'environment', environmentId: host.environmentId },
'gitlab.updateMR',
{
repo: args.repo.id,
iid: args.mrNumber,
updates: { readyForReview: true }
},
{ timeoutMs: 30_000 }
)
}
return window.api.gl.updateMR({
repoPath: args.repo.path,
repoId: args.repo.id,
iid: args.mrNumber,
updates: { readyForReview: true }
})
}
@@ -12,7 +12,8 @@ const confirmationMocks = vi.hoisted(() => ({
}))
const runtimeRpcMocks = vi.hoisted(() => ({
callRuntimeRpc: vi.fn()
callRuntimeRpc: vi.fn(),
assertRuntimeEnvironmentCapability: vi.fn()
}))
vi.mock('@/components/confirmation-dialog-context', () => ({
@@ -20,7 +21,8 @@ vi.mock('@/components/confirmation-dialog-context', () => ({
}))
vi.mock('@/runtime/runtime-rpc-client', () => ({
callRuntimeRpc: runtimeRpcMocks.callRuntimeRpc
callRuntimeRpc: runtimeRpcMocks.callRuntimeRpc,
assertRuntimeEnvironmentCapability: runtimeRpcMocks.assertRuntimeEnvironmentCapability
}))
const prRepo = { host: 'github.com', owner: 'stablyai', repo: 'orca-sta1015-sandbox' }
@@ -51,12 +53,13 @@ function HookProbe(props: {
repo: Repo
onRefreshReview: () => Promise<void>
pullRequest?: PRInfo
isGitLab?: boolean
}): null {
latest = useHostedReviewActions({
review,
githubPR: props.pullRequest ?? githubPR,
repo: props.repo,
isGitLab: false,
isGitLab: props.isGitLab ?? false,
shortLabel: 'PR',
reviewLabel: 'pull request',
defaultMergeMethod: 'squash',
@@ -69,13 +72,14 @@ function HookProbe(props: {
async function renderHook(
repo: Repo,
onRefreshReview = vi.fn().mockResolvedValue(undefined),
pullRequest?: PRInfo
pullRequest?: PRInfo,
isGitLab = false
) {
const container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
await act(async () => {
root?.render(createElement(HookProbe, { repo, onRefreshReview, pullRequest }))
root?.render(createElement(HookProbe, { repo, onRefreshReview, pullRequest, isGitLab }))
})
return { onRefreshReview }
}
@@ -84,18 +88,21 @@ describe('useHostedReviewActions', () => {
beforeEach(() => {
confirmationMocks.confirm.mockReset().mockResolvedValue(true)
runtimeRpcMocks.callRuntimeRpc.mockReset().mockResolvedValue({ ok: true })
runtimeRpcMocks.assertRuntimeEnvironmentCapability.mockReset().mockResolvedValue(undefined)
latest = null
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- test-only window.api shim
;(window as any).api = {
gh: {
mergePR: vi.fn().mockResolvedValue({ ok: true }),
setPRAutoMerge: vi.fn().mockResolvedValue({ ok: true }),
updatePRState: vi.fn().mockResolvedValue({ ok: true })
updatePRState: vi.fn().mockResolvedValue({ ok: true }),
markPRReadyForReview: vi.fn().mockResolvedValue({ ok: true })
},
gl: {
mergeMR: vi.fn().mockResolvedValue({ ok: true }),
closeMR: vi.fn().mockResolvedValue({ ok: true }),
reopenMR: vi.fn().mockResolvedValue({ ok: true })
reopenMR: vi.fn().mockResolvedValue({ ok: true }),
updateMR: vi.fn().mockResolvedValue({ ok: true })
}
}
})
@@ -150,6 +157,84 @@ describe('useHostedReviewActions', () => {
expect(onRefreshReview).toHaveBeenCalledTimes(1)
})
it('gates runtime-owned ready mutations on the host capability', async () => {
const { onRefreshReview } = await renderHook(makeRepo({ executionHostId: 'runtime:env-1' }))
await act(async () => {
await latest?.handleMarkReadyForReview()
})
expect(runtimeRpcMocks.assertRuntimeEnvironmentCapability).toHaveBeenCalledWith(
'env-1',
'github.markPRReadyForReview',
expect.stringContaining('newer Orca server')
)
expect(runtimeRpcMocks.callRuntimeRpc).toHaveBeenCalledWith(
{ kind: 'environment', environmentId: 'env-1' },
'github.markPRReadyForReview',
{ repo: 'repo-1', prNumber: 1015, prRepo },
{ timeoutMs: 30_000 }
)
expect(window.api.gh.markPRReadyForReview).not.toHaveBeenCalled()
expect(onRefreshReview).toHaveBeenCalledTimes(1)
})
it('keeps local GitHub ready mutations on desktop IPC', async () => {
const githubRefresh = vi.fn().mockResolvedValue(undefined)
await renderHook(makeRepo(), githubRefresh)
await act(async () => {
await latest?.handleMarkReadyForReview()
})
expect(window.api.gh.markPRReadyForReview).toHaveBeenCalledWith({
repoPath: '/repo',
repoId: 'repo-1',
prNumber: 1015,
prRepo
})
expect(githubRefresh).toHaveBeenCalledTimes(1)
})
it('routes GitLab ready mutations through the existing update API', async () => {
const gitLabRefresh = vi.fn().mockResolvedValue(undefined)
await renderHook(makeRepo(), gitLabRefresh, undefined, true)
await act(async () => {
await latest?.handleMarkReadyForReview()
})
expect(window.api.gl.updateMR).toHaveBeenCalledWith({
repoPath: '/repo',
repoId: 'repo-1',
iid: 1015,
updates: { readyForReview: true }
})
expect(gitLabRefresh).toHaveBeenCalledTimes(1)
})
it('gates runtime-owned GitLab ready mutations on the host capability', async () => {
const gitLabRefresh = vi.fn().mockResolvedValue(undefined)
await renderHook(makeRepo({ executionHostId: 'runtime:env-1' }), gitLabRefresh, undefined, true)
await act(async () => {
await latest?.handleMarkReadyForReview()
})
expect(runtimeRpcMocks.assertRuntimeEnvironmentCapability).toHaveBeenCalledWith(
'env-1',
'gitlab.updateMR.readyForReview.v1',
expect.stringContaining('newer Orca server')
)
expect(runtimeRpcMocks.callRuntimeRpc).toHaveBeenCalledWith(
{ kind: 'environment', environmentId: 'env-1' },
'gitlab.updateMR',
{ repo: 'repo-1', iid: 1015, updates: { readyForReview: true } },
{ timeoutMs: 30_000 }
)
expect(window.api.gl.updateMR).not.toHaveBeenCalled()
expect(gitLabRefresh).toHaveBeenCalledTimes(1)
})
it('confirms the downstack merge scope before merging a registered stack', async () => {
const stackedPR = {
...githubPR,
@@ -12,6 +12,7 @@ import {
} from './hosted-review-github-actions'
import { translate } from '@/i18n/i18n'
import { buildGitHubPRStackMergeConfirmation } from './github-pr-stack-confirmation'
import { useReadyHostedReviewAction } from './use-ready-hosted-review-action'
export type HostedReviewActionInfo = Pick<
HostedReviewInfo,
@@ -50,10 +51,12 @@ export function useHostedReviewActions({
onRefreshReview: () => Promise<void>
}): {
merging: boolean
readying: boolean
stateUpdating: 'open' | 'closed' | null
actionError: string | null
handleMerge: (method?: GitHubPRMergeMethod) => Promise<void>
handleAutoMerge: () => Promise<void>
handleMarkReadyForReview: () => Promise<void>
handleCloseReview: () => Promise<void>
handleReopenReview: () => Promise<void>
} {
@@ -61,6 +64,16 @@ export function useHostedReviewActions({
const [merging, setMerging] = useState(false)
const [stateUpdating, setStateUpdating] = useState<'open' | 'closed' | null>(null)
const [actionError, setActionError] = useState<string | null>(null)
const { readying, handleMarkReadyForReview } = useReadyHostedReviewAction({
reviewNumber: review.number,
githubPR,
repo,
isGitLab,
shortLabel,
reviewLabel,
onRefreshReview,
setActionError
})
const handleMerge = useCallback(
async (method: GitHubPRMergeMethod = defaultMergeMethod) => {
@@ -253,10 +266,12 @@ export function useHostedReviewActions({
return {
merging,
readying,
stateUpdating,
actionError,
handleMerge,
handleAutoMerge,
handleMarkReadyForReview,
handleCloseReview,
handleReopenReview
}
@@ -0,0 +1,80 @@
import { useCallback, useState } from 'react'
import { toast } from 'sonner'
import type { PRInfo } from '../../../../shared/github/pull-request-types'
import type { Repo } from '../../../../shared/repo-types'
import { translate } from '@/i18n/i18n'
import { markGitHubHostedReviewReadyForReview } from './hosted-review-github-actions'
import { markGitLabHostedReviewReadyForReview } from './hosted-review-gitlab-actions'
export function useReadyHostedReviewAction({
reviewNumber,
githubPR,
repo,
isGitLab,
shortLabel,
reviewLabel,
onRefreshReview,
setActionError
}: {
reviewNumber: number
githubPR?: PRInfo | null
repo: Repo
isGitLab: boolean
shortLabel: string
reviewLabel: string
onRefreshReview: () => Promise<void>
setActionError: (message: string | null) => void
}): {
readying: boolean
handleMarkReadyForReview: () => Promise<void>
} {
const [readying, setReadying] = useState(false)
const handleMarkReadyForReview = useCallback(async () => {
if (readying) {
return
}
setReadying(true)
setActionError(null)
try {
const result = isGitLab
? await markGitLabHostedReviewReadyForReview({ repo, mrNumber: reviewNumber })
: await markGitHubHostedReviewReadyForReview({
repo,
prNumber: reviewNumber,
prRepo: githubPR?.prRepo ?? null
})
if (!result.ok) {
setActionError(result.error)
toast.error(result.error)
return
}
toast.success(
translate(
'auto.components.right.sidebar.HostedReviewActions.readyToast',
'{{value0}} marked ready for review',
{ value0: shortLabel }
)
)
await onRefreshReview()
} catch (err) {
const message =
err instanceof Error ? err.message : `Failed to mark ${reviewLabel} ready for review`
setActionError(message)
toast.error(message)
} finally {
setReadying(false)
}
}, [
githubPR?.prRepo,
isGitLab,
onRefreshReview,
readying,
repo,
reviewLabel,
reviewNumber,
setActionError,
shortLabel
])
return { readying, handleMarkReadyForReview }
}
+7 -2
View File
@@ -11682,7 +11682,12 @@
"3de88351c5": "GitHub will add this pull request and every pull request below it to the merge queue.",
"a32fe6dba6": "GitHub will merge this pull request and every pull request below it in the stack.",
"73e0e1819d": "Queueing stack...",
"e555a41d32": "Merging stack..."
"e555a41d32": "Merging stack...",
"markingReady": "Marking ready...",
"markReady": "Mark ready for review",
"draftMoreActions": "More {{value0}} actions",
"closeDraft": "Close",
"readyToast": "{{value0}} marked ready for review"
},
"PortsPanel": {
"3ea4a02a8f": "Cancel",
@@ -16307,7 +16312,7 @@
"publishingOffDescription": "Learn about public links and enable sharing in Settings.",
"openSettings": "Open Artifacts settings",
"sharing": "Sharing…",
"sharePublicLink": "Share public link",
"sharePublicLink": "Generate link",
"publishedDescription": "Anyone with this link can view the shared file.",
"checkingLink": "Checking for an existing link…",
"checkFailed": "Could not check for an existing link.",
@@ -1,9 +1,13 @@
import type { PreloadApi } from '../../../../preload/api-types'
import {
GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY,
GITHUB_MARK_PR_READY_UPDATE_REQUIRED_MESSAGE
} from '../../../../shared/protocol-version'
import { translate } from '@/i18n/i18n'
import { GITHUB_WEB_RPC_METHODS } from './web-github-routes'
import type { WebGitHubRuntimeMethod } from './web-github-routes'
import { mapRepoPathArg } from './web-review-api'
import { callRuntimeResult } from './web-runtime-calls'
import { callRuntimeResult, getRemoteRuntimeStatus } from './web-runtime-calls'
import { noopUnsubscribe } from './web-storage'
export type WebGitHubApi = NonNullable<PreloadApi['gh']>
@@ -83,6 +87,16 @@ export function createGitHubApi(): WebGitHubApi {
updatePRTitle: (args) =>
route<WebGitHubResult<'updatePRTitle'>>(GITHUB_WEB_RPC_METHODS.updatePRTitle, args),
mergePR: (args) => route<WebGitHubResult<'mergePR'>>(GITHUB_WEB_RPC_METHODS.mergePR, args),
markPRReadyForReview: async (args) => {
const status = await getRemoteRuntimeStatus().catch(() => null)
if (!status?.capabilities?.includes(GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY)) {
return { ok: false, error: GITHUB_MARK_PR_READY_UPDATE_REQUIRED_MESSAGE }
}
return route<WebGitHubResult<'markPRReadyForReview'>>(
GITHUB_WEB_RPC_METHODS.markPRReadyForReview,
args
)
},
setPRAutoMerge: (args) =>
route<WebGitHubResult<'setPRAutoMerge'>>(GITHUB_WEB_RPC_METHODS.setPRAutoMerge, args),
updatePRState: (args) =>
@@ -20,6 +20,7 @@ export type WebGitHubRouteKey =
| 'setPRFileViewed'
| 'updatePRTitle'
| 'mergePR'
| 'markPRReadyForReview'
| 'setPRAutoMerge'
| 'updatePRState'
| 'requestPRReviewers'
@@ -70,6 +71,7 @@ export type WebGitHubRuntimeMethod =
| 'github.setPRFileViewed'
| 'github.updatePRTitle'
| 'github.mergePR'
| 'github.markPRReadyForReview'
| 'github.setPRAutoMerge'
| 'github.updatePRState'
| 'github.requestPRReviewers'
@@ -120,6 +122,7 @@ export const GITHUB_WEB_RPC_METHODS = {
setPRFileViewed: 'github.setPRFileViewed',
updatePRTitle: 'github.updatePRTitle',
mergePR: 'github.mergePR',
markPRReadyForReview: 'github.markPRReadyForReview',
setPRAutoMerge: 'github.setPRAutoMerge',
updatePRState: 'github.updatePRState',
requestPRReviewers: 'github.requestPRReviewers',
@@ -1,8 +1,12 @@
import type { PreloadApi } from '../../../../preload/api-types'
import {
GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY,
GITLAB_READY_FOR_REVIEW_UPDATE_REQUIRED_MESSAGE
} from '../../../../shared/protocol-version'
import { GITLAB_WEB_RPC_METHODS } from './web-gitlab-routes'
import type { WebGitLabRuntimeMethod } from './web-gitlab-routes'
import { mapRepoPathArg } from './web-review-api'
import { callRuntimeResult } from './web-runtime-calls'
import { callRuntimeResult, getRemoteRuntimeStatus } from './web-runtime-calls'
export type WebGitLabApi = NonNullable<PreloadApi['gl']>
@@ -49,7 +53,15 @@ export function createGitLabApi(): WebGitLabApi {
state: 'opened'
}),
mergeMR: (args) => route<WebGitLabResult<'mergeMR'>>(GITLAB_WEB_RPC_METHODS.mergeMR, args),
updateMR: (args) => route<WebGitLabResult<'updateMR'>>(GITLAB_WEB_RPC_METHODS.updateMR, args),
updateMR: async (args) => {
if (args.updates.readyForReview) {
const status = await getRemoteRuntimeStatus().catch(() => null)
if (!status?.capabilities?.includes(GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY)) {
return { ok: false, error: GITLAB_READY_FOR_REVIEW_UPDATE_REQUIRED_MESSAGE }
}
}
return route<WebGitLabResult<'updateMR'>>(GITLAB_WEB_RPC_METHODS.updateMR, args)
},
updateMRReviewers: (args) =>
route<WebGitLabResult<'updateMRReviewers'>>(GITLAB_WEB_RPC_METHODS.updateMRReviewers, args),
addMRComment: (args) =>
@@ -44,6 +44,7 @@ describe('web GitHub preload API', () => {
'listLabelsBySlug',
'listProjectViews',
'listWorkItems',
'markPRReadyForReview',
'mergePR',
'notifyWorkItemMutated',
'onPRRefreshEvent',
@@ -468,7 +469,9 @@ describe('web GitHub preload API', () => {
]
expect(routeCases.map((routeCase) => routeCase.key).sort()).toEqual(
Object.keys(GITHUB_WEB_RPC_METHODS).sort()
Object.keys(GITHUB_WEB_RPC_METHODS)
.filter((key) => key !== 'markPRReadyForReview')
.sort()
)
for (const routeCase of routeCases) {
@@ -510,4 +513,74 @@ describe('web GitHub preload API', () => {
}
])
})
it('gates marking a PR ready on the paired host capability', async () => {
const runtimeCalls: { method: string; params: unknown }[] = []
vi.doMock('./web-runtime-client', () => ({
WebRuntimeClient: class {
call(method: string, params?: unknown): Promise<RuntimeRpcResponse<unknown>> {
runtimeCalls.push({ method, params })
return Promise.resolve({
id: `call-${runtimeCalls.length}`,
ok: true,
result:
method === 'status.get'
? { capabilities: ['github.markPRReadyForReview'] }
: { ok: true },
_meta: { runtimeId: 'runtime-1' }
})
}
close(): void {}
}
}))
const globals = installBrowserGlobals('Linux')
writeStoredRuntimeEnvironment(globals.storage)
const { installWebPreloadApi } = await import('./web-preload-api')
installWebPreloadApi()
await expect(
globals.window.api.gh.markPRReadyForReview({ repoPath: '/workspace/repo', prNumber: 7 })
).resolves.toEqual({ ok: true })
expect(runtimeCalls).toEqual([
{ method: 'status.get', params: undefined },
{
method: 'github.markPRReadyForReview',
params: { repoPath: '/workspace/repo', repo: '/workspace/repo', prNumber: 7 }
}
])
})
it('does not call the ready RPC on an older paired host', async () => {
const runtimeCalls: { method: string; params: unknown }[] = []
vi.doMock('./web-runtime-client', () => ({
WebRuntimeClient: class {
call(method: string, params?: unknown): Promise<RuntimeRpcResponse<unknown>> {
runtimeCalls.push({ method, params })
return Promise.resolve({
id: `call-${runtimeCalls.length}`,
ok: true,
result: { capabilities: [] },
_meta: { runtimeId: 'runtime-1' }
})
}
close(): void {}
}
}))
const globals = installBrowserGlobals('Linux')
writeStoredRuntimeEnvironment(globals.storage)
const { installWebPreloadApi } = await import('./web-preload-api')
installWebPreloadApi()
const result = await globals.window.api.gh.markPRReadyForReview({
repoPath: '/workspace/repo',
prNumber: 7
})
expect(result).toMatchObject({ ok: false })
expect(runtimeCalls).toEqual([{ method: 'status.get', params: undefined }])
})
})
@@ -363,6 +363,39 @@ describe('web GitLab preload API', () => {
])
})
it('does not send the ready semantic field to an older paired host', async () => {
const runtimeCalls: { method: string; params: unknown }[] = []
vi.doMock('./web-runtime-client', () => ({
WebRuntimeClient: class {
call(method: string, params?: unknown): Promise<RuntimeRpcResponse<unknown>> {
runtimeCalls.push({ method, params })
return Promise.resolve({
id: `call-${runtimeCalls.length}`,
ok: true,
result: method === 'status.get' ? { capabilities: [] } : { ok: true },
_meta: { runtimeId: 'runtime-1' }
})
}
close(): void {}
}
}))
const globals = installBrowserGlobals('Linux')
writeStoredRuntimeEnvironment(globals.storage)
const { installWebPreloadApi } = await import('./web-preload-api')
installWebPreloadApi()
const result = await globals.window.api.gl.updateMR({
repoPath: '/workspace/repo',
iid: 8,
updates: { readyForReview: true }
})
expect(result).toMatchObject({ ok: false })
expect(runtimeCalls).toEqual([{ method: 'status.get', params: undefined }])
})
it('exposes the GitLab task methods used by the shared Tasks page', async () => {
const runtimeCalls: { method: string; params: unknown }[] = []
vi.doMock('./web-runtime-client', () => ({
+1
View File
@@ -287,6 +287,7 @@ export type GitLabMRUpdate = {
body?: string
addLabels?: string[]
removeLabels?: string[]
readyForReview?: true
}
// Why: GitLab-native MR list filter replacing GitHub's search-DSL; 'all' maps to no state filter.
@@ -0,0 +1,13 @@
import { describe, expect, it } from 'vitest'
import {
GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY,
GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY,
RUNTIME_CAPABILITIES
} from './protocol-version'
describe('hosted review Ready capabilities', () => {
it('advertises both provider mutation contracts', () => {
expect(RUNTIME_CAPABILITIES).toContain(GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY)
expect(RUNTIME_CAPABILITIES).toContain(GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY)
})
})
+9
View File
@@ -124,6 +124,13 @@ export const AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY = 'agent-session.kimi-
export const FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY = 'files.mutation-ownership.v1' as const
export const FILE_MUTATION_OWNERSHIP_UPDATE_REQUIRED_MESSAGE =
'Remote file changes require a newer Orca server. Update the HUB and try again.'
export const GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY = 'github.markPRReadyForReview' as const
export const GITHUB_MARK_PR_READY_UPDATE_REQUIRED_MESSAGE =
'Marking a pull request ready requires a newer Orca server. Update the server and try again.'
export const GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY =
'gitlab.updateMR.readyForReview.v1' as const
export const GITLAB_READY_FOR_REVIEW_UPDATE_REQUIRED_MESSAGE =
'Marking a merge request ready requires a newer Orca server. Update the server and try again.'
export const WORKTREE_VISIBILITY_DEFAULTS_RUNTIME_CAPABILITY =
'worktree.visibility-defaults.v1' as const
export const WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY =
@@ -202,6 +209,8 @@ export const RUNTIME_CAPABILITIES = [
AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY,
AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY,
FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY,
GITHUB_MARK_PR_READY_RUNTIME_CAPABILITY,
GITLAB_READY_FOR_REVIEW_RUNTIME_CAPABILITY,
WORKTREE_VISIBILITY_DEFAULTS_RUNTIME_CAPABILITY,
WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY,
ACCOUNT_IMPORT_RUNTIME_CAPABILITY,
@@ -7,21 +7,15 @@ import {
waitForActiveTerminalManager,
waitForTerminalOutput
} from './helpers/terminal'
import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection'
import {
cleanupDockerSshRelayTarget,
DOCKER_SSH_RELAY_REMOTE_REPO_PATH,
startDockerSshRelayTarget,
type DockerSshRelayTarget
} from './helpers/docker-ssh-relay-target'
const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
type ConnectedDockerRemote = {
targetId: string
repoId: string
worktreeId: string
}
type RuntimeTerminalStatus = {
isRunningAgent: boolean
status: string | null
@@ -33,73 +27,6 @@ type RuntimeTerminalSummary = {
title: string | null
}
async function connectDockerRemote(
page: Page,
target: DockerSshRelayTarget
): Promise<ConnectedDockerRemote> {
return await page.evaluate(
async ({ target, remotePath }) => {
const store = window.__store
if (!store) {
throw new Error('Store unavailable')
}
const credentialUnsub = window.api.ssh.onCredentialRequest((request) => {
void window.api.ssh.submitCredential({ requestId: request.requestId, value: null })
})
try {
const { target: createdTarget, repoReadoptions } = await window.api.ssh.addTarget({
target: {
label: `Docker SSH Pi-Compatible Agent ${Date.now()}`,
host: '127.0.0.1',
port: target.port,
username: 'root',
identityFile: target.identityFile,
identitiesOnly: true,
relayGracePeriodSeconds: 1
}
})
store.getState().recordSshRepoReadoptions(repoReadoptions)
const state = await window.api.ssh.connect({ targetId: createdTarget.id })
if (!state || state.status !== 'connected') {
throw new Error(`SSH target did not connect: ${JSON.stringify(state)}`)
}
store.getState().setSshConnectionState(createdTarget.id, state)
const labels = new Map(store.getState().sshTargetLabels)
labels.set(createdTarget.id, createdTarget.label)
store.getState().setSshTargetLabels(labels)
const result = await window.api.repos.addRemote({
connectionId: createdTarget.id,
remotePath,
displayName: 'Docker SSH Pi-Compatible Agent'
})
if ('error' in result) {
throw new Error(result.error)
}
await store.getState().fetchRepos()
await store.getState().fetchWorktrees(result.repo.id)
const worktree = (store.getState().worktreesByRepo[result.repo.id] ?? [])[0]
if (!worktree) {
throw new Error(`No remote worktree found for ${result.repo.path}`)
}
store.getState().setActiveWorktree(worktree.id)
if ((store.getState().tabsByWorktree[worktree.id] ?? []).length === 0) {
store.getState().createTab(worktree.id)
}
store.getState().setActiveTabType('terminal')
return {
targetId: createdTarget.id,
repoId: result.repo.id,
worktreeId: worktree.id
}
} finally {
credentialUnsub()
}
},
{ target, remotePath: DOCKER_SSH_RELAY_REMOTE_REPO_PATH }
)
}
async function emitOscTitle(page: Page, ptyId: string, title: string): Promise<void> {
await sendToTerminal(page, ptyId, `printf '\\033]0;${title}\\007'\r`)
}
@@ -153,7 +80,7 @@ test.describe('Docker SSH Pi-compatible agent titles', () => {
target = startDockerSshRelayTarget(testInfo)
await waitForSessionReady(orcaPage)
await waitForActiveWorktree(orcaPage)
const remote = await connectDockerRemote(orcaPage, target)
const remote = await connectDockerSshRelayTarget(orcaPage, target)
await ensureTerminalVisible(orcaPage, 45_000)
await waitForActiveTerminalManager(orcaPage, 60_000)
const ptyId = await waitForActivePanePtyId(orcaPage, 60_000)