mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
* fix(checks): stop skipped and manual checks reporting as failures Route every check-classification surface through one shared helper so desktop renderer, desktop main and mobile agree on the same verdict. - GitLab `manual` jobs and pipelines are neutral again, not action_required/failure - `skipped` counts as passed everywhere, including mobile - a neutral check no longer demotes a summary that has passing checks * fix(checks): move the check-classification parity test into the renderer project The parity table lived in src/shared but imported a renderer module, and both config/tsconfig.node.json and config/tsconfig.cli.json are composite projects that include src/shared without that renderer path, so `pnpm typecheck` failed with TS6307 on two of its three projects. Only the web project spans both trees. Co-authored-by: Orca <help@stably.ai> * fix(checks): stop the Tasks-grid pill contradicting its own verdict The checks pill's label, tone and icon all read one ProviderCheckSummary, but getChecksLabel short-circuited on the raw `neutral` counter while the tone and icon key off `state`. After the classification fix a PR with 19 success + 1 neutral renders an emerald CheckCircle2 pill that reads "1 unresolved", and mobile's own label (which keys off `state`) reads "19/20 passed" for the same summary. Move the label into src/shared/provider-check-summary.ts so desktop and mobile cannot fork it again, and key it off `state`. Also covers deriveWorkItemCheckSummary, the desktop-main producer of the summary that reaches the Tasks grid and the relay-paired mobile client. It was rewritten here with no test at all; the parity table stands in derivePRCheckStatusFromRollup, which is a different normalizer. The new main-process test drives getWorkItem with a real statusCheckRollup fixture, pinning the StatusContext `state` fallback that would otherwise be deletable with the whole suite still green. Co-authored-by: Orca <help@stably.ai> * fix(gitlab): route the pipeline job-array rollup through the shared check classifier The array path in derivePipelineStatus kept its own copy of the rollup rules, so manual-only read green and one unrecognized job status demoted a passing pipeline to neutral — both disagreeing with every other check surface. Also retry the packaged-CLI smoke temp cleanup on Windows: the copied Orca.exe can still be locked by AV/indexers after every assertion passed, failing the package job. Co-authored-by: Orca <help@stably.ai> * fix(gitlab): stop the skipped pipeline string diverging from the Checks tab - classifyPipelineString now counts a skipped pipeline as passing, matching the per-check classifier; canceled stays neutral and is pinned as an explicit, sign-off-pending divergence. - Pin the production string path (head_pipeline.status) in the parity table and note that the job-array branch has no production caller yet. - Count skipped checks in the Checks panel's passing header so it agrees with the checks pill. - Correct the packaged-CLI smoke retry comment: the EBUSY is the smoke's own just-exited Electron process, not AV/indexers. Co-authored-by: Orca <help@stably.ai> * fix(checks): finish cross-surface check parity and back out the skipped MR-card flip Review follow-ups on the check-classification PR. - PullRequestPage and GitHubItemDialog kept private copies of getCheckCounts / getChecksSummaryLabel that still counted only `success` as passing, so a 2-success/3-skipped PR read "2 passing · 3 skipped" there and "5 passing" in the sidebar. Both copies move to pr-check-counts.ts, which routes the passing bucket through classifyCheckOutcome; action_required keeps its own amber bucket. The summary icon now keys off passing count, so an all-neutral PR stops painting a green tick above "0 of N checks passing". - The sidebar checks header and triage strip still called `{status: completed, conclusion: null}` pending, contradicting the grey "Unresolved checks" pill. Both now read summarizeProviderChecks and render an unresolved chip/strip instead of an amber spinner that can never resolve. - classifyPipelineString('skipped') is reverted to neutral. That flip painted MR cards green for pipelines that never ran, on the only GitLab path with production callers, and contradicted the same function's deferral of `canceled`. Both tone changes stay deferred, pinned by one test. - classifyPipelineString('manual') resolves to pending rather than neutral: a blocked pipeline is outstanding, and neutral let the worktree card fall through to its emerald `open` default while GitLab still refuses the merge. - TaskPage's checks pill helpers move to task-page-checks-pill.ts so the "1 unresolved on a green pill" fix is actually pinned by a test. - smoke-packaged-cli no longer lets an EBUSY cleanup replace the real failure. * fix(checks): stop completed unknown checks from spinning --------- Co-authored-by: Orca <help@stably.ai>
293 lines
11 KiB
TypeScript
293 lines
11 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
import {
|
|
derivePipelineStatus,
|
|
mapGitLabIssueInfo,
|
|
mapMRInfo,
|
|
mapMRState,
|
|
mapPipelineJobStatusToCheckStatus,
|
|
mapPipelineJobStatusToConclusion
|
|
} from './mappers'
|
|
|
|
describe('mapPipelineJobStatusToCheckStatus', () => {
|
|
it('classifies queued lifecycle states', () => {
|
|
expect(mapPipelineJobStatusToCheckStatus('created')).toBe('queued')
|
|
expect(mapPipelineJobStatusToCheckStatus('pending')).toBe('queued')
|
|
expect(mapPipelineJobStatusToCheckStatus('waiting_for_resource')).toBe('queued')
|
|
expect(mapPipelineJobStatusToCheckStatus('preparing')).toBe('queued')
|
|
})
|
|
|
|
it('classifies running as in_progress', () => {
|
|
expect(mapPipelineJobStatusToCheckStatus('running')).toBe('in_progress')
|
|
})
|
|
|
|
it('classifies success/failed/canceled/skipped/manual as completed', () => {
|
|
expect(mapPipelineJobStatusToCheckStatus('success')).toBe('completed')
|
|
expect(mapPipelineJobStatusToCheckStatus('failed')).toBe('completed')
|
|
expect(mapPipelineJobStatusToCheckStatus('canceled')).toBe('completed')
|
|
expect(mapPipelineJobStatusToCheckStatus('skipped')).toBe('completed')
|
|
expect(mapPipelineJobStatusToCheckStatus('manual')).toBe('completed')
|
|
})
|
|
})
|
|
|
|
describe('mapPipelineJobStatusToConclusion', () => {
|
|
it('maps terminal outcomes', () => {
|
|
expect(mapPipelineJobStatusToConclusion('success')).toBe('success')
|
|
expect(mapPipelineJobStatusToConclusion('failed')).toBe('failure')
|
|
expect(mapPipelineJobStatusToConclusion('canceled')).toBe('cancelled')
|
|
expect(mapPipelineJobStatusToConclusion('canceling')).toBe('cancelled')
|
|
expect(mapPipelineJobStatusToConclusion('skipped')).toBe('skipped')
|
|
})
|
|
|
|
it('keeps manual gates neutral while action-required stays actionable', () => {
|
|
expect(mapPipelineJobStatusToConclusion('manual')).toBe('neutral')
|
|
expect(mapPipelineJobStatusToConclusion('action_required')).toBe('action_required')
|
|
})
|
|
|
|
it('maps active lifecycle states to pending', () => {
|
|
expect(mapPipelineJobStatusToConclusion('running')).toBe('pending')
|
|
expect(mapPipelineJobStatusToConclusion('pending')).toBe('pending')
|
|
expect(mapPipelineJobStatusToConclusion('scheduled')).toBe('pending')
|
|
})
|
|
|
|
it('returns null for unknown', () => {
|
|
expect(mapPipelineJobStatusToConclusion('weird-status')).toBeNull()
|
|
})
|
|
})
|
|
|
|
describe('mapMRState', () => {
|
|
it('maps merged/closed/locked directly', () => {
|
|
expect(mapMRState('merged')).toBe('merged')
|
|
expect(mapMRState('closed')).toBe('closed')
|
|
expect(mapMRState('locked')).toBe('locked')
|
|
})
|
|
|
|
it('returns draft when the draft flag is set', () => {
|
|
expect(mapMRState('opened', true)).toBe('draft')
|
|
})
|
|
|
|
it("infers draft from a 'Draft:' title prefix", () => {
|
|
expect(mapMRState('opened', false, 'Draft: refactor auth')).toBe('draft')
|
|
expect(mapMRState('opened', undefined, 'WIP: in progress')).toBe('draft')
|
|
})
|
|
|
|
it("returns 'opened' for plain open MRs", () => {
|
|
expect(mapMRState('opened', false, 'Add gitlab support')).toBe('opened')
|
|
expect(mapMRState('opened')).toBe('opened')
|
|
})
|
|
})
|
|
|
|
describe('mapGitLabIssueInfo', () => {
|
|
it('uses iid as the number when present', () => {
|
|
expect(
|
|
mapGitLabIssueInfo({
|
|
iid: 42,
|
|
title: 'A',
|
|
state: 'opened',
|
|
web_url: 'https://gitlab.com/g/p/-/issues/42',
|
|
labels: [{ name: 'bug' }, { name: 'p1' }]
|
|
})
|
|
).toEqual({
|
|
number: 42,
|
|
title: 'A',
|
|
state: 'opened',
|
|
url: 'https://gitlab.com/g/p/-/issues/42',
|
|
labels: ['bug', 'p1']
|
|
})
|
|
})
|
|
|
|
it('falls back to number when iid is absent', () => {
|
|
expect(mapGitLabIssueInfo({ number: 7, title: 'B', state: 'closed' })).toEqual({
|
|
number: 7,
|
|
title: 'B',
|
|
state: 'closed',
|
|
url: '',
|
|
labels: []
|
|
})
|
|
})
|
|
|
|
it('handles string-only labels', () => {
|
|
expect(mapGitLabIssueInfo({ iid: 1, title: 'C', state: 'opened', labels: ['bug'] })).toEqual({
|
|
number: 1,
|
|
title: 'C',
|
|
state: 'opened',
|
|
url: '',
|
|
labels: ['bug']
|
|
})
|
|
})
|
|
|
|
it('passes description / author / authorAvatarUrl through when present', () => {
|
|
const info = mapGitLabIssueInfo({
|
|
iid: 9,
|
|
title: 'bug',
|
|
state: 'opened',
|
|
description: 'Steps to reproduce.',
|
|
author: { username: 'bob', avatar_url: 'https://example.com/b.png' }
|
|
})
|
|
expect(info.description).toBe('Steps to reproduce.')
|
|
expect(info.author).toBe('bob')
|
|
expect(info.authorAvatarUrl).toBe('https://example.com/b.png')
|
|
})
|
|
})
|
|
|
|
describe('mapMRInfo', () => {
|
|
it('builds an MRInfo from a typical glab payload', () => {
|
|
expect(
|
|
mapMRInfo(
|
|
{
|
|
iid: 10,
|
|
title: 'Add gitlab support',
|
|
state: 'opened',
|
|
draft: false,
|
|
web_url: 'https://gitlab.com/g/p/-/merge_requests/10',
|
|
updated_at: '2026-05-05T10:00:00Z',
|
|
sha: 'deadbeef',
|
|
has_conflicts: false,
|
|
detailed_merge_status: 'mergeable'
|
|
},
|
|
'success'
|
|
)
|
|
).toEqual({
|
|
number: 10,
|
|
title: 'Add gitlab support',
|
|
state: 'opened',
|
|
url: 'https://gitlab.com/g/p/-/merge_requests/10',
|
|
pipelineStatus: 'success',
|
|
updatedAt: '2026-05-05T10:00:00Z',
|
|
mergeable: 'MERGEABLE',
|
|
headSha: 'deadbeef'
|
|
})
|
|
})
|
|
|
|
it('marks CONFLICTING when has_conflicts is true', () => {
|
|
const info = mapMRInfo(
|
|
{
|
|
iid: 1,
|
|
title: 't',
|
|
state: 'opened',
|
|
has_conflicts: true,
|
|
detailed_merge_status: 'mergeable'
|
|
},
|
|
'pending'
|
|
)
|
|
expect(info.mergeable).toBe('CONFLICTING')
|
|
})
|
|
|
|
it('marks UNKNOWN when detailed_merge_status is non-mergeable but not a conflict', () => {
|
|
const info = mapMRInfo(
|
|
{ iid: 1, title: 't', state: 'opened', detailed_merge_status: 'checking' },
|
|
'pending'
|
|
)
|
|
expect(info.mergeable).toBe('UNKNOWN')
|
|
})
|
|
|
|
it('returns draft state when draft flag is set', () => {
|
|
const info = mapMRInfo({ iid: 1, title: 't', state: 'opened', draft: true }, 'neutral')
|
|
expect(info.state).toBe('draft')
|
|
})
|
|
|
|
it('passes description / author / authorAvatarUrl through when present', () => {
|
|
const info = mapMRInfo(
|
|
{
|
|
iid: 5,
|
|
title: 't',
|
|
state: 'opened',
|
|
description: '## Body\n\nDetails here.',
|
|
author: { username: 'alice', avatar_url: 'https://example.com/a.png' }
|
|
},
|
|
'success'
|
|
)
|
|
expect(info.description).toBe('## Body\n\nDetails here.')
|
|
expect(info.author).toBe('alice')
|
|
expect(info.authorAvatarUrl).toBe('https://example.com/a.png')
|
|
})
|
|
|
|
it('omits description / author when absent (distinguishes from list payloads)', () => {
|
|
// Why: detail vs list endpoints differ — a `description` of '' on the
|
|
// type would be ambiguous with "list payload that stripped the body".
|
|
// Prefer absent over default '' so callers can tell them apart.
|
|
const info = mapMRInfo({ iid: 5, title: 't', state: 'opened' }, 'success')
|
|
expect('description' in info).toBe(false)
|
|
expect('author' in info).toBe(false)
|
|
expect('authorAvatarUrl' in info).toBe(false)
|
|
})
|
|
})
|
|
|
|
// Why: mapMRToWorkItem / mapIssueToWorkItem tests live in
|
|
// mappers-workitem.test.ts so this file stays under the oxlint
|
|
// max-lines budget. Same import surface, same describe-per-export
|
|
// shape — split is mechanical, not behavioral.
|
|
|
|
describe('derivePipelineStatus', () => {
|
|
it('returns neutral for null/undefined/empty', () => {
|
|
expect(derivePipelineStatus(null)).toBe('neutral')
|
|
expect(derivePipelineStatus(undefined)).toBe('neutral')
|
|
expect(derivePipelineStatus([])).toBe('neutral')
|
|
})
|
|
|
|
it('classifies a top-level pipeline string', () => {
|
|
expect(derivePipelineStatus('success')).toBe('success')
|
|
expect(derivePipelineStatus('failed')).toBe('failure')
|
|
expect(derivePipelineStatus('running')).toBe('pending')
|
|
// Why: pipeline-level `manual` means GitLab blocked the pipeline on a human trigger — it is
|
|
// outstanding, not broken (red) and not resolved (which would paint the MR card green while
|
|
// "Pipelines must succeed" still refuses the merge).
|
|
expect(derivePipelineStatus('manual')).toBe('pending')
|
|
expect(derivePipelineStatus({ status: 'manual' })).toBe('pending')
|
|
})
|
|
|
|
// Why: pins the two remaining string/array divergences. The job rollup calls skipped jobs
|
|
// passing and canceled jobs failing, but `head_pipeline.status` is the only production entry
|
|
// point and GitLab paints both pipeline states grey — flipping either card tone needs product
|
|
// sign-off, so these assertions exist so neither flip can land silently.
|
|
it('keeps skipped and canceled pipeline strings neutral (deferred tone changes)', () => {
|
|
expect(derivePipelineStatus('skipped')).toBe('neutral')
|
|
expect(derivePipelineStatus({ status: 'skipped' })).toBe('neutral')
|
|
expect(derivePipelineStatus('canceled')).toBe('neutral')
|
|
expect(derivePipelineStatus('canceling')).toBe('neutral')
|
|
expect(derivePipelineStatus([{ status: 'canceled' }])).toBe('failure')
|
|
})
|
|
|
|
it('rolls up an array of jobs', () => {
|
|
expect(derivePipelineStatus([{ status: 'success' }, { status: 'success' }])).toBe('success')
|
|
expect(derivePipelineStatus([{ status: 'success' }, { status: 'failed' }])).toBe('failure')
|
|
expect(derivePipelineStatus([{ status: 'success' }, { status: 'running' }])).toBe('pending')
|
|
})
|
|
|
|
it('failure beats pending in the rollup', () => {
|
|
expect(derivePipelineStatus([{ status: 'failed' }, { status: 'running' }])).toBe('failure')
|
|
expect(derivePipelineStatus([{ status: 'action_required' }, { status: 'success' }])).toBe(
|
|
'failure'
|
|
)
|
|
})
|
|
|
|
it('keeps a manual deploy gate from failing an otherwise green pipeline', () => {
|
|
expect(derivePipelineStatus([{ status: 'manual' }, { status: 'success' }])).toBe('success')
|
|
})
|
|
|
|
it('leaves a manual-only pipeline unresolved rather than green', () => {
|
|
expect(derivePipelineStatus([{ status: 'manual' }])).toBe('neutral')
|
|
expect(derivePipelineStatus([{ status: 'manual' }, { status: 'manual' }])).toBe('neutral')
|
|
})
|
|
|
|
it('counts skipped jobs as passing', () => {
|
|
expect(derivePipelineStatus([{ status: 'skipped' }, { status: 'skipped' }])).toBe('success')
|
|
})
|
|
|
|
it('handles a single object with status', () => {
|
|
expect(derivePipelineStatus({ status: 'success' })).toBe('success')
|
|
})
|
|
|
|
it('keeps malformed and unknown array jobs neutral', () => {
|
|
expect(derivePipelineStatus([{ status: 'future_status' }])).toBe('neutral')
|
|
expect(derivePipelineStatus([{}])).toBe('neutral')
|
|
})
|
|
|
|
// Why: matches the shared rollup rule — one unresolved job must not demote a pipeline that has
|
|
// a passing job, or the same MR reads green in the Checks tab and grey on the card.
|
|
it('lets a passing job outweigh an unknown one', () => {
|
|
expect(derivePipelineStatus([{ status: 'success' }, { status: 'future_status' }])).toBe(
|
|
'success'
|
|
)
|
|
})
|
|
})
|