diff --git a/src/core/cli.ts b/src/core/cli.ts index ea1070e..afa4601 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -263,7 +263,13 @@ function render(value: VerbResult, packageDir: string): string[] { return lines; } case 'attach': - if (value.via === 'gh') return [`${value.alreadyPosted ? 'already posted' : 'posted'} ${value.posted.map((p) => basename(p)).join(', ')} in the description of ${value.prUrl}`]; + if (value.via === 'gh') { + const { comment } = value; + return [ + `${value.alreadyPosted ? 'already posted' : 'posted'} ${value.posted.map((p) => basename(p)).join(', ')} in the description of ${value.prUrl}`, + ...(comment === undefined ? [] : [comment.failure === null ? `commented ${comment.body}` : `not commented gh pr comment failed: ${comment.failure}. Post this comment on the PR yourself: ${comment.body}`]), + ]; + } return [ `handed off ${value.handedOff.map((p) => basename(p)).join(', ')} to the session's runner (${value.sessionRunUrl}), because ${value.because}`, `next {cli} down ends the session; the verify-attach workflow then puts the evidence in the description of PR #${value.pr}`, diff --git a/src/core/publish.ts b/src/core/publish.ts index 8d13ebd..d8d4f1c 100644 --- a/src/core/publish.ts +++ b/src/core/publish.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os'; import { basename, extname, join } from 'node:path'; import { run as defaultRunner, type Runner } from './exec.ts'; import type { Publishable, Showable } from './evidence.ts'; -import { VerifyFailure, everyFile, type AttachResult, type EvidenceBundle, type EvidencePath, type EvidenceSummary, type HostAdapter, type MediaChoice, type Platform } from './types.ts'; +import { VerifyFailure, everyFile, type AttachResult, type EvidenceBundle, type EvidenceComment, type EvidencePath, type EvidenceSummary, type HostAdapter, type MediaChoice, type Platform } from './types.ts'; export function summarize(evidence: Showable): EvidenceSummary { return { @@ -192,14 +192,21 @@ function uploadedIn(body: string, platform: Platform, run: string): readonly str return block === null || !block.includes(`\`${run}\``) ? [] : (block.match(UPLOADED) ?? []); } -async function readDescription(runner: Runner, repo: string, pr: number): Promise<{ readonly body: string; readonly url: string }> { - const viewed = await runner('gh', ['pr', 'view', String(pr), '--repo', repo, '--json', 'body,url,state']); +async function readDescription(runner: Runner, repo: string, pr: number): Promise<{ readonly body: string; readonly url: string; readonly isDraft: boolean }> { + const viewed = await runner('gh', ['pr', 'view', String(pr), '--repo', repo, '--json', 'body,url,state,isDraft']); if (viewed.code !== 0) throw new CannotPostHere('gh pr view failed', `gh pr view failed: ${viewed.stderr.trim()}`, 'check `gh auth status` and that the PR exists'); - const parsed = JSON.parse(viewed.stdout) as { readonly body?: string | null; readonly url?: string; readonly state?: string }; + const parsed = JSON.parse(viewed.stdout) as { readonly body?: string | null; readonly url?: string; readonly state?: string; readonly isDraft?: boolean }; if (parsed.state !== 'OPEN') { throw new VerifyFailure('USAGE', `PR #${pr} of ${repo} is ${parsed.state?.toLowerCase() ?? 'not known to be open'}, and attach writes only to an open pull request`, `pass the number of an open pull request as --pr, or reopen PR #${pr}, then rerun {cli} attach`); } - return { body: parsed.body ?? '', url: parsed.url ?? '' }; + return { body: parsed.body ?? '', url: parsed.url ?? '', isDraft: parsed.isDraft === true }; +} + +// The evidence is already in the description by now, so a comment that can't be posted is reported, and doesn't fail the attach. +async function commentAfterEvidence(host: HostAdapter, pr: number, runner: Runner): Promise { + const body = host.commentAfterEvidence!; + const sent = await runner('gh', ['pr', 'comment', String(pr), '--repo', host.githubRepo, '--body', body]); + return { body, failure: sent.code === 0 ? null : sent.stderr.trim() }; } export async function postToPullRequest(bundle: EvidenceBundle, host: HostAdapter, runner: Runner = defaultRunner): Promise { @@ -252,7 +259,7 @@ export async function postToPullRequest(bundle: EvidenceBundle, host: HostAdapte const notLaidOut = (why: string): VerifyFailure => new VerifyFailure('NOT_READY', `the files of run ${run} are in the description of PR #${pr} as a list, one under the other, and were not laid out: ${why}`, 'rerun {cli} attach once nobody is editing the description'); - const listedNow = (await readDescription(runner, host.githubRepo, pr)).body; + const { body: listedNow, isDraft } = await readDescription(runner, host.githubRepo, pr); const uploaded = uploadedIn(listedNow, platform, run); if (uploaded.length !== files.length) throw notLaidOut(`the block there now does not show run ${run} with the ${files.length} addresses GitHub gave its files`); const laidOut = withEvidenceBlock(listedNow, platform, laidOutWith(uploaded)); @@ -260,7 +267,9 @@ export async function postToPullRequest(bundle: EvidenceBundle, host: HostAdapte const arranged = await edit(laidOut.body, []); if (arranged.code !== 0) throw notLaidOut(`gh pr edit failed: ${arranged.stderr.trim()}`); - return { verb: 'attach', via: 'gh', prUrl, posted: paths, alreadyPosted: false }; + // A draft gets no review yet, and CodeRabbit reads the description when the pull request turns ready. + const comment = host.commentAfterEvidence === undefined || isDraft ? null : await commentAfterEvidence(host, pr, runner); + return { verb: 'attach', via: 'gh', prUrl, posted: paths, alreadyPosted: false, ...(comment === null ? {} : { comment }) }; } finally { rmSync(scratch, { recursive: true, force: true }); } diff --git a/src/core/types.ts b/src/core/types.ts index 5c8f2e9..4c88209 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -207,8 +207,13 @@ export interface HandOffReceipt { readonly sessionRunUrl: string; } +export interface EvidenceComment { + readonly body: string; + readonly failure: string | null; +} + export type AttachResult = - | { readonly verb: 'attach'; readonly via: 'gh'; readonly prUrl: string; readonly posted: readonly EvidencePath[]; readonly alreadyPosted: boolean } + | { readonly verb: 'attach'; readonly via: 'gh'; readonly prUrl: string; readonly posted: readonly EvidencePath[]; readonly alreadyPosted: boolean; readonly comment?: EvidenceComment } | ({ readonly verb: 'attach'; readonly via: 'runner'; readonly pr: number; readonly handedOff: readonly EvidencePath[]; readonly because: string } & HandOffReceipt); export type DownResult = @@ -470,6 +475,8 @@ export interface HostAdapter { readonly cli: string; readonly platforms: readonly Platform[]; readonly githubRepo: string; + /** `attach` posts this on a ready pull request after it changes the description, because a review bot does not reread an edited description by itself. */ + readonly commentAfterEvidence?: string; appId(platform: Platform): string; buildInputs(platform: Platform, backend: BackendKind): readonly string[]; build(platform: Platform, key: BuildKey, into: ScratchPath, progress: (line: string) => void): Promise; diff --git a/test/evidence.test.ts b/test/evidence.test.ts index e7282f6..64f2756 100644 --- a/test/evidence.test.ts +++ b/test/evidence.test.ts @@ -13,7 +13,8 @@ import { Secret } from '../specs/support/secret.ts'; import { attach } from '../src/core/verbs.ts'; import { newRunId } from '../specs/support/inputs.ts'; import { openWorkspace } from '../src/core/workspace.ts'; -import type { BuildKey, EvidencePath, EvidenceRecord, EvidenceSummary, HostAdapter, RunId } from '../src/core/types.ts'; +import { createOutput } from '../src/core/cli.ts'; +import type { BuildKey, EvidencePath, EvidenceRecord, EvidenceSummary, HostAdapter, RunId, VerbResult } from '../src/core/types.ts'; const OWN_USER = 'user_own'; @@ -325,9 +326,10 @@ describe('attach', () => { readonly attached: readonly string[]; } - function fakeGh(options: { readonly ffmpeg?: Runner; readonly attachFlag?: boolean; readonly body?: string; readonly onView?: (views: number, pr: { body: string; state: string }) => void; readonly upload?: (reference: string, address: string) => string } = {}) { + function fakeGh(options: { readonly ffmpeg?: Runner; readonly attachFlag?: boolean; readonly body?: string; readonly onView?: (views: number, pr: { body: string; state: string }) => void; readonly upload?: (reference: string, address: string) => string; readonly isDraft?: boolean; readonly commentFails?: boolean } = {}) { const pr = { body: options.body ?? DESCRIPTION, state: 'OPEN' }; const edits: Edit[] = []; + const comments: (readonly string[])[] = []; let views = 0; let uploads = 0; const runner: Runner = async (command, args, runOptions) => { @@ -337,7 +339,11 @@ describe('attach', () => { if (args[1] === 'view') { views += 1; options.onView?.(views, pr); - return { code: 0, stdout: JSON.stringify({ body: pr.body, url: PR_URL, state: pr.state }), stderr: '' }; + return { code: 0, stdout: JSON.stringify({ body: pr.body, url: PR_URL, state: pr.state, isDraft: options.isDraft === true }), stderr: '' }; + } + if (args[1] === 'comment') { + comments.push(args); + return options.commentFails === true ? { code: 1, stdout: '', stderr: 'HTTP 403: nope\n' } : { code: 0, stdout: `${PR_URL}#issuecomment-1\n`, stderr: '' }; } const bodyFile = readFileSync(args[args.indexOf('--body-file') + 1]!, 'utf8'); const attached = args.filter((_, i) => args[i - 1] === '--attach').map((file) => readFileSync(join(runOptions!.cwd!, file), 'utf8')); @@ -348,13 +354,13 @@ describe('attach', () => { }); return { code: 0, stdout: `${PR_URL}\n`, stderr: '' }; }; - return { runner, pr, edits }; + return { runner, pr, edits, comments }; } - function checkout(gh: { readonly runner: Runner } = fakeGh()) { + function checkout(gh: { readonly runner: Runner } = fakeGh(), hostOptions: Partial = {}) { const packageDir = mkdtempSync(join(tmpdir(), 'verify-attach-')); const workspace = openWorkspace({ packageDir, worktree: packageDir, home: join(packageDir, 'home') }); - const deps = { host, workspace, runner: gh.runner, env: {}, progress: () => undefined, instances: heldInstances() }; + const deps = { host: { ...host, ...hostOptions }, workspace, runner: gh.runner, env: {}, progress: () => undefined, instances: heldInstances() }; const sealed = (shape: { readonly called: string; readonly shots?: readonly string[]; readonly states?: readonly string[]; readonly spec?: string; readonly specs?: readonly string[]; readonly video?: boolean; readonly failed?: boolean; readonly failureShot?: boolean; readonly recordsTests?: boolean; readonly change?: Partial; readonly log?: string }) => { const { run, dir } = workspace.newRun(); mkdirSync(join(dir, 'screenshots'), { recursive: true }); @@ -651,6 +657,60 @@ describe('attach', () => { assert.deepEqual(result, { verb: 'attach', via: 'gh', prUrl: PR_URL, posted: [join(dir, 'screenshots', 'profile.png')], alreadyPosted: false }); }); + describe('the comment after the evidence', () => { + const RECHECK = '@coderabbitai run pre-merge checks'; + const printed = (result: VerbResult): readonly string[] => { + let out = ''; + createOutput(false, '/tmp', 'bin/control-x', { write: (text: string) => (out += text) }, { write: () => true }).result(result); + return out.trimEnd().split('\n'); + }; + + it('posts the host\'s comment on a ready pull request once the tables are written', async () => { + const gh = fakeGh(); + const { deps, sealed } = checkout(gh, { commentAfterEvidence: RECHECK }); + const result = await attach(deps, { verb: 'attach', run: sealed({ called: 'after' }).run, pr: 9, media: still, before: null }); + assert.equal(gh.edits.length, 2); + assert.deepEqual(gh.comments, [['pr', 'comment', '9', '--repo', 'clerk/clerk-ios', '--body', RECHECK]]); + assert.deepEqual(result.verb === 'attach' && result.via === 'gh' ? result.comment : null, { body: RECHECK, failure: null }); + }); + + it('posts nothing on a draft, for a host without a comment, or when the description already holds the evidence', async () => { + const draft = fakeGh({ isDraft: true }); + const onDraft = checkout(draft, { commentAfterEvidence: RECHECK }); + const draftResult = await attach(onDraft.deps, { verb: 'attach', run: onDraft.sealed({ called: 'after' }).run, pr: 9, media: still, before: null }); + assert.deepEqual(draft.comments, []); + assert.equal('comment' in draftResult, false); + + const silent = fakeGh(); + const noComment = checkout(silent); + await attach(noComment.deps, { verb: 'attach', run: noComment.sealed({ called: 'after' }).run, pr: 9, media: still, before: null }); + assert.deepEqual(silent.comments, []); + + const repeated = fakeGh(); + const again = checkout(repeated, { commentAfterEvidence: RECHECK }); + const { run } = again.sealed({ called: 'after' }); + await attach(again.deps, { verb: 'attach', run, pr: 9, media: still, before: null }); + const second = await attach(again.deps, { verb: 'attach', run, pr: 9, media: still, before: null }); + assert.equal(repeated.comments.length, 1); + assert.equal('comment' in second, false); + }); + + it('keeps the attach and reports the failure when the comment cannot be posted', async () => { + const gh = fakeGh({ commentFails: true }); + const { deps, sealed } = checkout(gh, { commentAfterEvidence: RECHECK }); + const result = await attach(deps, { verb: 'attach', run: sealed({ called: 'after' }).run, pr: 9, media: still, before: null }); + assert.equal(gh.edits.length, 2); + assert.deepEqual(result.verb === 'attach' && result.via === 'gh' ? result.comment : null, { body: RECHECK, failure: 'HTTP 403: nope' }); + assert.deepEqual(printed(result).slice(1), [`not commented gh pr comment failed: HTTP 403: nope. Post this comment on the PR yourself: ${RECHECK}`]); + }); + + it('says what it commented', async () => { + const { deps, sealed } = checkout(fakeGh(), { commentAfterEvidence: RECHECK }); + const result = await attach(deps, { verb: 'attach', run: sealed({ called: 'after' }).run, pr: 9, media: still, before: null }); + assert.deepEqual(printed(result).slice(1), [`commented ${RECHECK}`]); + }); + }); + it('leaves the list in the description and says so when the addresses of the files cannot be read back, or the block names another run by then', async () => { const gh = fakeGh({ upload: (reference) => reference }); const { deps, sealed } = checkout(gh);