diff --git a/Makefile b/Makefile index fe32e0ff2..9ddd7263f 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,6 @@ -include .env export ANTHROPIC_API_KEY OPENAI_API_KEY GEMINI_API_KEY GROQ_API_KEY \ OPENROUTER_API_KEY XAI_API_KEY EXA_API_KEY \ - MAESTRO_MODEL MAESTRO_MODEL_PROVIDER \ MAESTRO_CEREBRO_URL CEREBRO_URL CEREBRO_SERVICE_URL \ MAESTRO_CEREBRO_TOKEN CEREBRO_TOKEN \ MAESTRO_EVALOPS_ACCESS_TOKEN EVALOPS_TOKEN \ @@ -27,6 +26,15 @@ export ANTHROPIC_API_KEY OPENAI_API_KEY GEMINI_API_KEY GROQ_API_KEY \ MAESTRO_AGENT_MCP_SCOPES MAESTRO_EVALOPS_AGENT_MCP_SCOPES \ MAESTRO_EVALOPS_MEMORY_MODE EVALOPS_MEMORY_MODE +# GNU Make exports an undefined variable as an empty string. An absent model +# override must stay absent so configuration files can select the model. +ifneq ($(origin MAESTRO_MODEL),undefined) +export MAESTRO_MODEL +endif +ifneq ($(origin MAESTRO_MODEL_PROVIDER),undefined) +export MAESTRO_MODEL_PROVIDER +endif + LOCAL_CEREBRO_REPO ?= ../cerebro .PHONY: help setup build build-all run-rs run-rs-debug \ diff --git a/scripts/resolve-public-mirror-bot-review-threads.mjs b/scripts/resolve-public-mirror-bot-review-threads.mjs new file mode 100644 index 000000000..816df6079 --- /dev/null +++ b/scripts/resolve-public-mirror-bot-review-threads.mjs @@ -0,0 +1,318 @@ +#!/usr/bin/env node + +import { execFileSync } from "node:child_process"; +import process from "node:process"; + +const DEFAULT_REPO = "evalops/maestro"; + +// GitHub Apps usually have login suffix "[bot]". Some review apps post +// without that suffix; keep this list aligned with pr-feedback-audit.mjs. +const KNOWN_REVIEW_BOT_LOGINS = + /^(?:cursor|coderabbitai|chatgpt-codex-connector|devin-ai-integration)\b/iu; + +const LIST_REVIEW_THREADS_QUERY = `query($owner:String!,$repo:String!,$number:Int!,$after:String){ + repository(owner:$owner,name:$repo){ + pullRequest(number:$number){ + reviewThreads(first:100,after:$after){ + nodes{ + id + isResolved + isOutdated + path + line + comments(first:20){nodes{url body author{__typename login}}} + } + pageInfo{ + hasNextPage + endCursor + } + } + } + } +}`; + +const RESOLVE_REVIEW_THREAD_MUTATION = `mutation($threadId:ID!){ + resolveReviewThread(input:{threadId:$threadId}){ + thread{id isResolved} + } +}`; + +function parseArgs(argv) { + const args = { + pr: 0, + repo: DEFAULT_REPO, + triageUrl: "", + }; + + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + switch (arg) { + case "--pr": + args.pr = Number(argv[++index] ?? ""); + break; + case "--repo": + args.repo = argv[++index] ?? ""; + break; + case "--triage-url": + args.triageUrl = argv[++index] ?? ""; + break; + default: + throw new Error(`Unknown argument: ${arg}`); + } + } + + if (!Number.isInteger(args.pr) || args.pr <= 0) { + throw new Error("--pr must be a positive integer"); + } + if (!args.repo || !args.repo.includes("/")) { + throw new Error(`Expected --repo owner/name, got ${args.repo}`); + } + + return args; +} + +function ghJson(args) { + const output = execFileSync("gh", args, { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + return JSON.parse(output); +} + +function graphqlData(payload, label) { + if (payload?.errors?.length) { + const messages = payload.errors + .map((error) => String(error?.message ?? error)) + .join("; "); + throw new Error(`${label} GraphQL errors: ${messages}`); + } + if (!payload?.data) { + throw new Error(`${label} returned no GraphQL data`); + } + return payload.data; +} + +export function firstCommentAuthor(thread) { + return thread?.comments?.nodes?.[0]?.author ?? null; +} + +export function isReviewBotAuthor(author) { + if (!author || typeof author !== "object") { + return false; + } + const typename = String(author.__typename ?? "").trim(); + if (typename === "Bot") { + return true; + } + const login = String(author.login ?? "").trim(); + if (!login) { + return false; + } + if (login.toLowerCase().endsWith("[bot]")) { + return true; + } + return KNOWN_REVIEW_BOT_LOGINS.test(login); +} + +export function shouldResolveReviewThread(thread) { + if (!thread || thread.isResolved) { + return false; + } + if (!thread.id) { + return false; + } + return isReviewBotAuthor(firstCommentAuthor(thread)); +} + +export function formatSummaryComment({ + resolvedCount, + humanCount, + authors, + triageUrl, +}) { + const uniqueAuthors = [...new Set(authors.filter(Boolean))]; + const authorNote = uniqueAuthors.length ? ` (${uniqueAuthors.join(", ")})` : ""; + const lines = [ + `Resolved ${resolvedCount} review-bot thread(s) on this generated public mirror PR${authorNote}.`, + `Left ${humanCount} human-started thread(s) unresolved.`, + "", + "Review of mirrored code belongs on the mono source PR.", + ]; + if (triageUrl) { + lines.push(`See ${triageUrl} for the triage pattern.`); + } + return `${lines.join("\n")}\n`; +} + +function flagValue(args, name) { + const prefix = `${name}=`; + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + if ((arg === "-f" || arg === "-F") && (args[index + 1] ?? "").startsWith(prefix)) { + return args[index + 1].slice(prefix.length); + } + if (arg.startsWith(`-f${prefix}`) || arg.startsWith(`-F${prefix}`)) { + return arg.slice(arg.indexOf("=") + 1); + } + } + return ""; +} + +export function isResolveReviewThreadRequest(args) { + const query = flagValue(args, "query"); + return /\bresolveReviewThread\b/.test(query); +} + +export function fetchReviewThreads(owner, repo, number, queryGh) { + const threads = []; + let cursor = ""; + + do { + const apiArgs = [ + "api", + "graphql", + "-f", + `query=${LIST_REVIEW_THREADS_QUERY}`, + "-f", + `owner=${owner}`, + "-f", + `repo=${repo}`, + "-F", + `number=${number}`, + ]; + if (cursor) { + apiArgs.push("-f", `after=${cursor}`); + } + + const data = graphqlData(queryGh(apiArgs), "list review threads"); + const pullRequest = data.repository?.pullRequest; + if (!pullRequest) { + throw new Error(`No pull request ${owner}/${repo}#${number}`); + } + const reviewThreads = pullRequest.reviewThreads; + if (!reviewThreads) { + throw new Error(`Missing reviewThreads on ${owner}/${repo}#${number}`); + } + threads.push(...(reviewThreads.nodes ?? [])); + cursor = reviewThreads.pageInfo?.hasNextPage + ? reviewThreads.pageInfo.endCursor + : ""; + } while (cursor); + + return threads; +} + +function resolveReviewThread(queryGh, threadId) { + const payload = queryGh([ + "api", + "graphql", + "-f", + `query=${RESOLVE_REVIEW_THREAD_MUTATION}`, + "-f", + `threadId=${threadId}`, + ]); + const data = graphqlData(payload, `resolveReviewThread ${threadId}`); + if (!data.resolveReviewThread?.thread?.isResolved) { + throw new Error(`resolveReviewThread did not resolve ${threadId}`); + } +} + +export function resolvePublicMirrorBotReviewThreads({ + owner, + repo, + number, + triageUrl = "", + ghJson: queryGh, + postComment, +}) { + const threads = fetchReviewThreads(owner, repo, number, queryGh); + const toResolve = []; + const authors = []; + let humanCount = 0; + + for (const thread of threads) { + if (thread?.isResolved) { + continue; + } + if (shouldResolveReviewThread(thread)) { + toResolve.push(thread); + const login = String(firstCommentAuthor(thread)?.login ?? "").trim(); + if (login && !authors.includes(login)) { + authors.push(login); + } + continue; + } + humanCount += 1; + } + + const resolvedIds = []; + for (const thread of toResolve) { + resolveReviewThread(queryGh, thread.id); + resolvedIds.push(thread.id); + } + + const commentBody = formatSummaryComment({ + authors, + humanCount, + resolvedCount: resolvedIds.length, + triageUrl, + }); + postComment(commentBody); + + return { + commentBody, + humanCount, + resolvedIds, + }; +} + +function defaultPostComment(repo, number, body) { + execFileSync( + "gh", + [ + "api", + "--method", + "POST", + `repos/${repo}/issues/${number}/comments`, + "--input", + "-", + ], + { + encoding: "utf8", + input: JSON.stringify({ body }), + stdio: ["pipe", "pipe", "pipe"], + }, + ); +} + +function main() { + const args = parseArgs(process.argv.slice(2)); + const [owner, repoName] = args.repo.split("/"); + const result = resolvePublicMirrorBotReviewThreads({ + ghJson, + number: args.pr, + owner, + postComment: (body) => defaultPostComment(args.repo, args.pr, body), + repo: repoName, + triageUrl: args.triageUrl, + }); + console.log( + `Resolved ${result.resolvedIds.length} review-bot thread(s) on ${args.repo}#${args.pr}; left ${result.humanCount} human-started thread(s) unresolved.`, + ); +} + +export { + KNOWN_REVIEW_BOT_LOGINS, + LIST_REVIEW_THREADS_QUERY, + RESOLVE_REVIEW_THREAD_MUTATION, + flagValue, +}; + +if (import.meta.url === `file://${process.argv[1]}`) { + try { + main(); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exit(1); + } +} diff --git a/scripts/resolve-public-mirror-bot-review-threads.test.mjs b/scripts/resolve-public-mirror-bot-review-threads.test.mjs new file mode 100644 index 000000000..434455e9a --- /dev/null +++ b/scripts/resolve-public-mirror-bot-review-threads.test.mjs @@ -0,0 +1,279 @@ +import assert from "node:assert/strict"; +import { existsSync, readFileSync } from "node:fs"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { + flagValue, + formatSummaryComment, + isResolveReviewThreadRequest, + isReviewBotAuthor, + resolvePublicMirrorBotReviewThreads, + shouldResolveReviewThread, +} from "./resolve-public-mirror-bot-review-threads.mjs"; + +const ROOT_SYNC_WORKFLOW = fileURLToPath( + new URL( + "../../../.github/workflows/maestro-sync-public-release-mirror.yml", + import.meta.url, + ), +); + +function thread({ + id, + login, + typename = "User", + isResolved = false, + isOutdated = false, +}) { + return { + id, + isOutdated, + isResolved, + comments: { + nodes: [ + { + author: login == null ? null : { __typename: typename, login }, + body: `comment on ${id}`, + url: `https://example.test/${id}`, + }, + ], + }, + }; +} + +function page(nodes, { hasNextPage = false, endCursor = "" } = {}) { + return { + nodes, + pageInfo: { endCursor, hasNextPage }, + }; +} + +function makeGhJson(pages) { + const resolvedIds = []; + let listCalls = 0; + const comments = []; + + const ghJson = (args) => { + if (isResolveReviewThreadRequest(args)) { + const threadId = flagValue(args, "threadId"); + resolvedIds.push(threadId); + return { + data: { + resolveReviewThread: { + thread: { id: threadId, isResolved: true }, + }, + }, + }; + } + + const current = pages[listCalls] ?? page([]); + listCalls += 1; + return { + data: { + repository: { + pullRequest: { + reviewThreads: current, + }, + }, + }, + }; + }; + + return { + comments, + ghJson, + listCalls: () => listCalls, + postComment: (body) => { + comments.push(body); + }, + resolvedIds, + }; +} + +test("review-bot authors include [bot] suffix, known apps, and GraphQL Bot actors", () => { + assert.equal(isReviewBotAuthor({ login: "chatgpt-codex-connector" }), true); + assert.equal(isReviewBotAuthor({ login: "devin-ai-integration" }), true); + assert.equal(isReviewBotAuthor({ login: "cursor[bot]" }), true); + assert.equal(isReviewBotAuthor({ login: "coderabbitai" }), true); + assert.equal(isReviewBotAuthor({ login: "github-actions[bot]" }), true); + assert.equal( + isReviewBotAuthor({ __typename: "Bot", login: "some-review-app" }), + true, + ); + assert.equal(isReviewBotAuthor({ login: "alice" }), false); + assert.equal(isReviewBotAuthor({ __typename: "User", login: "alice" }), false); + assert.equal(isReviewBotAuthor(null), false); + assert.equal(isReviewBotAuthor({ login: "" }), false); +}); + +test("resolved threads are never selected even when the author is a bot", () => { + assert.equal( + shouldResolveReviewThread( + thread({ id: "PRRT_resolved", login: "chatgpt-codex-connector", isResolved: true }), + ), + false, + ); + assert.equal( + shouldResolveReviewThread( + thread({ id: "PRRT_human", login: "alice" }), + ), + false, + ); + assert.equal( + shouldResolveReviewThread( + thread({ + id: "PRRT_outdated_bot", + login: "github-actions[bot]", + isOutdated: true, + }), + ), + true, + ); +}); + +test("resolveReviewThread is called only for unresolved bot-started threads", () => { + const fixtures = [ + thread({ id: "PRRT_bot", login: "chatgpt-codex-connector" }), + thread({ id: "PRRT_human", login: "alice" }), + thread({ + id: "PRRT_resolved_bot", + login: "devin-ai-integration", + isResolved: true, + }), + thread({ + id: "PRRT_outdated_bot", + login: "github-actions[bot]", + isOutdated: true, + }), + thread({ + id: "PRRT_outdated_human", + login: "bob", + isOutdated: true, + }), + thread({ id: "PRRT_devin", login: "devin-ai-integration" }), + thread({ + id: "PRRT_typename_bot", + login: "some-review-app", + typename: "Bot", + }), + thread({ id: "PRRT_missing_author", login: null }), + { + id: "PRRT_human_then_bot", + isOutdated: false, + isResolved: false, + comments: { + nodes: [ + { + author: { __typename: "User", login: "alice" }, + body: "human started this", + url: "https://example.test/PRRT_human_then_bot", + }, + { + author: { login: "chatgpt-codex-connector" }, + body: "bot replied", + url: "https://example.test/PRRT_human_then_bot-reply", + }, + ], + }, + }, + ]; + const fake = makeGhJson([page(fixtures)]); + const result = resolvePublicMirrorBotReviewThreads({ + ghJson: fake.ghJson, + number: 1122, + owner: "evalops", + postComment: fake.postComment, + repo: "maestro", + triageUrl: "https://example.test/triage/9100", + }); + + assert.deepEqual(result.resolvedIds, [ + "PRRT_bot", + "PRRT_outdated_bot", + "PRRT_devin", + "PRRT_typename_bot", + ]); + assert.deepEqual(fake.resolvedIds, result.resolvedIds); + assert.equal(result.humanCount, 4); + assert.equal(fake.comments.length, 1); + assert.match(fake.comments[0], /Resolved 4 review-bot thread\(s\)/); + assert.match(fake.comments[0], /Left 4 human-started thread\(s\) unresolved/); + assert.match(fake.comments[0], /Review of mirrored code belongs on the mono source PR/); + assert.match(fake.comments[0], /https:\/\/example\.test\/triage\/9100/); + assert.doesNotMatch(fake.comments[0], /PRRT_human/); +}); + +test("paginated GraphQL thread lists still resolve only bot-started threads", () => { + const fake = makeGhJson([ + page([thread({ id: "PRRT_human_page1", login: "alice" })], { + endCursor: "cursor-1", + hasNextPage: true, + }), + page([thread({ id: "PRRT_bot_page2", login: "cursor[bot]" })]), + ]); + const result = resolvePublicMirrorBotReviewThreads({ + ghJson: fake.ghJson, + number: 1122, + owner: "evalops", + postComment: fake.postComment, + repo: "maestro", + triageUrl: "https://example.test/triage/9100", + }); + assert.deepEqual(result.resolvedIds, ["PRRT_bot_page2"]); + assert.equal(result.humanCount, 1); + assert.equal(fake.listCalls(), 2); +}); + +test("summary comment still posts when no bot threads need resolving", () => { + const fake = makeGhJson([ + page([thread({ id: "PRRT_human", login: "alice" })]), + ]); + const result = resolvePublicMirrorBotReviewThreads({ + ghJson: fake.ghJson, + number: 1122, + owner: "evalops", + postComment: fake.postComment, + repo: "maestro", + triageUrl: "https://example.test/triage/9100", + }); + assert.deepEqual(result.resolvedIds, []); + assert.equal(fake.comments.length, 1); + assert.equal( + fake.comments[0], + formatSummaryComment({ + authors: [], + humanCount: 1, + resolvedCount: 0, + triageUrl: "https://example.test/triage/9100", + }), + ); +}); + +test("sync workflow resolves bot threads after opening the PR and before auto-merge", (t) => { + if (!existsSync(ROOT_SYNC_WORKFLOW)) { + t.skip("mono-root sync workflow is not in this checkout"); + return; + } + const workflow = readFileSync(ROOT_SYNC_WORKFLOW, "utf8"); + const openPr = workflow.indexOf("- name: Open or update public sync PR"); + const resolve = workflow.indexOf( + "- name: Resolve review-bot threads on public sync PR", + ); + const autoMerge = workflow.indexOf("- name: Enable auto-merge on public sync PR"); + assert.notEqual(openPr, -1); + assert.notEqual(resolve, -1); + assert.notEqual(autoMerge, -1); + assert.ok(openPr < resolve); + assert.ok(resolve < autoMerge); + const resolveBlock = workflow.slice(resolve, autoMerge); + assert.match( + resolveBlock, + /resolve-public-mirror-bot-review-threads\.mjs/, + ); + assert.match(resolveBlock, /steps\.token\.outputs\.token/); + assert.match(resolveBlock, /--triage-url /); + assert.match(resolveBlock, /\/issues\/9100\b/); + assert.doesNotMatch(resolveBlock, /continue-on-error/); + assert.match(workflow, /permission-pull-requests:\s*write/); +}); diff --git a/scripts/run-ci-tooling.sh b/scripts/run-ci-tooling.sh index 07e28e0aa..c018e0511 100755 --- a/scripts/run-ci-tooling.sh +++ b/scripts/run-ci-tooling.sh @@ -90,6 +90,7 @@ for test_file in \ scripts/check-integration-required-gate.test.mjs \ scripts/check-review-thread-guard-workflow.test.mjs \ scripts/check-sync-public-release-mirror-workflow.test.mjs \ + scripts/resolve-public-mirror-bot-review-threads.test.mjs \ scripts/tag-release-contract.test.mjs; do # The public projection intentionally restores public-owned workflows and # excludes the internal mirror workflow. Keep the shared tooling lane diff --git a/scripts/update-lifecycle-contract.test.mjs b/scripts/update-lifecycle-contract.test.mjs index 1d5a2c6dd..8a8375ebd 100644 --- a/scripts/update-lifecycle-contract.test.mjs +++ b/scripts/update-lifecycle-contract.test.mjs @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import { test } from "node:test"; import { buildReleaseMetadata } from "./create-release-metadata.mjs"; @@ -47,7 +47,7 @@ test("release metadata carries changelog notes and exact runtime passports", asy ]); }); -test("installer and signed release workflow publish receipt metadata", () => { +test("installer and release workflow preserve receipt metadata", () => { const installer = read("scripts/install.sh"); const release = readReleaseWorkflow(); const updater = read("packages/tui-rs/src/update_cli.rs"); @@ -83,12 +83,26 @@ test("installer and signed release workflow publish receipt metadata", () => { assert.match(updater, /legacyExplicit/); assert.doesNotMatch(updater, /MAESTRO_UPDATE_PREVIEW_FALLBACK/); assert.doesNotMatch(read("docs/protocols/release-channels.json"), /MAESTRO_UPDATE_PREVIEW_FALLBACK/); - assert.match(release, /create-release-metadata\.mjs/); + // The nested Mono template creates the signed receipt. In the generated + // public mirror, the public-owned workflow authenticates that same receipt + // before packaging and publishing it. CI must enforce the actual owner in + // each checkout, rather than accepting a private build template as the + // public publishing entrypoint. + const publicMirror = existsSync(new URL("../.github/workflows/check-release-workflow-contract.mjs", import.meta.url)); + if (publicMirror) { + assert.match(release, /repository_dispatch:/); + assert.match(release, /maestro-signed-release/); + assert.match(release, /MONO_SHA256SUMS\.cosign\.bundle/); + assert.match(release, /node scripts\/verify-staged-release\.mjs release-binaries "\$RELEASE_VERSION"/); + assert.match(release, /files\+=\(release-metadata\.json/); + assert.doesNotMatch(release, /evalops-internal-arc/); + } else { + assert.match(release, /create-release-metadata\.mjs/); + assert.match(release, /files\+=\([^\n]*release-metadata\.json/); + } assert.match(release, /release-metadata\.json/); - assert.match(release, /files\+=\([^\n]*release-metadata\.json/); assert.match(release, /create-release-channel-manifest\.mjs/); assert.match(release, /channel-manifest\.json/); assert.match(channelManifest, /createPrivateKey/); assert.match(channelResolver, /alpha or beta/); - assert.match(release, /release-metadata\.json/); });