fix(hosted-review-sitter): wait out a running GitHub workflow before rerunning it

src/main/fork-hosted-review-sitter/provider-github-actions.ts

@@ -5,6 +5,7 @@ import type {
   HostedReviewSitterDefinition
 } from '../../shared/fork-hosted-review-sitter/types'
 import { areCurrentHeadChecksGreen } from '../../shared/fork-hosted-review-sitter/decision-check-groups'
+import { extractExecError } from '../git/exec-error'
 import { throwIfAborted, type HostedReviewSitterGitExecution } from './provider-git'
 import {
   expectedStateMismatch,
@@ -114,6 +115,12 @@ function parseRerunTargets(checkIds: readonly string[]): {
   return { workflowIds: [...workflowIds], checkIds: [...directCheckIds] }
 }
 
+/** GitHub's refusal to rerun a run that has not finished; it guarantees no job was requeued. */
+function isWorkflowRunInProgressRefusal(error: unknown): boolean {
+  const { stderr } = extractExecError(error)
+  return /already running[\s\S]*HTTP 403/i.test(stderr)
+}
+
 async function rerunChecks(
   state: GitHubState,
   action: Extract<ProviderAction, { kind: 'rerun-check' }>,
@@ -136,22 +143,34 @@ async function rerunChecks(
   if (targets.workflowIds.length === 0 && targets.checkIds.length === 0) {
     throw new Error('GitHub returned no rerunnable failed checks.')
   }
+  let rerunAccepted = false
   for (const workflowId of targets.workflowIds) {
     throwIfAborted(signal)
     await assertLeaseHeld()
-    await runGitHubApi(
-      state.ownerRepo,
-      state.ghOptions,
-      'core',
-      [
-        'api',
-        '-X',
-        'POST',
-        `repos/${state.ownerRepo.owner}/${state.ownerRepo.repo}/actions/runs/${workflowId}/rerun-failed-jobs`
-      ],
-      signal,
-      () => tracker.markDispatched()
-    )
+    try {
+      await runGitHubApi(
+        state.ownerRepo,
+        state.ghOptions,
+        'core',
+        [
+          'api',
+          '-X',
+          'POST',
+          `repos/${state.ownerRepo.owner}/${state.ownerRepo.repo}/actions/runs/${workflowId}/rerun-failed-jobs`
+        ],
+        signal,
+        () => tracker.markDispatched()
+      )
+    } catch (error) {
+      if (!rerunAccepted && isWorkflowRunInProgressRefusal(error)) {
+        throw Object.assign(error instanceof Error ? error : new Error(String(error)), {
+          effect: 'not-landed' as const,
+          reason: 'workflow-run-in-progress'
+        })
+      }
+      throw error
+    }
+    rerunAccepted = true
   }
   for (const checkId of targets.checkIds) {
     throwIfAborted(signal)

src/main/fork-hosted-review-sitter/provider-github-rerun.test.ts

@@ -0,0 +1,147 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { buildRerunAction } from '../../shared/fork-hosted-review-sitter/decision-action-builders'
+import type {
+  HostedReviewCheckSnapshot,
+  HostedReviewSitterDefinition,
+  HostedReviewSnapshot
+} from '../../shared/fork-hosted-review-sitter/types'
+import { createHostedReviewSitterMutationTracker } from './provider-action-effect'
+import { executeGitHubSitterAction } from './provider-github-actions'
+import type { GitHubState } from './provider-github-read'
+import type { HostedReviewSitterGitExecution } from './provider-git'
+
+const mocks = vi.hoisted(() => ({
+  loadGitHubState: vi.fn(),
+  runGitHubApi: vi.fn()
+}))
+
+vi.mock('./provider-github-read', () => ({
+  ENQUEUE_PULL_REQUEST_MUTATION: 'mutation EnqueuePullRequest',
+  loadGitHubState: mocks.loadGitHubState,
+  runGitHubApi: mocks.runGitHubApi,
+  stringValue: (value: unknown) => (typeof value === 'string' ? value : '')
+}))
+
+vi.mock('./provider-git', () => ({ throwIfAborted: vi.fn() }))
+
+const HEAD = 'head-1'
+
+function failedCheck(checkId: string, checkKey: string): HostedReviewCheckSnapshot {
+  return {
+    checkKey,
+    checkId,
+    name: checkKey,
+    required: true,
+    headSha: HEAD,
+    state: 'failed',
+    observationId: `${checkId}:1`,
+    failureSignature: null
+  }
+}
+
+function snapshot(checks: HostedReviewCheckSnapshot[]): HostedReviewSnapshot {
+  return {
+    provider: 'github',
+    reviewNumber: 42,
+    url: 'https://github.com/acme/repo/pull/42',
+    lifecycle: 'open',
+    headSha: HEAD,
+    baseSha: 'base-1',
+    draft: false,
+    checks,
+    checksComplete: true,
+    providerReadiness: { verdict: 'blocked', blockers: ['checks'] },
+    behindBase: false,
+    conflicts: 'none',
+    queue: { required: false, membership: 'not-enqueued' },
+    defaultMergeMethod: 'squash'
+  }
+}
+
+const definition: HostedReviewSitterDefinition = {
+  repoId: 'repo-1',
+  worktreeId: 'worktree-1',
+  repoPath: '/repo',
+  branch: 'feature',
+  provider: 'github',
+  reviewNumber: 42,
+  reviewUrl: 'https://github.com/acme/repo/pull/42',
+  capabilities: { updateBranch: 'on', resolveConflicts: 'on', fixChecks: 'on', merge: 'on' },
+  branchUpdateMode: 'merge-base-update',
+  mergeMethod: null,
+  mergeCheckScope: 'required'
+}
+
+function githubState(review: HostedReviewSnapshot): GitHubState {
+  return {
+    snapshot: review,
+    ownerRepo: { owner: 'acme', repo: 'repo' },
+    ghOptions: {},
+    pullRequestId: 'pr-42',
+    baseRefName: 'main',
+    sourceIdentityComplete: true
+  }
+}
+
+function ghFailure(stderr: string): Error {
+  return Object.assign(new Error('Command failed: gh api'), { stderr })
+}
+
+// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a rerun never touches a Git method.
+const git = {} as unknown as HostedReviewSitterGitExecution
+
+async function rerun(checks: HostedReviewCheckSnapshot[]) {
+  const review = snapshot(checks)
+  mocks.loadGitHubState.mockResolvedValue(githubState(review))
+  const action = buildRerunAction(review, { checkKey: checks[0]!.checkKey, checks })
+  return executeGitHubSitterAction(
+    definition,
+    action,
+    git,
+    undefined,
+    createHostedReviewSitterMutationTracker(),
+    async () => {}
+  )
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+})
+
+describe('GitHub sitter check reruns', () => {
+  it('reports a rerun refused because the workflow run is still going as landing nothing', async () => {
+    mocks.runGitHubApi.mockRejectedValueOnce(
+      ghFailure('gh: This workflow is already running (HTTP 403)\n')
+    )
+
+    await expect(rerun([failedCheck('workflow:900:check:1', 'test')])).rejects.toMatchObject({
+      effect: 'not-landed',
+      reason: 'workflow-run-in-progress'
+    })
+  })
+
+  it('keeps the refusal indeterminate once an earlier workflow in the same action reran', async () => {
+    mocks.runGitHubApi
+      .mockResolvedValueOnce('')
+      .mockRejectedValueOnce(ghFailure('gh: This workflow is already running (HTTP 403)\n'))
+
+    const error: unknown = await rerun([
+      failedCheck('workflow:900:check:1', 'test'),
+      failedCheck('workflow:901:check:2', 'test')
+    ]).catch((caught: unknown) => caught)
+    expect(error).toBeInstanceOf(Error)
+    expect(error).not.toHaveProperty('effect')
+  })
+
+  it('leaves other forbidden responses indeterminate', async () => {
+    mocks.runGitHubApi.mockRejectedValueOnce(
+      ghFailure('gh: Resource not accessible by integration (HTTP 403)\n')
+    )
+
+    const error: unknown = await rerun([failedCheck('workflow:900:check:1', 'test')]).catch(
+      (caught: unknown) => caught
+    )
+    expect(error).toBeInstanceOf(Error)
+    expect(error).not.toHaveProperty('effect')
+  })
+})

src/shared/fork-hosted-review-sitter/decision-action-builders.ts

@@ -3,7 +3,11 @@ import {
   hostedReviewContentIdentity,
   makeHostedReviewEvidenceKey
 } from './action-identity'
-import { currentHeadChecks, type FailedCheckGroup } from './decision-check-groups'
+import {
+  currentHeadChecks,
+  workflowRunSiblingChecks,
+  type FailedCheckGroup
+} from './decision-check-groups'
 import type {
   HostedReviewAttemptEntry,
   HostedReviewCheckSnapshot,
@@ -29,12 +33,17 @@ export function buildRerunAction(
     .filter((signature): signature is string => signature !== null)
     .sort()
   const failureSignature = signatures[0] ?? null
+  // the gate retries a not-landed rerun only on new evidence; its workflow run moving is that evidence
+  const siblingObservationIds = workflowRunSiblingChecks(review, group)
+    .map((check) => check.observationId)
+    .sort()
   const evidenceKey = makeHostedReviewEvidenceKey([
     'rerun-check',
     review.headSha,
     group.checkKey,
     failureSignature,
-    ...observationIds
+    ...observationIds,
+    ...(siblingObservationIds.length > 0 ? ['workflow-run', ...siblingObservationIds] : [])
   ])
   return {
     kind: 'rerun-check',

src/shared/fork-hosted-review-sitter/decision-check-groups.ts

@@ -90,6 +90,34 @@ export function areCurrentHeadChecksGreen(
   return true
 }
 
+function githubWorkflowRunId(check: HostedReviewCheckSnapshot): string | null {
+  return check.checkId.match(/^workflow:(\d+):check:/)?.[1] ?? null
+}
+
+/**
+ * Current-head checks outside the group that share a GitHub workflow run with it, in any scope.
+ * GitHub refuses to rerun a run's failed jobs until every job in that run has finished.
+ */
+export function workflowRunSiblingChecks(
+  review: HostedReviewSnapshot,
+  group: FailedCheckGroup
+): readonly HostedReviewCheckSnapshot[] {
+  if (review.provider !== 'github') {
+    return []
+  }
+  const runIds = new Set(group.checks.map(githubWorkflowRunId).filter((id) => id !== null))
+  const own = new Set(group.checks.map((check) => check.checkId))
+  return review.checks.filter((check) => {
+    const runId = githubWorkflowRunId(check)
+    return (
+      check.headSha === review.headSha &&
+      runId !== null &&
+      runIds.has(runId) &&
+      !own.has(check.checkId)
+    )
+  })
+}
+
 /** Failed current-head checks in scope, grouped by check key in a stable order. */
 export function failedCheckGroups(
   review: HostedReviewSnapshot,