fix(gitlab): harden check detail loading

Keep provider transport outside the sidebar component layer, route discussion mutations through the viewed worktree owner, and cap pathological single-line traces before retaining them in renderer state.
This commit is contained in:
Brennan Benson
2026-07-20 15:29:37 -07:00
parent 664b3f4329
commit f570ec6d54
6 changed files with 32 additions and 22 deletions
@@ -63,7 +63,7 @@ import type {
PRComment,
PRRefreshErrorType
} from '../../../../shared/types'
import { loadGitLabCheckRunDetails } from './gitlab-check-details-loader'
import { loadGitLabCheckRunDetails } from '@/lib/gitlab-check-details-loader'
import { getConnectionId } from '@/lib/connection-context'
import {
buildResolvePullRequestConflictsPrompt,
@@ -2627,7 +2627,9 @@ export default function ChecksPanel(): React.JSX.Element {
const result = await resolveGitLabMRDiscussionForChecks({
repoPath: repo.path,
repoId: repo.id,
settings,
// Why: resolve against the viewed worktree's GitLab host, even when
// another local or remote runtime currently has focus.
settings: ownerSettings,
iid: activeGitLabReview.number,
discussionId: threadId,
resolved: resolve
@@ -2686,7 +2688,7 @@ export default function ChecksPanel(): React.JSX.Element {
prNumber,
repo,
resolveReviewThread,
settings
ownerSettings
]
)
@@ -2,7 +2,7 @@ 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'
import type { PRCheckDetail } from '../../../shared/types'
vi.mock('@/runtime/runtime-rpc-client', async () => {
const actual = await vi.importActual<typeof RuntimeRpcClient>('@/runtime/runtime-rpc-client')
@@ -1,19 +1,10 @@
import type {
GitLabJobTraceResult,
PRCheckDetail,
PRCheckRunDetails
} from '../../../../shared/types'
import { gitLabJobTraceToCheckRunDetails } from '../../../../shared/gitlab-job-trace-check-details'
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.
* Loads a GitLab job trace from the worktree's runtime owner and adapts it to
* the provider-neutral check-details shape used by the sidebar and editor.
*/
export async function loadGitLabCheckRunDetails(args: {
repoPath: string
+1 -1
View File
@@ -62,7 +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 { loadGitLabCheckRunDetails } from '@/lib/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'
@@ -40,6 +40,16 @@ describe('gitLabJobTraceToCheckRunDetails', () => {
expect(tailLines.at(-1)).toBe('line 499')
})
it('bounds a huge single-line trace without splitting a surrogate pair', () => {
const trace = `${'x'.repeat(10)}💥${'y'.repeat(16 * 1024 - 5)}tail`
const [job] = gitLabJobTraceToCheckRunDetails(CHECK, trace).jobs
expect(job.logTail?.length).toBeLessThanOrEqual(16 * 1024)
expect(job.logTail?.endsWith('tail')).toBe(true)
expect(job.logTail?.charCodeAt(0)).not.toBeGreaterThanOrEqual(0xdc00)
})
it('emits no job when the trace is empty so the panel shows its empty state', () => {
expect(gitLabJobTraceToCheckRunDetails(CHECK, ' \n ').jobs).toEqual([])
})
+11 -4
View File
@@ -3,19 +3,26 @@ 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
const GITLAB_TRACE_TAIL_CODE_UNITS = 16 * 1024
function sliceTraceTail(trace: string): string {
// Walk back from the end over N newlines instead of splitting the whole
// (possibly multi-MB) trace into an array we'd immediately discard all but the tail of.
let cut = trace.length
for (let remaining = GITLAB_TRACE_TAIL_LINES; remaining > 0; remaining--) {
const newline = trace.lastIndexOf('\n', cut - 1)
if (newline === -1) {
return trace
cut = -1
break
}
cut = newline
}
return trace.slice(cut + 1)
// Why: a single generated line can be megabytes, so the line limit alone
// does not bound the retained renderer state.
let start = Math.max(cut + 1, trace.length - GITLAB_TRACE_TAIL_CODE_UNITS)
const firstCodeUnit = trace.charCodeAt(start)
if (start > 0 && firstCodeUnit >= 0xdc00 && firstCodeUnit <= 0xdfff) {
start += 1
}
return trace.slice(start)
}
/**