mirror of
https://github.com/stablyai/orca.git
synced 2026-10-27 08:02:51 +00:00
fix(gitlab): load pipeline job details in the Checks panel
GitLab pipeline jobs showed up in the Checks side panel with a status, but expanding a row always rendered "No inline details are available for this check." The expand path was wired only to GitHub's getPRCheckDetails, which needs an owner/repo plus a check-run/workflow id — none of which a GitLab job has — so it returned null. Carry the raw GitLab job id through gitLabPipelineJobsToPRChecks and, when a check has one, load its details via the existing gitlab:jobTrace IPC, adapting the trace into the shared PRCheckRunDetails shape so both the side panel and the full-details editor tab reuse the GitHub rendering path. Routes through runtime RPC for SSH/remote workspaces. Also fixes two adjacent gaps surfaced while wiring this up: - the requestCheckDetails guard now treats gitlabJobId as a valid detail source, so jobs with no web_url (e.g. manual/delayed jobs) still load; - getCheckRunTabIdentity keys GitLab jobs on the job id so same-named jobs across pipelines don't collide onto one full-details tab. Fixes #7732 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
committed by
Brennan Benson
co-authored by
Claude Opus 4.8
parent
e218dfbf8b
commit
3d3fd7fc71
@@ -38,6 +38,20 @@ describe('check-run-details-tab', () => {
|
||||
).toBe('url:https://github.com/acme/widgets/actions/runs/1')
|
||||
})
|
||||
|
||||
it('keys GitLab jobs on the job id so same-named jobs across pipelines stay distinct', () => {
|
||||
// Why: GitLab jobs share stage:name across pipeline runs and may have no
|
||||
// url, so the name fallback would collide two distinct jobs onto one tab.
|
||||
expect(
|
||||
getCheckRunTabIdentity({
|
||||
name: 'deploy: deploy',
|
||||
status: 'completed',
|
||||
conclusion: 'failure',
|
||||
url: null,
|
||||
gitlabJobId: 42
|
||||
})
|
||||
).toBe('gitlab-job:42')
|
||||
})
|
||||
|
||||
it('uses the check name for the tab label', () => {
|
||||
expect(
|
||||
getCheckRunDetailsTabLabel({
|
||||
|
||||
@@ -15,6 +15,11 @@ export function getCheckRunTabIdentity(check: PRCheckDetail): string {
|
||||
if (check.workflowRunId) {
|
||||
return `workflow-run:${check.workflowRunId}`
|
||||
}
|
||||
// Why: GitLab jobs share names across pipelines and may have no url, so key
|
||||
// on the job id first to avoid two distinct jobs colliding onto one tab.
|
||||
if (check.gitlabJobId) {
|
||||
return `gitlab-job:${check.gitlabJobId}`
|
||||
}
|
||||
if (check.url) {
|
||||
return `url:${check.url}`
|
||||
}
|
||||
|
||||
@@ -63,6 +63,7 @@ import type {
|
||||
PRComment,
|
||||
PRRefreshErrorType
|
||||
} from '../../../../shared/types'
|
||||
import { loadGitLabCheckRunDetails } from './gitlab-check-details-loader'
|
||||
import { getConnectionId } from '@/lib/connection-context'
|
||||
import {
|
||||
buildResolvePullRequestConflictsPrompt,
|
||||
@@ -1954,9 +1955,19 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
)
|
||||
|
||||
const handleLoadCheckDetails = useCallback(
|
||||
(check: PRCheckDetail) => {
|
||||
async (check: PRCheckDetail): Promise<PRCheckRunDetails | null> => {
|
||||
if (!repo) {
|
||||
return Promise.resolve(null)
|
||||
return null
|
||||
}
|
||||
// GitLab pipeline jobs have no check-run/workflow ids; load their trace
|
||||
// through `gitlab:jobTrace` instead of GitHub's check-details API.
|
||||
if (check.gitlabJobId) {
|
||||
return loadGitLabCheckRunDetails({
|
||||
repoPath: repo.path,
|
||||
repoId: repo.id,
|
||||
settings,
|
||||
check
|
||||
})
|
||||
}
|
||||
return fetchPRCheckDetails(
|
||||
repo.path,
|
||||
@@ -1970,7 +1981,7 @@ export default function ChecksPanel(): React.JSX.Element {
|
||||
{ repoId: repo.id }
|
||||
)
|
||||
},
|
||||
[fetchPRCheckDetails, pr?.prRepo, repo]
|
||||
[fetchPRCheckDetails, pr?.prRepo, repo, settings]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
|
||||
@@ -1043,7 +1043,12 @@ export function ChecksList({
|
||||
if (detailsByCheckKey[row.key]?.loading || detailsByCheckKey[row.key]?.details) {
|
||||
return
|
||||
}
|
||||
if (!row.check.checkRunId && !row.check.workflowRunId && !row.check.url) {
|
||||
if (
|
||||
!row.check.checkRunId &&
|
||||
!row.check.workflowRunId &&
|
||||
!row.check.url &&
|
||||
!row.check.gitlabJobId
|
||||
) {
|
||||
setDetailsByCheckKey((current) => ({
|
||||
...current,
|
||||
[row.key]: {
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { callRuntimeRpc } from '@/runtime/runtime-rpc-client'
|
||||
import type * as RuntimeRpcClient from '@/runtime/runtime-rpc-client'
|
||||
import { loadGitLabCheckRunDetails } from './gitlab-check-details-loader'
|
||||
import type { PRCheckDetail } from '../../../../shared/types'
|
||||
|
||||
vi.mock('@/runtime/runtime-rpc-client', async () => {
|
||||
const actual = await vi.importActual<typeof RuntimeRpcClient>('@/runtime/runtime-rpc-client')
|
||||
return {
|
||||
...actual,
|
||||
callRuntimeRpc: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
const jobTrace = vi.fn()
|
||||
|
||||
const CHECK: PRCheckDetail = {
|
||||
name: 'test: unit',
|
||||
status: 'completed',
|
||||
conclusion: 'failure',
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/1',
|
||||
gitlabJobId: 1
|
||||
}
|
||||
|
||||
describe('loadGitLabCheckRunDetails', () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(callRuntimeRpc).mockReset()
|
||||
jobTrace.mockReset()
|
||||
vi.stubGlobal('window', { api: { gl: { jobTrace } } })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
// Why: restore the real `window` so sibling suites relying on jsdom's
|
||||
// localStorage/DOM aren't broken by this stub leaking across files.
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
it('returns null for a non-GitLab check without hitting any transport', async () => {
|
||||
const { gitlabJobId: _omit, ...githubCheck } = CHECK
|
||||
|
||||
await expect(
|
||||
loadGitLabCheckRunDetails({
|
||||
repoPath: '/workspace/app',
|
||||
repoId: 'repo-1',
|
||||
settings: { activeRuntimeEnvironmentId: null },
|
||||
check: githubCheck
|
||||
})
|
||||
).resolves.toBeNull()
|
||||
expect(jobTrace).not.toHaveBeenCalled()
|
||||
expect(callRuntimeRpc).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fetches the trace over the local IPC bridge and adapts it', async () => {
|
||||
jobTrace.mockResolvedValue({ ok: true, trace: 'boom' })
|
||||
|
||||
const details = await loadGitLabCheckRunDetails({
|
||||
repoPath: '/workspace/app',
|
||||
repoId: 'repo-1',
|
||||
settings: { activeRuntimeEnvironmentId: null },
|
||||
check: CHECK
|
||||
})
|
||||
|
||||
expect(jobTrace).toHaveBeenCalledWith({
|
||||
repoPath: '/workspace/app',
|
||||
repoId: 'repo-1',
|
||||
jobId: 1
|
||||
})
|
||||
expect(callRuntimeRpc).not.toHaveBeenCalled()
|
||||
expect(details?.jobs[0]).toMatchObject({ id: 1, logTail: 'boom' })
|
||||
})
|
||||
|
||||
it('routes through runtime RPC when a runtime environment is active (SSH/remote)', async () => {
|
||||
vi.mocked(callRuntimeRpc).mockResolvedValue({ ok: true, trace: 'remote log' })
|
||||
|
||||
const details = await loadGitLabCheckRunDetails({
|
||||
repoPath: '/workspace/app',
|
||||
repoId: 'repo-1',
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' },
|
||||
check: CHECK
|
||||
})
|
||||
|
||||
expect(callRuntimeRpc).toHaveBeenCalledWith(
|
||||
{ kind: 'environment', environmentId: 'env-1' },
|
||||
'gitlab.jobTrace',
|
||||
{ repo: 'repo-1', jobId: 1 },
|
||||
{ timeoutMs: 30_000 }
|
||||
)
|
||||
expect(jobTrace).not.toHaveBeenCalled()
|
||||
expect(details?.jobs[0]?.logTail).toBe('remote log')
|
||||
})
|
||||
|
||||
it('throws the GitLab error verbatim when the trace fetch fails', async () => {
|
||||
jobTrace.mockResolvedValue({ ok: false, error: 'job log expired' })
|
||||
|
||||
await expect(
|
||||
loadGitLabCheckRunDetails({
|
||||
repoPath: '/workspace/app',
|
||||
repoId: 'repo-1',
|
||||
settings: { activeRuntimeEnvironmentId: null },
|
||||
check: CHECK
|
||||
})
|
||||
).rejects.toThrow('job log expired')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,46 @@
|
||||
import type {
|
||||
GitLabJobTraceResult,
|
||||
PRCheckDetail,
|
||||
PRCheckRunDetails
|
||||
} from '../../../../shared/types'
|
||||
import { gitLabJobTraceToCheckRunDetails } from '../../../../shared/gitlab-job-trace-check-details'
|
||||
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
|
||||
|
||||
/**
|
||||
* Loads inline details for a GitLab pipeline job by fetching its trace via
|
||||
* `gitlab:jobTrace` and adapting it into the shared `PRCheckRunDetails` shape.
|
||||
* Shared by the Checks panel and the full-details editor tab so both surfaces
|
||||
* render GitLab jobs through the same GitHub-parity rendering path.
|
||||
*
|
||||
* Returns `null` when the check is not a GitLab job (no `gitlabJobId`); throws
|
||||
* on a failed trace fetch so callers can surface the GitLab error verbatim.
|
||||
*/
|
||||
export async function loadGitLabCheckRunDetails(args: {
|
||||
repoPath: string
|
||||
repoId?: string
|
||||
settings: Parameters<typeof getActiveRuntimeTarget>[0]
|
||||
check: PRCheckDetail
|
||||
}): Promise<PRCheckRunDetails | null> {
|
||||
const jobId = args.check.gitlabJobId
|
||||
if (!jobId) {
|
||||
return null
|
||||
}
|
||||
const target = getActiveRuntimeTarget(args.settings)
|
||||
const result =
|
||||
target.kind === 'environment'
|
||||
? await callRuntimeRpc<GitLabJobTraceResult>(
|
||||
target,
|
||||
'gitlab.jobTrace',
|
||||
{ repo: args.repoId ?? args.repoPath, jobId },
|
||||
{ timeoutMs: 30_000 }
|
||||
)
|
||||
: ((await window.api.gl.jobTrace({
|
||||
repoPath: args.repoPath,
|
||||
repoId: args.repoId,
|
||||
jobId
|
||||
})) as GitLabJobTraceResult)
|
||||
if (!result.ok) {
|
||||
throw new Error(result.error || 'Failed to load GitLab job log.')
|
||||
}
|
||||
return gitLabJobTraceToCheckRunDetails(args.check, result.trace)
|
||||
}
|
||||
@@ -62,6 +62,7 @@ import {
|
||||
statRuntimePath
|
||||
} from '@/runtime/runtime-file-client'
|
||||
import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client'
|
||||
import { loadGitLabCheckRunDetails } from '@/components/right-sidebar/gitlab-check-details-loader'
|
||||
import { notifyHostOfMirroredEditorClose } from '@/runtime/close-mirrored-editor-tab'
|
||||
import { findWorktreeById, getRepoIdFromWorktreeId } from './worktree-helpers'
|
||||
import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner'
|
||||
@@ -3139,17 +3140,26 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
|
||||
}
|
||||
patch({ details: checkRunDetails.details, loading: true, error: null })
|
||||
try {
|
||||
const details = await get().fetchPRCheckDetails(
|
||||
repo.path,
|
||||
{
|
||||
checkRunId: check.checkRunId,
|
||||
workflowRunId: check.workflowRunId,
|
||||
checkName: check.name,
|
||||
url: check.url,
|
||||
prRepo: null
|
||||
},
|
||||
{ repoId: repo.id }
|
||||
)
|
||||
// GitLab pipeline jobs have no check-run/workflow ids; reload their trace
|
||||
// through `gitlab:jobTrace` instead of GitHub's check-details API.
|
||||
const details = check.gitlabJobId
|
||||
? await loadGitLabCheckRunDetails({
|
||||
repoPath: repo.path,
|
||||
repoId: repo.id,
|
||||
settings: settingsForRuntimeOwner(state.settings, file.runtimeEnvironmentId),
|
||||
check
|
||||
})
|
||||
: await get().fetchPRCheckDetails(
|
||||
repo.path,
|
||||
{
|
||||
checkRunId: check.checkRunId,
|
||||
workflowRunId: check.workflowRunId,
|
||||
checkName: check.name,
|
||||
url: check.url,
|
||||
prRepo: null
|
||||
},
|
||||
{ repoId: repo.id }
|
||||
)
|
||||
patch({
|
||||
details,
|
||||
loading: false,
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { gitLabJobTraceToCheckRunDetails } from './gitlab-job-trace-check-details'
|
||||
import type { PRCheckDetail } from './types'
|
||||
|
||||
const CHECK: PRCheckDetail = {
|
||||
name: 'test: unit',
|
||||
status: 'completed',
|
||||
conclusion: 'failure',
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/1',
|
||||
gitlabJobId: 1
|
||||
}
|
||||
|
||||
describe('gitLabJobTraceToCheckRunDetails', () => {
|
||||
it('adapts a trace into a single-job PRCheckRunDetails carrying the log tail', () => {
|
||||
const details = gitLabJobTraceToCheckRunDetails(CHECK, 'line 1\nERROR: boom\nline 3')
|
||||
|
||||
expect(details.name).toBe('test: unit')
|
||||
expect(details.status).toBe('completed')
|
||||
expect(details.conclusion).toBe('failure')
|
||||
expect(details.url).toBe('https://gitlab.com/acme/orca/-/jobs/1')
|
||||
expect(details.detailsUrl).toBe('https://gitlab.com/acme/orca/-/jobs/1')
|
||||
expect(details.annotations).toEqual([])
|
||||
expect(details.jobs).toHaveLength(1)
|
||||
expect(details.jobs[0]).toMatchObject({
|
||||
id: 1,
|
||||
name: 'test: unit',
|
||||
logTail: 'line 1\nERROR: boom\nline 3',
|
||||
steps: []
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps only the trailing 200 lines of a large trace', () => {
|
||||
const trace = Array.from({ length: 500 }, (_, index) => `line ${index}`).join('\n')
|
||||
|
||||
const [job] = gitLabJobTraceToCheckRunDetails(CHECK, trace).jobs
|
||||
const tailLines = job.logTail?.split('\n') ?? []
|
||||
|
||||
expect(tailLines).toHaveLength(200)
|
||||
expect(tailLines[0]).toBe('line 300')
|
||||
expect(tailLines.at(-1)).toBe('line 499')
|
||||
})
|
||||
|
||||
it('emits no job when the trace is empty so the panel shows its empty state', () => {
|
||||
expect(gitLabJobTraceToCheckRunDetails(CHECK, ' \n ').jobs).toEqual([])
|
||||
})
|
||||
|
||||
it('falls back to a null job id when the check has no gitlabJobId', () => {
|
||||
const { gitlabJobId: _omit, ...withoutId } = CHECK
|
||||
const [job] = gitLabJobTraceToCheckRunDetails(withoutId, 'boom').jobs
|
||||
|
||||
expect(job.id).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,54 @@
|
||||
import type { PRCheckDetail, PRCheckRunDetails } from './types'
|
||||
|
||||
// Why: GitLab job traces can be megabytes; the Checks panel only needs the tail
|
||||
// to surface the failure, matching the GitHub log-excerpt behavior.
|
||||
const GITLAB_TRACE_TAIL_LINES = 200
|
||||
|
||||
function sliceTraceTail(trace: string): string {
|
||||
const lines = trace.split(/\r?\n/)
|
||||
if (lines.length <= GITLAB_TRACE_TAIL_LINES) {
|
||||
return trace
|
||||
}
|
||||
return lines.slice(-GITLAB_TRACE_TAIL_LINES).join('\n')
|
||||
}
|
||||
|
||||
/**
|
||||
* Adapts a GitLab job trace into the shared `PRCheckRunDetails` shape so the
|
||||
* Checks panel and full-details tab can reuse the GitHub rendering path. GitLab
|
||||
* exposes a single flat trace per job rather than GitHub's step/annotation
|
||||
* breakdown, so the trace lands in one synthetic job's `logTail`.
|
||||
*/
|
||||
export function gitLabJobTraceToCheckRunDetails(
|
||||
check: PRCheckDetail,
|
||||
trace: string
|
||||
): PRCheckRunDetails {
|
||||
const logTail = sliceTraceTail(trace).trim()
|
||||
return {
|
||||
name: check.name,
|
||||
status: check.status,
|
||||
conclusion: check.conclusion,
|
||||
url: check.url,
|
||||
detailsUrl: check.url,
|
||||
startedAt: null,
|
||||
completedAt: null,
|
||||
title: null,
|
||||
summary: null,
|
||||
text: null,
|
||||
annotations: [],
|
||||
jobs: logTail
|
||||
? [
|
||||
{
|
||||
id: check.gitlabJobId ?? null,
|
||||
name: check.name,
|
||||
status: check.status,
|
||||
conclusion: check.conclusion,
|
||||
startedAt: null,
|
||||
completedAt: null,
|
||||
url: check.url,
|
||||
logTail,
|
||||
steps: []
|
||||
}
|
||||
]
|
||||
: []
|
||||
}
|
||||
}
|
||||
@@ -44,26 +44,51 @@ describe('gitLabPipelineJobsToPRChecks', () => {
|
||||
name: 'test: unit',
|
||||
status: 'completed',
|
||||
conclusion: 'failure',
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/1'
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/1',
|
||||
gitlabJobId: 1
|
||||
},
|
||||
{
|
||||
name: 'deploy: deploy',
|
||||
status: 'completed',
|
||||
conclusion: 'neutral',
|
||||
url: null
|
||||
url: null,
|
||||
gitlabJobId: 2
|
||||
},
|
||||
{
|
||||
name: 'deploy: delayed deploy',
|
||||
status: 'queued',
|
||||
conclusion: 'pending',
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/3'
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/3',
|
||||
gitlabJobId: 3
|
||||
},
|
||||
{
|
||||
name: 'integration: external callback',
|
||||
status: 'queued',
|
||||
conclusion: 'pending',
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/4'
|
||||
url: 'https://gitlab.com/acme/orca/-/jobs/4',
|
||||
gitlabJobId: 4
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('omits gitlabJobId when the job has no id', () => {
|
||||
const [row] = gitLabPipelineJobsToPRChecks([
|
||||
{
|
||||
id: 0,
|
||||
name: 'orphan',
|
||||
stage: '',
|
||||
status: 'success',
|
||||
webUrl: '',
|
||||
duration: null
|
||||
}
|
||||
])
|
||||
|
||||
expect(row).toEqual({
|
||||
name: 'orphan',
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
url: null
|
||||
})
|
||||
expect(row).not.toHaveProperty('gitlabJobId')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -59,6 +59,9 @@ export function gitLabPipelineJobsToPRChecks(jobs: GitLabPipelineJob[]): PRCheck
|
||||
name: job.stage ? `${job.stage}: ${job.name}` : job.name,
|
||||
status: mapGitLabPipelineJobStatusToCheckStatus(job.status),
|
||||
conclusion: mapGitLabPipelineJobStatusToConclusion(job.status),
|
||||
url: job.webUrl || null
|
||||
url: job.webUrl || null,
|
||||
// Why: lets the Checks panel fetch this job's trace via `gitlab:jobTrace`,
|
||||
// since GitLab jobs have no GitHub-style check-run/workflow ids.
|
||||
...(job.id ? { gitlabJobId: job.id } : {})
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -1339,6 +1339,9 @@ export type PRCheckDetail = {
|
||||
url: string | null
|
||||
checkRunId?: number
|
||||
workflowRunId?: number
|
||||
// Why: GitLab pipeline jobs have no check-run/workflow ids; carry the raw job
|
||||
// id so the Checks panel can load its trace via `gitlab:jobTrace`.
|
||||
gitlabJobId?: number
|
||||
}
|
||||
|
||||
export type PRCheckAnnotation = {
|
||||
|
||||
Reference in New Issue
Block a user