diff --git a/trios/agent-server/apps/server/src/api/services/queen-ci-verdict.ts b/trios/agent-server/apps/server/src/api/services/queen-ci-verdict.ts new file mode 100644 index 0000000000..f5268a16c3 --- /dev/null +++ b/trios/agent-server/apps/server/src/api/services/queen-ci-verdict.ts @@ -0,0 +1,303 @@ +/** + * A REQUIRED CHECK THAT REFUSES THE PULL REQUEST TAKES THE ACCEPTANCE BACK. + * + * The review accepts a bee's work in the container; the work lands only when + * its pull request passes the repository's required checks. Nothing connected + * the two. Measured 2026-09-22 on gHashTag/t27: the review accepted #4385 + * ("t27c 0.2.0", machineUnmet=0), publish opened #4578 for it, and the + * required `parse-ratchet` refused it - + * + * this spec parsed at the base and does not now -- Parse error in fn + * 'is_coq' near line 17: unexpected token after expression statement + * + * - so the pull request sat red for ever, the issue stayed open with an + * `accept` on it, and the Queen skipped it every round as "the work already + * landed". 42 issues were in that state while the swarm ran 2 bees of 20. No + * bee ever saw the error, because nothing handed it to one. + * + * So each round asks GitHub about a few accepted issues: is there an open + * pull request for the branch, and did a REQUIRED check on its head come back + * red? If so the acceptance becomes a send-back whose note is the check's own + * error line, which the next bee reads in its brief (`judged_note`). The + * required checks decide because they are what stops the work landing - an + * acceptance that cannot land is not an acceptance. + * + * What it will not do: + * - decide on a check still running, or on a check that is not required + * (dozens of advisory checks are red on most pull requests); + * - take anything back when the required list cannot be read - a gate it + * cannot see is not a gate it may enforce; + * - touch a closed or merged pull request; + * - loop for ever: the second refusal escalates to a person, the same + * ceiling the review uses (QueenReviewDecision.maximumSendBacks = 2). + */ +import type { Pool } from 'pg' +import { logger } from '../../lib/logger' + +/** Accepted issues asked about per round. Two GitHub reads each, plus a log per red check. */ +export const CI_CHECKS_PER_ROUND = 8 + +/** The review's own ceiling: the send-back that reaches it escalates instead. */ +export const CI_MAXIMUM_SEND_BACKS = 2 + +/** What the next bee's brief shows of a note (`PREVIOUS_REVIEW_MAX_CHARS`). */ +const NOTE_MAX = 1500 +const ERROR_MAX = 600 + +const RED = new Set([ + 'failure', + 'timed_out', + 'cancelled', + 'action_required', + 'startup_failure', +]) + +export interface CheckRun { + id: number + name: string + status: string + conclusion: string | null + url: string | null +} + +export interface PullRequest { + number: number + state: string + merged: boolean + headSha: string +} + +/** + * The required checks that came back red on this head. A re-run leaves two runs + * of one name; the newest decides. A check still running decides nothing. + */ +export function refusedRequired( + runs: CheckRun[], + required: readonly string[], +): CheckRun[] { + const wanted = new Set(required) + const newest = new Map() + for (const run of runs) { + if (!wanted.has(run.name)) continue + const seen = newest.get(run.name) + if (!seen || run.id > seen.id) newest.set(run.name, run) + } + return [...newest.values()].filter( + (run) => + run.status === 'completed' && + run.conclusion !== null && + RED.has(run.conclusion), + ) +} + +/** + * What the check said, from its Actions log: the `##[error]` lines, without + * the runner's own "Process completed with exit code" epilogue, which names + * no defect. + */ +export function errorLinesOf(log: string): string { + return log + .split('\n') + .map((line) => line.replace(/^\s*\d{4}-\d\d-\d\dT[\d:.]+Z\s?/, '').trim()) + .filter((line) => line.startsWith('##[error]')) + .map((line) => line.slice('##[error]'.length).trim()) + .filter((line) => line && !/^Process completed with exit code/.test(line)) + .join(' | ') + .slice(0, ERROR_MAX) +} + +/** The note the next bee reads. The check's words first; ours are only the frame. */ +export function ciRefusalNote( + pull: number, + branch: string, + refused: Array<{ name: string; error: string; url: string | null }>, +): string { + const lines = refused.map( + (r) => + `- ${r.name}: ${r.error || 'failed; its log held no error line'}` + + (r.url ? ` (${r.url})` : ''), + ) + return [ + `A required check refused pull request #${pull} for ${branch}:`, + ...lines, + 'The review had accepted this work, but the required checks are what let ' + + 'it land, so they decide. Fix it on the same branch; the pull request ' + + 'runs its checks again on the next push.', + ] + .join('\n') + .slice(0, NOTE_MAX) +} + +export interface CiDeps { + /** Names of the required checks on the base branch, or null when unreadable. */ + requiredChecks(): Promise + /** The pull requests whose head is this branch, or null when unreadable. */ + pullsForBranch(branch: string): Promise + checkRuns(sha: string): Promise + jobLog(jobId: number): Promise +} + +export interface TakenBack { + issue: number + pull: number + state: 'sendBack' | 'escalate' + checks: string[] +} + +export async function takeBackRefusedAcceptances( + pool: Pool, + deps: CiDeps, + limit = CI_CHECKS_PER_ROUND, +): Promise { + const required = await deps.requiredChecks() + if (!required || required.length === 0) return [] + + // Oldest-asked first, so eight a round walks the whole accepted set rather + // than asking about the same eight for ever. + const rows = await pool.query( + `SELECT issue, branch, send_backs + FROM queen_dispatch + WHERE review_state = 'accept' AND finished_at IS NOT NULL + ORDER BY ci_checked_at ASC NULLS FIRST, issue + LIMIT $1`, + [limit], + ) + + const taken: TakenBack[] = [] + for (const row of rows.rows) { + const issue = Number(row.issue) + const branch = String(row.branch || `queen-${issue}`) + await pool.query( + `UPDATE queen_dispatch SET ci_checked_at = now() WHERE issue = $1`, + [issue], + ) + + const pulls = await deps.pullsForBranch(branch) + const open = pulls?.find((p) => p.state === 'open' && !p.merged) + if (!open) continue + const runs = await deps.checkRuns(open.headSha) + if (!runs) continue + const red = refusedRequired(runs, required) + if (red.length === 0) continue + + const refused: Array<{ name: string; error: string; url: string | null }> = + [] + for (const run of red) { + const log = await deps.jobLog(run.id) + refused.push({ + name: run.name, + error: log ? errorLinesOf(log) : '', + url: run.url, + }) + } + const note = ciRefusalNote(open.number, branch, refused) + const sendBacks = Number(row.send_backs ?? 0) + 1 + const state: 'sendBack' | 'escalate' = + sendBacks >= CI_MAXIMUM_SEND_BACKS ? 'escalate' : 'sendBack' + + // Guarded on the state it read, so a review that moved the row in the + // meantime is never overwritten. + const updated = await pool.query( + `UPDATE queen_dispatch + SET review_state = $2, review_note = $3, judged_note = $3, + reviewed_at = now(), send_backs = $4::integer + WHERE issue = $1 AND review_state = 'accept'`, + [issue, state, note, sendBacks], + ) + if (!updated.rowCount) continue + const checks = red.map((r) => r.name) + taken.push({ issue, pull: open.number, state, checks }) + logger.info('Queen took back an acceptance a required check refused', { + issue, + pull: open.number, + state, + checks, + }) + } + return taken +} + +/** + * GitHub, read with the round's token. Every failure is null - "could not + * ask" - never an empty list, which would read as "asked, and found nothing". + */ +export function githubCiDeps( + repo: string, + baseBranch: string, + headers: Record, +): CiDeps { + const api = `https://api.github.com/repos/${repo}` + const owner = repo.split('/')[0] + const json = async (url: string): Promise => { + try { + const response = await fetch(url, { headers }) + return response.ok ? await response.json() : null + } catch { + return null + } + } + return { + async requiredChecks() { + const rules = (await json( + `${api}/rules/branches/${encodeURIComponent(baseBranch)}`, + )) as Array<{ + type?: string + parameters?: { required_status_checks?: Array<{ context?: string }> } + }> | null + if (!Array.isArray(rules)) return null + return rules + .filter((rule) => rule.type === 'required_status_checks') + .flatMap((rule) => rule.parameters?.required_status_checks ?? []) + .map((check) => check.context) + .filter((name): name is string => typeof name === 'string') + }, + async pullsForBranch(branch) { + const pulls = (await json( + `${api}/pulls?state=all&per_page=5&head=${encodeURIComponent(`${owner}:${branch}`)}`, + )) as Array<{ + number: number + state: string + merged_at: string | null + head: { sha: string } + }> | null + if (!Array.isArray(pulls)) return null + return pulls.map((p) => ({ + number: p.number, + state: p.state, + merged: p.merged_at !== null, + headSha: p.head.sha, + })) + }, + async checkRuns(sha) { + const body = (await json( + `${api}/commits/${sha}/check-runs?per_page=100`, + )) as { + check_runs?: Array<{ + id: number + name: string + status: string + conclusion: string | null + html_url: string | null + }> + } | null + if (!body?.check_runs) return null + return body.check_runs.map((run) => ({ + id: run.id, + name: run.name, + status: run.status, + conclusion: run.conclusion, + url: run.html_url, + })) + }, + async jobLog(jobId) { + try { + const response = await fetch(`${api}/actions/jobs/${jobId}/logs`, { + headers, + }) + return response.ok ? await response.text() : null + } catch { + return null + } + }, + } +} diff --git a/trios/agent-server/apps/server/src/api/services/queen-tick.ts b/trios/agent-server/apps/server/src/api/services/queen-tick.ts index 4cc99de5da..8a7ab44a02 100644 --- a/trios/agent-server/apps/server/src/api/services/queen-tick.ts +++ b/trios/agent-server/apps/server/src/api/services/queen-tick.ts @@ -42,6 +42,7 @@ import { createQueenPool } from '../../lib/db/queen-pool' import { logger } from '../../lib/logger' import { startModelProbes, workerModelRanking } from '../../lib/model-ranking' import { outstandingEscalations } from '../routes/queen-needs-you' +import { githubCiDeps, takeBackRefusedAcceptances } from './queen-ci-verdict' import { type CriterionRun, criteriaCounts, @@ -51,6 +52,7 @@ import { parseCriterionChecks, } from './queen-criteria-run' import { + baseRef, DISPATCH_OUTCOME_LABELS, dispatchBee, reapDispatchesFromPreviousBoot, @@ -464,6 +466,10 @@ async function ensureQueenColumns(pool: Pool): Promise { ADD COLUMN IF NOT EXISTS judged_head text, ADD COLUMN IF NOT EXISTS judged_conversation text, ADD COLUMN IF NOT EXISTS judged_note text, + -- When the round last asked GitHub whether an accepted issue's pull + -- request passed its required checks (queen-ci-verdict.ts), so a few a + -- round walk the whole accepted set instead of the same few for ever. + ADD COLUMN IF NOT EXISTS ci_checked_at timestamptz, -- The issue's own criterion commands, run by the Queen on the commit, -- keyed like the reviewer cache (branch head, merge base, criteria). A -- wait row is re-read every 60 s, and a measurement is a temporary @@ -1553,6 +1559,20 @@ export async function runRound( logger.info('Queen reviewed her own work', { verdicts: reviewed.acted }) } + // An acceptance whose pull request a REQUIRED check refused is not one: + // the work cannot land, and the issue would otherwise be skipped as "the + // work already landed" for ever (queen-ci-verdict.ts). Housekeeping: a + // failure here is logged and the round goes on to start bees. + await takeBackRefusedAcceptances( + pool, + githubCiDeps(repo, baseRef().replace(/^origin\//, ''), githubReadHeaders()), + ).catch((error) => { + logger.warn('Queen could not ask CI about accepted work', { + error: error instanceof Error ? error.message : String(error), + }) + return [] + }) + const reaped = await reapStalledDispatches(pool) if (reaped.length > 0) { logger.info('Queen tick reaped stalled dispatches', { issues: reaped }) diff --git a/trios/agent-server/apps/server/tests/api/queen-ci-verdict.test.ts b/trios/agent-server/apps/server/tests/api/queen-ci-verdict.test.ts new file mode 100644 index 0000000000..4168342b3e --- /dev/null +++ b/trios/agent-server/apps/server/tests/api/queen-ci-verdict.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, it } from 'bun:test' +import type { Pool } from 'pg' +import { + type CheckRun, + type CiDeps, + ciRefusalNote, + errorLinesOf, + type PullRequest, + refusedRequired, + takeBackRefusedAcceptances, +} from '../../src/api/services/queen-ci-verdict' + +/** + * An acceptance whose pull request a required check refused is taken back. + * + * 2026-09-22, gHashTag/t27: #4385 was accepted, publish opened #4578, and the + * required `parse-ratchet` refused it. The pull request sat red, the issue + * was skipped as "the work already landed", and no bee ever saw the error. + */ + +const REQUIRED = ['validate', 'check-linked-issue', 'parse-ratchet'] + +// The shape of the real log of #4578's parse-ratchet job. +const PARSE_RATCHET_LOG = [ + '2026-09-22T06:18:01.1023264Z ##[group]Run python3 tools/ci/check_specs_still_parse.py', + '2026-09-22T06:18:04.0000000Z changed specs: 1; newly unparseable: 1; repaired: 0', + "2026-09-22T06:18:04.1000000Z ##[error]this spec parsed at the base and does not now -- Error: Parse error: parse error in fn 'is_coq' near line 17: unexpected token after expression statement: Ident", + '2026-09-22T06:18:04.2000000Z A spec that does not parse generates nothing, so every test it carries', + '2026-09-22T06:18:04.3000000Z ##[error]Process completed with exit code 1.', +].join('\n') + +const run = ( + id: number, + name: string, + conclusion: string | null, + status = 'completed', +): CheckRun => ({ id, name, status, conclusion, url: `https://ci/${id}` }) + +describe('which required checks refused', () => { + it('counts only required checks that completed red', () => { + const red = refusedRequired( + [ + run(1, 'parse-ratchet', 'failure'), + run(2, 'validate', 'success'), + run(3, 'coverage', 'failure'), // advisory, red on most pull requests + run(4, 'check-linked-issue', null, 'in_progress'), + ], + REQUIRED, + ) + expect(red.map((r) => r.name)).toEqual(['parse-ratchet']) + }) + + it('lets the newest run of a check decide, so a green re-run clears a red one', () => { + expect( + refusedRequired( + [ + run(10, 'parse-ratchet', 'failure'), + run(11, 'parse-ratchet', 'success'), + ], + REQUIRED, + ), + ).toEqual([]) + expect( + refusedRequired( + [ + run(11, 'parse-ratchet', 'success'), + run(12, 'parse-ratchet', 'failure'), + ], + REQUIRED, + ).map((r) => r.id), + ).toEqual([12]) + }) +}) + +describe('what the check said', () => { + it('keeps the error line and drops the runner epilogue', () => { + const error = errorLinesOf(PARSE_RATCHET_LOG) + expect(error).toContain("parse error in fn 'is_coq' near line 17") + expect(error).not.toContain('Process completed with exit code') + expect(error).not.toContain('2026-09-22T') + }) + + it('puts the error first in the note the next bee reads', () => { + const note = ciRefusalNote(4578, 'queen-4385', [ + { + name: 'parse-ratchet', + error: errorLinesOf(PARSE_RATCHET_LOG), + url: null, + }, + ]) + expect(note.split('\n')[0]).toBe( + 'A required check refused pull request #4578 for queen-4385:', + ) + expect(note).toContain('- parse-ratchet: this spec parsed at the base') + expect(note.length).toBeLessThanOrEqual(1500) + }) +}) + +/** A pool that answers the SELECT with `rows` and records every statement. */ +function fakePool( + rows: Array<{ issue: number; branch: string; send_backs: number }>, +) { + const statements: Array<{ sql: string; params: unknown[] }> = [] + const pool = { + async query(sql: string, params: unknown[] = []) { + statements.push({ sql, params }) + if (/^\s*SELECT/.test(sql)) return { rows, rowCount: rows.length } + return { rows: [], rowCount: 1 } + }, + } + return { pool: pool as unknown as Pool, statements } +} + +function deps(over: Partial = {}): CiDeps { + const pull: PullRequest = { + number: 4578, + state: 'open', + merged: false, + headSha: 'abc', + } + return { + requiredChecks: async () => REQUIRED, + pullsForBranch: async () => [pull], + checkRuns: async () => [ + run(7, 'parse-ratchet', 'failure'), + run(8, 'validate', 'success'), + ], + jobLog: async () => PARSE_RATCHET_LOG, + ...over, + } +} + +const verdictWrites = (statements: Array<{ sql: string; params: unknown[] }>) => + statements.filter((s) => /SET review_state/.test(s.sql)) + +describe('taking an acceptance back', () => { + it('turns a refused acceptance into a send-back carrying the check error', async () => { + const { pool, statements } = fakePool([ + { issue: 4385, branch: 'queen-4385', send_backs: 0 }, + ]) + const taken = await takeBackRefusedAcceptances(pool, deps()) + expect(taken).toEqual([ + { issue: 4385, pull: 4578, state: 'sendBack', checks: ['parse-ratchet'] }, + ]) + const [write] = verdictWrites(statements) + expect(write.params[0]).toBe(4385) + expect(write.params[1]).toBe('sendBack') + expect(String(write.params[2])).toContain("fn 'is_coq' near line 17") + expect(write.params[3]).toBe(1) + // Guarded on the state it read: a row the review moved is not overwritten. + expect(write.sql).toContain("review_state = 'accept'") + }) + + it('escalates the refusal that reaches the send-back ceiling', async () => { + const { pool } = fakePool([ + { issue: 4385, branch: 'queen-4385', send_backs: 1 }, + ]) + const [taken] = await takeBackRefusedAcceptances(pool, deps()) + expect(taken.state).toBe('escalate') + }) + + it('takes nothing back when the required checks cannot be read', async () => { + const { pool, statements } = fakePool([ + { issue: 4385, branch: 'queen-4385', send_backs: 0 }, + ]) + const taken = await takeBackRefusedAcceptances( + pool, + deps({ requiredChecks: async () => null }), + ) + expect(taken).toEqual([]) + expect(statements).toEqual([]) + }) + + for (const [what, over] of [ + [ + 'a green pull request', + { checkRuns: async () => [run(7, 'parse-ratchet', 'success')] }, + ], + [ + 'a pending check', + { checkRuns: async () => [run(7, 'parse-ratchet', null, 'queued')] }, + ], + [ + 'a merged pull request', + { + pullsForBranch: async () => [ + { number: 1, state: 'closed', merged: true, headSha: 'x' }, + ], + }, + ], + ['no pull request', { pullsForBranch: async () => [] }], + ['an unreadable pull request list', { pullsForBranch: async () => null }], + ['unreadable check runs', { checkRuns: async () => null }], + ] as Array<[string, Partial]>) { + it(`leaves the acceptance alone for ${what}, and remembers it asked`, async () => { + const { pool, statements } = fakePool([ + { issue: 4385, branch: 'queen-4385', send_backs: 0 }, + ]) + expect(await takeBackRefusedAcceptances(pool, deps(over))).toEqual([]) + expect(verdictWrites(statements)).toEqual([]) + expect( + statements.some((s) => /ci_checked_at = now\(\)/.test(s.sql)), + ).toBe(true) + }) + } + + it('still takes it back when the log cannot be read, and says so', async () => { + const { pool, statements } = fakePool([ + { issue: 4385, branch: 'queen-4385', send_backs: 0 }, + ]) + await takeBackRefusedAcceptances(pool, deps({ jobLog: async () => null })) + const [write] = verdictWrites(statements) + expect(String(write.params[2])).toContain('its log held no error line') + }) +})