mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +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>
156 lines
5.3 KiB
TypeScript
156 lines
5.3 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
const { ghExecFileAsyncMock, getOwnerRepoMock, rateLimitGuardMock } = vi.hoisted(() => ({
|
|
ghExecFileAsyncMock: vi.fn(),
|
|
getOwnerRepoMock: vi.fn(),
|
|
rateLimitGuardMock: vi.fn(() => ({ blocked: false }))
|
|
}))
|
|
|
|
vi.mock('./gh-utils', () => ({
|
|
execFileAsync: vi.fn(),
|
|
ghExecFileAsync: ghExecFileAsyncMock,
|
|
githubRepoContext: (repoPath: string, connectionId?: string | null) => ({
|
|
repoPath,
|
|
connectionId: connectionId ?? null
|
|
}),
|
|
ghRepoExecOptions: (context: { repoPath: string }) => ({ cwd: context.repoPath }),
|
|
getOwnerRepo: getOwnerRepoMock,
|
|
getIssueOwnerRepo: vi.fn(),
|
|
getOwnerRepoForRemote: (
|
|
repoPath: string,
|
|
remoteName: string,
|
|
connectionId?: string | null,
|
|
localGitOptions?: unknown
|
|
) =>
|
|
remoteName === 'origin'
|
|
? getOwnerRepoMock(repoPath, connectionId, localGitOptions)
|
|
: Promise.resolve(null),
|
|
resolveIssueSource: vi.fn(),
|
|
extractExecError: vi.fn((err: unknown) => ({ stderr: String(err), stdout: '' })),
|
|
acquire: vi.fn(),
|
|
release: vi.fn(),
|
|
_resetOwnerRepoCache: vi.fn(),
|
|
classifyGhError: (stderr: string) => ({ type: 'unknown', message: stderr }),
|
|
classifyListIssuesError: (stderr: string) => ({ type: 'unknown', message: stderr })
|
|
}))
|
|
|
|
vi.mock('../git/runner', () => ({
|
|
gitExecFileAsync: vi.fn()
|
|
}))
|
|
|
|
vi.mock('./rate-limit', () => ({
|
|
rateLimitGuard: rateLimitGuardMock,
|
|
noteRateLimitSpend: vi.fn(),
|
|
getRateLimit: vi.fn(async () => ({ ok: false, error: 'not probed in tests' })),
|
|
repositoryRateLimitGuard: vi.fn(() => ({ blocked: false })),
|
|
noteRepositoryRateLimitSpend: vi.fn(),
|
|
spendsSharedGitHubComQuota: vi.fn(() => true)
|
|
}))
|
|
|
|
import { getWorkItem, _resetMergeQueueCacheForTests, _resetOwnerRepoCache } from './client'
|
|
import { _resetOriginGitHubApiRepositoryCache } from './github-api-repository'
|
|
|
|
// Why: only `pr view` is fixtured; the merge-metadata fan-out is best-effort and must not mask the summary.
|
|
function mockPullRequestDetail(payload: Record<string, unknown>): void {
|
|
ghExecFileAsyncMock.mockImplementation(async (args: string[]) => {
|
|
if (args[0] !== 'pr' || args[1] !== 'view') {
|
|
throw new Error('unexpected gh call')
|
|
}
|
|
return { stdout: JSON.stringify(payload) }
|
|
})
|
|
}
|
|
|
|
describe('work item checksSummary', () => {
|
|
beforeEach(() => {
|
|
ghExecFileAsyncMock.mockReset()
|
|
getOwnerRepoMock.mockReset()
|
|
rateLimitGuardMock.mockReset()
|
|
rateLimitGuardMock.mockReturnValue({ blocked: false })
|
|
_resetOwnerRepoCache()
|
|
_resetOriginGitHubApiRepositoryCache()
|
|
_resetMergeQueueCacheForTests()
|
|
getOwnerRepoMock.mockResolvedValue({ owner: 'acme', repo: 'widgets' })
|
|
})
|
|
|
|
it('counts a skipped run as passing and reads a StatusContext state as its conclusion', async () => {
|
|
mockPullRequestDetail({
|
|
number: 42,
|
|
title: 'Add feature',
|
|
state: 'OPEN',
|
|
url: 'https://github.com/acme/widgets/pull/42',
|
|
updatedAt: '2026-04-01T00:00:00Z',
|
|
statusCheckRollup: [
|
|
{ __typename: 'CheckRun', status: 'COMPLETED', conclusion: 'SKIPPED' },
|
|
// Why: StatusContext carries no status/conclusion — only `state`.
|
|
{ __typename: 'StatusContext', state: 'SUCCESS' },
|
|
{ __typename: 'CheckRun', status: 'COMPLETED', conclusion: 'NEUTRAL' }
|
|
]
|
|
})
|
|
|
|
const item = await getWorkItem('/repo-root', 42, 'pr', null, {}, 'origin')
|
|
|
|
expect(item?.checksSummary).toEqual({
|
|
state: 'success',
|
|
total: 3,
|
|
passed: 2,
|
|
failed: 0,
|
|
pending: 0,
|
|
neutral: 1
|
|
})
|
|
})
|
|
|
|
it('reports a still-running check as pending', async () => {
|
|
mockPullRequestDetail({
|
|
number: 43,
|
|
title: 'Add feature',
|
|
state: 'OPEN',
|
|
url: 'https://github.com/acme/widgets/pull/43',
|
|
updatedAt: '2026-04-01T00:00:00Z',
|
|
statusCheckRollup: [
|
|
{ __typename: 'CheckRun', status: 'COMPLETED', conclusion: 'SUCCESS' },
|
|
{ __typename: 'CheckRun', status: 'IN_PROGRESS', conclusion: null }
|
|
]
|
|
})
|
|
|
|
const item = await getWorkItem('/repo-root', 43, 'pr', null, {}, 'origin')
|
|
|
|
expect(item?.checksSummary).toEqual({
|
|
state: 'pending',
|
|
total: 2,
|
|
passed: 1,
|
|
failed: 0,
|
|
pending: 1,
|
|
neutral: 0
|
|
})
|
|
})
|
|
|
|
it('fails the summary on a failing run and leaves an empty rollup with no state', async () => {
|
|
mockPullRequestDetail({
|
|
number: 44,
|
|
title: 'Add feature',
|
|
state: 'OPEN',
|
|
url: 'https://github.com/acme/widgets/pull/44',
|
|
updatedAt: '2026-04-01T00:00:00Z',
|
|
statusCheckRollup: [
|
|
{ __typename: 'CheckRun', status: 'COMPLETED', conclusion: 'SUCCESS' },
|
|
{ __typename: 'CheckRun', status: 'COMPLETED', conclusion: 'FAILURE' }
|
|
]
|
|
})
|
|
await expect(getWorkItem('/repo-root', 44, 'pr', null, {}, 'origin')).resolves.toMatchObject({
|
|
checksSummary: { state: 'failure', total: 2, passed: 1, failed: 1, pending: 0, neutral: 0 }
|
|
})
|
|
|
|
mockPullRequestDetail({
|
|
number: 45,
|
|
title: 'Add feature',
|
|
state: 'OPEN',
|
|
url: 'https://github.com/acme/widgets/pull/45',
|
|
updatedAt: '2026-04-01T00:00:00Z',
|
|
statusCheckRollup: []
|
|
})
|
|
await expect(getWorkItem('/repo-root', 45, 'pr', null, {}, 'origin')).resolves.toMatchObject({
|
|
checksSummary: { state: 'none', total: 0, passed: 0, failed: 0, pending: 0, neutral: 0 }
|
|
})
|
|
})
|
|
})
|