diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 49201fa15cc..17c8d9e6926 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 31m + + downloads: 32m @@ -15,7 +15,7 @@ downloads downloads - 31m - 31m + 32m + 32m diff --git a/src/cli/runtime-client-deferral.test.ts b/src/cli/runtime-client-deferral.test.ts index 9345283431c..658cc60f0a4 100644 --- a/src/cli/runtime-client-deferral.test.ts +++ b/src/cli/runtime-client-deferral.test.ts @@ -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 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() + } } ) diff --git a/src/main/agent-hooks/managed-agent-hook-controls.test.ts b/src/main/agent-hooks/managed-agent-hook-controls.test.ts index 6ea28d66247..dd77b5e8d20 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.test.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.test.ts @@ -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) + }) +}) diff --git a/src/main/agent-hooks/managed-agent-hook-controls.ts b/src/main/agent-hooks/managed-agent-hook-controls.ts index 64d426ae072..6c875257089 100644 --- a/src/main/agent-hooks/managed-agent-hook-controls.ts +++ b/src/main/agent-hooks/managed-agent-hook-controls.ts @@ -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, diff --git a/src/main/github/client-pr-state.test.ts b/src/main/github/client-pr-state.test.ts index 24391dc49ba..8ca2cd85b75 100644 --- a/src/main/github/client-pr-state.test.ts +++ b/src/main/github/client-pr-state.test.ts @@ -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) + }) }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 7bf17c42664..3adec83a13a 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -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' diff --git a/src/main/github/client/update/pr-ready.ts b/src/main/github/client/update/pr-ready.ts new file mode 100644 index 00000000000..40c764d290d --- /dev/null +++ b/src/main/github/client/update/pr-ready.ts @@ -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() + } +} diff --git a/src/main/gitlab/client-mr-review-actions.test.ts b/src/main/gitlab/client-mr-review-actions.test.ts index 13410290e4b..4d651bdc618 100644 --- a/src/main/gitlab/client-mr-review-actions.test.ts +++ b/src/main/gitlab/client-mr-review-actions.test.ts @@ -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', () => { diff --git a/src/main/gitlab/merge-request-draft-title.ts b/src/main/gitlab/merge-request-draft-title.ts new file mode 100644 index 00000000000..462b0010f98 --- /dev/null +++ b/src/main/gitlab/merge-request-draft-title.ts @@ -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 +} diff --git a/src/main/gitlab/merge-request-update.ts b/src/main/gitlab/merge-request-update.ts index 0c82cfcf8ca..5fe1584ce2d 100644 --- a/src/main/gitlab/merge-request-update.ts +++ b/src/main/gitlab/merge-request-update.ts @@ -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) diff --git a/src/main/index.ts b/src/main/index.ts index 79f3f53f3a5..3ed31fadecc 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -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. diff --git a/src/main/ipc/github-ipc-channel-parity.test.ts b/src/main/ipc/github-ipc-channel-parity.test.ts index a54b7cfbf19..928e0df9fbf 100644 --- a/src/main/ipc/github-ipc-channel-parity.test.ts +++ b/src/main/ipc/github-ipc-channel-parity.test.ts @@ -46,6 +46,7 @@ const EXPECTED_GITHUB_IPC_CHANNELS = [ 'gh:mergePR', 'gh:setPRAutoMerge', 'gh:updatePRState', + 'gh:markPRReadyForReview', 'gh:rerunPRChecks', 'gh:requestPRReviewers', 'gh:removePRReviewers', diff --git a/src/main/ipc/github-ipc-module-mocks.ts b/src/main/ipc/github-ipc-module-mocks.ts index a594f979ecc..4644516eed5 100644 --- a/src/main/ipc/github-ipc-module-mocks.ts +++ b/src/main/ipc/github-ipc-module-mocks.ts @@ -29,6 +29,7 @@ const CLIENT_EXPORTS = [ 'mergePR', 'setPRAutoMerge', 'updatePRState', + 'markPRReadyForReview', 'rerunPRChecks', 'requestPRReviewers', 'removePRReviewers', diff --git a/src/main/ipc/github-pr-mutation-handlers.ts b/src/main/ipc/github-pr-mutation-handlers.ts index 1d3aecc38e3..495480cde93 100644 --- a/src/main/ipc/github-pr-mutation-handlers.ts +++ b/src/main/ipc/github-pr-mutation-handlers.ts @@ -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 ( diff --git a/src/main/ipc/github-wsl-runtime-routing.test.ts b/src/main/ipc/github-wsl-runtime-routing.test.ts index e52a3537ecb..e2f30c18eba 100644 --- a/src/main/ipc/github-wsl-runtime-routing.test.ts +++ b/src/main/ipc/github-wsl-runtime-routing.test.ts @@ -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, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index be103294da5..532cd1acadc 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -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>> { const repo = await this.resolveRepoSelector(repoSelector) @@ -23663,6 +23665,21 @@ export class OrcaRuntimeService { ) } + async markRepoPRReadyForReview( + repoSelector: string, + prNumber: number, + prRepo?: GitHubOwnerRepo | null + ): Promise>> { + 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, diff --git a/src/main/runtime/rpc/methods/github-pull-request-update-methods.ts b/src/main/runtime/rpc/methods/github-pull-request-update-methods.ts index 1c413b5571c..4d34b89420e 100644 --- a/src/main/runtime/rpc/methods/github-pull-request-update-methods.ts +++ b/src/main/runtime/rpc/methods/github-pull-request-update-methods.ts @@ -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, diff --git a/src/main/runtime/rpc/methods/github.test.ts b/src/main/runtime/rpc/methods/github.test.ts index 72a4e150077..d1bf9946806 100644 --- a/src/main/runtime/rpc/methods/github.test.ts +++ b/src/main/runtime/rpc/methods/github.test.ts @@ -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', diff --git a/src/main/runtime/rpc/methods/gitlab.test.ts b/src/main/runtime/rpc/methods/gitlab.test.ts index e363752ec34..a5cda8dc144 100644 --- a/src/main/runtime/rpc/methods/gitlab.test.ts +++ b/src/main/runtime/rpc/methods/gitlab.test.ts @@ -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', diff --git a/src/main/runtime/rpc/methods/gitlab.ts b/src/main/runtime/rpc/methods/gitlab.ts index 3cb1faf8be6..ac8fcad6991 100644 --- a/src/main/runtime/rpc/methods/gitlab.ts +++ b/src/main/runtime/rpc/methods/gitlab.ts @@ -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 }) diff --git a/src/preload/api/github-pull-request-api.ts b/src/preload/api/github-pull-request-api.ts index 76f407598b2..5e044286b95 100644 --- a/src/preload/api/github-pull-request-api.ts +++ b/src/preload/api/github-pull-request-api.ts @@ -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 diff --git a/src/preload/index.ts b/src/preload/index.ts index 88fd03e4e89..46a392d4e34 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -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 diff --git a/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx b/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx index 630c05b9847..6867ced65c5 100644 --- a/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx +++ b/src/renderer/src/components/artifacts/ArtifactPublishButton.test.tsx @@ -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() - 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() 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 }) diff --git a/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx b/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx index 052a2e8e95f..f07d9f41619 100644 --- a/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx +++ b/src/renderer/src/components/artifacts/ArtifactPublishButton.tsx @@ -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' )} )} diff --git a/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx b/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx new file mode 100644 index 00000000000..fcf501216ee --- /dev/null +++ b/src/renderer/src/components/right-sidebar/HostedReviewActions.draft.test.tsx @@ -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 }) =>
{children}
, + DropdownMenuItem: ({ children }: { children: ReactNode }) =>
{children}
, + DropdownMenuSeparator: () =>
+})) + +const repo = { id: 'repo-1', path: '/repo' } as Repo +const worktree = { id: 'worktree-1' } as Worktree + +function renderDraft(provider: HostedReviewActionInfo['provider']): string { + return renderToStaticMarkup( + + ) +} + +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('') + } + ) +}) diff --git a/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx b/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx index c8bc4e9e19e..4fd819463a0 100644 --- a/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx +++ b/src/renderer/src/components/right-sidebar/HostedReviewActions.tsx @@ -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 ( + void handleMarkReadyForReview()} + onCloseReview={() => void handleCloseReview()} + /> + ) + } + if (review.state === 'open') { return (
diff --git a/src/renderer/src/components/right-sidebar/HostedReviewStateActions.tsx b/src/renderer/src/components/right-sidebar/HostedReviewStateActions.tsx index 23e437b184c..5679592d37f 100644 --- a/src/renderer/src/components/right-sidebar/HostedReviewStateActions.tsx +++ b/src/renderer/src/components/right-sidebar/HostedReviewStateActions.tsx @@ -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 ?
{message}
: 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 ( +
+
+ + + + + + + + + {translate( + 'auto.components.right.sidebar.HostedReviewActions.closeDraft', + 'Close' + )}{' '} + {shortLabel} + + + +
+ +
+ ) +} + export function ClosedReviewActions({ shortLabel, stateUpdating, diff --git a/src/renderer/src/components/right-sidebar/hosted-review-github-actions.ts b/src/renderer/src/components/right-sidebar/hosted-review-github-actions.ts index 8940f8adf5b..66731e5638e 100644 --- a/src/renderer/src/components/right-sidebar/hosted-review-github-actions.ts +++ b/src/renderer/src/components/right-sidebar/hosted-review-github-actions.ts @@ -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>> { + 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>>( + 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 + }) +} diff --git a/src/renderer/src/components/right-sidebar/hosted-review-gitlab-actions.ts b/src/renderer/src/components/right-sidebar/hosted-review-gitlab-actions.ts new file mode 100644 index 00000000000..bc36773d199 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/hosted-review-gitlab-actions.ts @@ -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>> { + 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>>( + { 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 } + }) +} diff --git a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx index daf0dffe1b7..c7012ff4430 100644 --- a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx +++ b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx @@ -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 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, diff --git a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts index 44b8eb7940f..caad7c8e893 100644 --- a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts +++ b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts @@ -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 }): { merging: boolean + readying: boolean stateUpdating: 'open' | 'closed' | null actionError: string | null handleMerge: (method?: GitHubPRMergeMethod) => Promise handleAutoMerge: () => Promise + handleMarkReadyForReview: () => Promise handleCloseReview: () => Promise handleReopenReview: () => Promise } { @@ -61,6 +64,16 @@ export function useHostedReviewActions({ const [merging, setMerging] = useState(false) const [stateUpdating, setStateUpdating] = useState<'open' | 'closed' | null>(null) const [actionError, setActionError] = useState(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 } diff --git a/src/renderer/src/components/right-sidebar/use-ready-hosted-review-action.ts b/src/renderer/src/components/right-sidebar/use-ready-hosted-review-action.ts new file mode 100644 index 00000000000..d5076531561 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/use-ready-hosted-review-action.ts @@ -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 + setActionError: (message: string | null) => void +}): { + readying: boolean + handleMarkReadyForReview: () => Promise +} { + 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 } +} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 90211dc914b..b3bdbfbcc65 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -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.", diff --git a/src/renderer/src/web/preload-api/web-github-api.ts b/src/renderer/src/web/preload-api/web-github-api.ts index ce22933ab30..dfc955a0768 100644 --- a/src/renderer/src/web/preload-api/web-github-api.ts +++ b/src/renderer/src/web/preload-api/web-github-api.ts @@ -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 @@ -83,6 +87,16 @@ export function createGitHubApi(): WebGitHubApi { updatePRTitle: (args) => route>(GITHUB_WEB_RPC_METHODS.updatePRTitle, args), mergePR: (args) => route>(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>( + GITHUB_WEB_RPC_METHODS.markPRReadyForReview, + args + ) + }, setPRAutoMerge: (args) => route>(GITHUB_WEB_RPC_METHODS.setPRAutoMerge, args), updatePRState: (args) => diff --git a/src/renderer/src/web/preload-api/web-github-routes.ts b/src/renderer/src/web/preload-api/web-github-routes.ts index ce688832ec5..49c85b58246 100644 --- a/src/renderer/src/web/preload-api/web-github-routes.ts +++ b/src/renderer/src/web/preload-api/web-github-routes.ts @@ -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', diff --git a/src/renderer/src/web/preload-api/web-gitlab-api.ts b/src/renderer/src/web/preload-api/web-gitlab-api.ts index a183ee88df2..de07c1ad428 100644 --- a/src/renderer/src/web/preload-api/web-gitlab-api.ts +++ b/src/renderer/src/web/preload-api/web-gitlab-api.ts @@ -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 @@ -49,7 +53,15 @@ export function createGitLabApi(): WebGitLabApi { state: 'opened' }), mergeMR: (args) => route>(GITLAB_WEB_RPC_METHODS.mergeMR, args), - updateMR: (args) => route>(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>(GITLAB_WEB_RPC_METHODS.updateMR, args) + }, updateMRReviewers: (args) => route>(GITLAB_WEB_RPC_METHODS.updateMRReviewers, args), addMRComment: (args) => diff --git a/src/renderer/src/web/web-preload-api-github.test.ts b/src/renderer/src/web/web-preload-api-github.test.ts index 3483ab8fd40..bd90429736a 100644 --- a/src/renderer/src/web/web-preload-api-github.test.ts +++ b/src/renderer/src/web/web-preload-api-github.test.ts @@ -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> { + 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> { + 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 }]) + }) }) diff --git a/src/renderer/src/web/web-preload-api-gitlab.test.ts b/src/renderer/src/web/web-preload-api-gitlab.test.ts index 558c3527773..34de240bbac 100644 --- a/src/renderer/src/web/web-preload-api-gitlab.test.ts +++ b/src/renderer/src/web/web-preload-api-gitlab.test.ts @@ -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> { + 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', () => ({ diff --git a/src/shared/gitlab-types.ts b/src/shared/gitlab-types.ts index 56a506a0f5a..1b709d35ee9 100644 --- a/src/shared/gitlab-types.ts +++ b/src/shared/gitlab-types.ts @@ -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. diff --git a/src/shared/hosted-review-ready-capabilities.test.ts b/src/shared/hosted-review-ready-capabilities.test.ts new file mode 100644 index 00000000000..88058827716 --- /dev/null +++ b/src/shared/hosted-review-ready-capabilities.test.ts @@ -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) + }) +}) diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 9a262cafd7e..bfd721ce672 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -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, diff --git a/tests/e2e/ssh-pi-compatible-agent-title.spec.ts b/tests/e2e/ssh-pi-compatible-agent-title.spec.ts index d8c28d0efd0..5cea70a9bfc 100644 --- a/tests/e2e/ssh-pi-compatible-agent-title.spec.ts +++ b/tests/e2e/ssh-pi-compatible-agent-title.spec.ts @@ -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 { - 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 { 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)