Surface CI Agent errors in PR Checks UI with per-trigger check runs

.github/workflows/ci-agent.yml

@@ -23,8 +23,10 @@ on:
 permissions:
   pull-requests: write
   contents: read
+  checks: write
 
 jobs:
+  # Magic string: the `ci-agent` job id below must not change as the agent looks for a job with this name to update reviews with result details
   ci-agent:
     # if triggered by an issue_comment, the issue must be a Pull Request comment and the commentor must be semi-trusted (not internet rando)
     if: github.event_name != 'issue_comment' || (github.event.issue.pull_request && contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR", "CONTRIBUTOR"]'), github.event.comment.author_association))

Dockerfile

@@ -30,8 +30,10 @@ RUN bun run setup
 COPY agents/ /ci-agent/agents/
 COPY source/ /ci-agent/source/
 COPY tests/ /ci-agent/tests/
-# copy the dockerfile because we have some test covareg that ensures it stays in-sync with code
+# copy the dockerfile because we have test that ensures it stays in-sync with code
 COPY Dockerfile /ci-agent/Dockerfile
+# copy the workflow file because we have a test that ensures it stays in-sync with code
+COPY .github/workflows/ci-agent.yml /ci-agent/.github/workflows/ci-agent.yml
 
 # Seed a git history so diff tests have commits to work with
 RUN <<EOF

source/github.mts

@@ -1,12 +1,23 @@
 import type { GitHubConfiguration, GitHubReviewPayload } from "./github-types.mts"
 import type { Logger } from "./logger.mts"
+import { guard, isArrayOf, isInteger, isString } from "./typescript-helpers.mts"
 
 const REQUEST_TIMEOUT_MILLISECONDS = 10_000
 const RETRY_DELAY_MILLISECONDS = 30_000
 const DEADLINE_MILLISECONDS = 300_000
 
 export type GitHubFetch = (url: string, options: RequestInit) => Promise<Response>
 
+function buildRepoUrl(configuration: GitHubConfiguration, pathSegments: readonly string[], query?: Readonly<Record<string, string>>): string {
+	const { apiUrl, owner, repositoryName } = configuration
+	let url = `${apiUrl}/repos/${owner}/${repositoryName}/${pathSegments.join("/")}`
+	if (query) {
+		const queryString = Object.entries(query).map(([key, value]) => `${encodeURIComponent(key)}=${encodeURIComponent(value)}`).join("&")
+		url += `?${queryString}`
+	}
+	return url
+}
+
 export function createGithubFetch(logger: Logger): GitHubFetch {
 	return async function githubFetch(url: string, options: RequestInit): Promise<Response> {
 		// Retries rate limiting (429) and request timeouts; everything else fails fast.
@@ -60,9 +71,10 @@ export function createGithubFetch(logger: Logger): GitHubFetch {
 }
 
 export async function fetchPullRequestDiff(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration): Promise<string> {
-	const { apiUrl, token, owner, repositoryName, pullRequestNumber } = configuration
+	const { token } = configuration
+	const url = buildRepoUrl(configuration, ["pulls", String(configuration.pullRequestNumber)])
 
-	const response = await dependencies.githubFetch(`${apiUrl}/repos/${owner}/${repositoryName}/pulls/${pullRequestNumber}`, {
+	const response = await dependencies.githubFetch(url, {
 		method: "GET",
 		headers: { Authorization: `token ${token}`, Accept: "application/vnd.github.diff" },
 	})
@@ -77,9 +89,10 @@ export async function fetchPullRequestDiff(dependencies: { githubFetch: GitHubFe
 
 export type SubmitReviewResult = { ok: true } | { ok: false, status: number, body: string }
 export async function submitReview(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, review: GitHubReviewPayload): Promise<SubmitReviewResult> {
-	const { apiUrl, token, owner, repositoryName, pullRequestNumber } = configuration
+	const { token } = configuration
+	const url = buildRepoUrl(configuration, ["pulls", String(configuration.pullRequestNumber), "reviews"])
 
-	const response = await dependencies.githubFetch(`${apiUrl}/repos/${owner}/${repositoryName}/pulls/${pullRequestNumber}/reviews`, {
+	const response = await dependencies.githubFetch(url, {
 		method: "POST",
 		headers: { Authorization: `token ${token}`, Accept: "application/vnd.github.v3+json", "Content-Type": "application/json" },
 		body: JSON.stringify(review),
@@ -92,29 +105,100 @@ export async function submitReview(dependencies: { githubFetch: GitHubFetch }, c
 	return { ok: true }
 }
 
-function isValidPrMetadata(data: unknown): data is { base: { sha: string } } {
-	if (typeof data !== "object") return false
-	if (data === null) return false
-	if (!("base" in data) || typeof data.base !== "object" || data.base === null) return false
-	if (!("sha" in data.base) || typeof data.base.sha !== "string") return false
-	return true
+const isPrRef = guard({ sha: isString })
+const isValidPrMetadata = guard({
+	base: isPrRef,
+	head: isPrRef,
+})
+
+async function fetchPrShaField(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, field: "base" | "head"): Promise<string> {
+	const { token } = configuration
+	const url = buildRepoUrl(configuration, ["pulls", String(configuration.pullRequestNumber)])
+
+	const response = await dependencies.githubFetch(url, {
+		method: "GET",
+		headers: { Authorization: `token ${token}`, Accept: "application/vnd.github.v3+json" },
+	})
+
+	if (!response.ok) {
+		const body = await response.text().catch(() => "")
+		throw new Error(`Failed to fetch PR ${field} commit: ${response.status} ${response.statusText}${body ? `\n${body}` : ""}`)
+	}
+
+	const data: unknown = await response.json()
+	if (!isValidPrMetadata(data)) throw new Error(`Invalid PR response: missing ${field}.sha`)
+	return data[field].sha
 }
 
 export async function fetchPullRequestBaseCommit(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration): Promise<string> {
-	const { apiUrl, token, owner, repositoryName, pullRequestNumber } = configuration
+	return fetchPrShaField(dependencies, configuration, "base")
+}
+
+export async function fetchPullRequestHeadSha(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration): Promise<string> {
+	return fetchPrShaField(dependencies, configuration, "head")
+}
+
+export interface CheckRunOutput {
+	title: string
+	summary: string
+	text?: string
+}
+
+const isValidCheckRunResponse = guard({ id: isInteger })
+const isCheckRunSummary = guard({ id: isInteger, name: isString, status: isString })
+const isValidCheckRunsList = guard({ check_runs: isArrayOf(isCheckRunSummary) })
 
-	const response = await dependencies.githubFetch(`${apiUrl}/repos/${owner}/${repositoryName}/pulls/${pullRequestNumber}`, {
+export async function findActiveCheckRunByName(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, headSha: string, name: string): Promise<number | null> {
+	const { token } = configuration
+	const url = buildRepoUrl(configuration, ["commits", headSha, "check-runs"], { check_name: name })
+
+	const response = await dependencies.githubFetch(url, {
 		method: "GET",
 		headers: { Authorization: `token ${token}`, Accept: "application/vnd.github.v3+json" },
 	})
 
+	if (!response.ok) return null
+
+	const data: unknown = await response.json()
+	if (!isValidCheckRunsList(data)) return null
+
+	const active = data.check_runs.find(run => run.name === name && run.status === "in_progress")
+	return active?.id ?? null
+}
+
+export async function createCheckRun(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, headSha: string, name: string, output: CheckRunOutput): Promise<number> {
+	const { token } = configuration
+	const url = buildRepoUrl(configuration, ["check-runs"])
+
+	const response = await dependencies.githubFetch(url, {
+		method: "POST",
+		headers: { Authorization: `token ${token}`, Accept: "application/vnd.github.v3+json", "Content-Type": "application/json" },
+		body: JSON.stringify({ name, head_sha: headSha, status: "in_progress", output }),
+	})
+
 	if (!response.ok) {
 		const body = await response.text().catch(() => "")
-		throw new Error(`Failed to fetch PR base commit: ${response.status} ${response.statusText}${body ? `\n${body}` : ""}`)
+		throw new Error(`Failed to create check run: ${response.status} ${response.statusText}${body ? `\n${body}` : ""}`)
 	}
 
 	const data: unknown = await response.json()
-	if (!isValidPrMetadata(data)) throw new Error("Invalid PR response: missing base.sha")
+	if (!isValidCheckRunResponse(data)) throw new Error("Invalid check run response: missing id")
+
+	return data.id
+}
+
+export async function updateCheckRun(dependencies: { githubFetch: GitHubFetch }, configuration: GitHubConfiguration, checkRunId: number, conclusion: "success" | "failure" | "cancelled", output: CheckRunOutput): Promise<void> {
+	const { token } = configuration
+	const url = buildRepoUrl(configuration, ["check-runs", String(checkRunId)])
+
+	const response = await dependencies.githubFetch(url, {
+		method: "PATCH",
+		headers: { Authorization: `token ${token}`, Accept: "application/vnd.github.v3+json", "Content-Type": "application/json" },
+		body: JSON.stringify({ status: "completed", conclusion, output }),
+	})
 
-	return data.base.sha
+	if (!response.ok) {
+		const body = await response.text().catch(() => "")
+		throw new Error(`Failed to update check run: ${response.status} ${response.statusText}${body ? `\n${body}` : ""}`)
+	}
 }

source/orchestrator.mts

@@ -8,8 +8,8 @@ import type { DebugWriter } from "./debug.mts"
 import type { SpawnGit } from "./diff.mts"
 import { ensureCommitAvailable, generateLocalDiff, validateGitEnvironment } from "./diff.mts"
 import type { GitHubConfiguration } from "./github-types.mts"
-import type { GitHubFetch } from "./github.mts"
-import { fetchPullRequestBaseCommit, fetchPullRequestDiff, submitReview } from "./github.mts"
+import type { CheckRunOutput, GitHubFetch } from "./github.mts"
+import { createCheckRun, fetchPullRequestBaseCommit, fetchPullRequestDiff, fetchPullRequestHeadSha, findActiveCheckRunByName, submitReview, updateCheckRun } from "./github.mts"
 import type { Logger } from "./logger.mts"
 import type { ProviderProfile } from "./provider-profiles.mts"
 import type { AiReviewResult } from "./review.mts"
@@ -26,6 +26,48 @@ type OrchestratorDependencies = {
 	debugWriter: DebugWriter
 }
 
+const COMMENT_REVIEW_CHECK_RUN_NAME = "CI Agent Comment Reviewer"
+const PULL_REQUEST_REVIEW_CHECK_RUN_NAME = "CI Agent Pull Request Reviewer"
+
+// Magic string: must match the `id:` of the job in .github/workflows/ci-agent.yml that creates a check run before invoking CI Agent. The integration test in tests/integration.test.mts enforces this.
+export const EXISTING_CHECK_RUN_JOB_NAME = "ci-agent"
+
+type CheckRunIdSource = (dependencies: OrchestratorDependencies, githubConfiguration: GitHubConfiguration, headSha: string) => Promise<number>
+
+function extractErrorMessage(error: unknown): string {
+	if (error instanceof Error) return error.message
+	return String(error)
+}
+
+function formatCheckRunErrorOutput(error: unknown, name: string): CheckRunOutput {
+	return {
+		title: `${name} failed`,
+		summary: "The CI Agent encountered an error while reviewing this pull request. See details below or check the GitHub Actions logs for more context.",
+		text: `## Error\n\n> ${extractErrorMessage(error)}`,
+	}
+}
+
+async function runWithCheckRun(dependencies: OrchestratorDependencies, githubConfiguration: GitHubConfiguration, name: string, getCheckRunId: CheckRunIdSource, operation: () => Promise<void>): Promise<void> {
+	const headSha = await fetchPullRequestHeadSha(dependencies, githubConfiguration)
+	const checkRunId = await getCheckRunId(dependencies, githubConfiguration, headSha)
+
+	try {
+		await operation()
+	} catch (error) {
+		await updateCheckRun(dependencies, githubConfiguration, checkRunId, "failure", formatCheckRunErrorOutput(error, name)).catch(reportingError => {
+			dependencies.logger.log(`Failed to update check run ${checkRunId} with error: ${reportingError}`)
+		})
+		throw error
+	}
+
+	await updateCheckRun(dependencies, githubConfiguration, checkRunId, "success", {
+		title: name,
+		summary: "Review submitted successfully.",
+	}).catch(reportingError => {
+		dependencies.logger.log(`Failed to update check run ${checkRunId} with success status: ${reportingError}`)
+	})
+}
+
 async function runAnalysis(dependencies: OrchestratorDependencies, agentNames: AgentNames, userAgentsDirectory: string, builtinAgentsDirectory: string, diffText: string, baseCommit: string, model: string, profile: ProviderProfile, submit?: (result: AiReviewResult) => Promise<AggregatorSubmitResult>): Promise<AiReviewResult> {
 	const agents = await loadAgents(dependencies, userAgentsDirectory, builtinAgentsDirectory, agentNames)
 	const aggregator = await loadAggregator(dependencies, userAgentsDirectory, builtinAgentsDirectory)
@@ -66,11 +108,32 @@ export async function runOnCommentTrigger(dependencies: OrchestratorDependencies
 		return
 	}
 
-	await submitPrReview(dependencies, triggerResult, userAgentsDirectory, builtinAgentsDirectory, configuration.github, model, profile)
+	await runWithCheckRun(
+		dependencies,
+		configuration.github,
+		COMMENT_REVIEW_CHECK_RUN_NAME,
+		async (deps, config, headSha) => createCheckRun(deps, config, headSha, COMMENT_REVIEW_CHECK_RUN_NAME, {
+			title: COMMENT_REVIEW_CHECK_RUN_NAME,
+			summary: "Review is in progress.",
+		}),
+		() => submitPrReview(dependencies, triggerResult, userAgentsDirectory, builtinAgentsDirectory, configuration.github, model, profile),
+	)
 }
 
 export async function runOnPullRequest(dependencies: OrchestratorDependencies, configuration: PullRequestConfiguration, userAgentsDirectory: string, builtinAgentsDirectory: string, model: string, profile: ProviderProfile): Promise<void> {
-	await submitPrReview(dependencies, configuration.agents, userAgentsDirectory, builtinAgentsDirectory, configuration.github, model, profile)
+	await runWithCheckRun(
+		dependencies,
+		configuration.github,
+		PULL_REQUEST_REVIEW_CHECK_RUN_NAME,
+		async (deps, config, headSha) => {
+			const existingId = await findActiveCheckRunByName(deps, config, headSha, EXISTING_CHECK_RUN_JOB_NAME)
+			if (existingId === null) {
+				throw new Error(`No active check run named "${EXISTING_CHECK_RUN_JOB_NAME}" found for head SHA ${headSha}. The workflow must create a check run with this name before invoking CI Agent.`)
+			}
+			return existingId
+		},
+		() => submitPrReview(dependencies, configuration.agents, userAgentsDirectory, builtinAgentsDirectory, configuration.github, model, profile),
+	)
 }
 
 export async function runOnLocalDiff(dependencies: OrchestratorDependencies, configuration: LocalDiffConfiguration, userAgentsDirectory: string, builtinAgentsDirectory: string, model: string, workspaceDirectory: string, profile: ProviderProfile): Promise<string> {