From 3d3fd7fc71b833285f863720c1e994936737b6f0 Mon Sep 17 00:00:00 2001 From: William Quintal Date: Tue, 7 Jul 2026 13:59:58 -0400 Subject: [PATCH] fix(gitlab): load pipeline job details in the Checks panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../editor/check-run-details-tab.test.ts | 14 +++ .../editor/check-run-details-tab.ts | 5 + .../components/right-sidebar/ChecksPanel.tsx | 17 ++- .../right-sidebar/checks-panel-content.tsx | 7 +- .../gitlab-check-details-loader.test.ts | 104 ++++++++++++++++++ .../gitlab-check-details-loader.ts | 46 ++++++++ src/renderer/src/store/slices/editor.ts | 32 ++++-- .../gitlab-job-trace-check-details.test.ts | 53 +++++++++ src/shared/gitlab-job-trace-check-details.ts | 54 +++++++++ src/shared/gitlab-pipeline-checks.test.ts | 33 +++++- src/shared/gitlab-pipeline-checks.ts | 5 +- src/shared/types.ts | 3 + 12 files changed, 353 insertions(+), 20 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/gitlab-check-details-loader.test.ts create mode 100644 src/renderer/src/components/right-sidebar/gitlab-check-details-loader.ts create mode 100644 src/shared/gitlab-job-trace-check-details.test.ts create mode 100644 src/shared/gitlab-job-trace-check-details.ts diff --git a/src/renderer/src/components/editor/check-run-details-tab.test.ts b/src/renderer/src/components/editor/check-run-details-tab.test.ts index c6beb882802..bb7bc507edc 100644 --- a/src/renderer/src/components/editor/check-run-details-tab.test.ts +++ b/src/renderer/src/components/editor/check-run-details-tab.test.ts @@ -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({ diff --git a/src/renderer/src/components/editor/check-run-details-tab.ts b/src/renderer/src/components/editor/check-run-details-tab.ts index 0acd550f7b2..7b21c3f4e94 100644 --- a/src/renderer/src/components/editor/check-run-details-tab.ts +++ b/src/renderer/src/components/editor/check-run-details-tab.ts @@ -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}` } diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index fb1a8857aca..248b34dcd74 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -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 => { 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(() => { diff --git a/src/renderer/src/components/right-sidebar/checks-panel-content.tsx b/src/renderer/src/components/right-sidebar/checks-panel-content.tsx index 0dd54d7b7dd..9ff7beaf501 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-content.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel-content.tsx @@ -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]: { diff --git a/src/renderer/src/components/right-sidebar/gitlab-check-details-loader.test.ts b/src/renderer/src/components/right-sidebar/gitlab-check-details-loader.test.ts new file mode 100644 index 00000000000..df8a8cd21e9 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/gitlab-check-details-loader.test.ts @@ -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('@/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') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/gitlab-check-details-loader.ts b/src/renderer/src/components/right-sidebar/gitlab-check-details-loader.ts new file mode 100644 index 00000000000..2e6ea9e1691 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/gitlab-check-details-loader.ts @@ -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[0] + check: PRCheckDetail +}): Promise { + const jobId = args.check.gitlabJobId + if (!jobId) { + return null + } + const target = getActiveRuntimeTarget(args.settings) + const result = + target.kind === 'environment' + ? await callRuntimeRpc( + 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) +} diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index fcfffeb641e..25a26f74dd7 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -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 = (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, diff --git a/src/shared/gitlab-job-trace-check-details.test.ts b/src/shared/gitlab-job-trace-check-details.test.ts new file mode 100644 index 00000000000..c1884f314a0 --- /dev/null +++ b/src/shared/gitlab-job-trace-check-details.test.ts @@ -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() + }) +}) diff --git a/src/shared/gitlab-job-trace-check-details.ts b/src/shared/gitlab-job-trace-check-details.ts new file mode 100644 index 00000000000..a3de28c3225 --- /dev/null +++ b/src/shared/gitlab-job-trace-check-details.ts @@ -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: [] + } + ] + : [] + } +} diff --git a/src/shared/gitlab-pipeline-checks.test.ts b/src/shared/gitlab-pipeline-checks.test.ts index 673fcf3cd16..6c5687e5fa5 100644 --- a/src/shared/gitlab-pipeline-checks.test.ts +++ b/src/shared/gitlab-pipeline-checks.test.ts @@ -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') + }) }) diff --git a/src/shared/gitlab-pipeline-checks.ts b/src/shared/gitlab-pipeline-checks.ts index efcea5ee4bf..f3e0b0ee10f 100644 --- a/src/shared/gitlab-pipeline-checks.ts +++ b/src/shared/gitlab-pipeline-checks.ts @@ -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 } : {}) })) } diff --git a/src/shared/types.ts b/src/shared/types.ts index 65d2a1124f8..951c07a815b 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -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 = {