From 93bcbb5447c2df68f34f01a623d32b44e807c508 Mon Sep 17 00:00:00 2001 From: StanPonomarev Date: Sun, 19 Jul 2026 10:19:37 +0200 Subject: [PATCH] feat: publish implementations with review findings --- README.md | 2 +- .../execution/orchestration/implementation.ts | 18 +++-- .../handlers/conversations/review.ts | 48 +++++++++++++ .../publication/handlers/implementation.ts | 23 +++++- app/src/core/contracts.ts | 3 +- app/src/tests/application/review.test.ts | 72 ++++++++++++++++++- 6 files changed, 157 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index b06ef17..ab499dc 100644 --- a/README.md +++ b/README.md @@ -176,7 +176,7 @@ Plans published under an earlier marker namespace are intentionally not imported Keep `CI_AGENT_BOT_LOGIN` set to the account that authored existing protocol-v1 plan comments if those plans must remain implementable. Changing bot accounts requires replanning outstanding issues. -Initial implementation requires an accepted plan whose issue digest and base SHA are current. The executor uses a deterministic branch named `agent/issue--p`. The controller publishes only after independently validating the reviewed workspace and remote branch state. `/agent review` performs one fresh review of any open pull request and publishes a Gitea comment review with validated inline anchors. Only pull requests tied to a durable published implementation record can use `/agent fix` or `agent:fix-review`. +Initial implementation requires an accepted plan whose issue digest and base SHA are current. The executor uses a deterministic branch named `agent/issue--p`. The controller publishes only after independently validating the reviewed workspace and remote branch state. If blocking findings remain after the final implementation review iteration, a non-empty implementation is still published as a pull request and the remaining findings are posted as a marked Gitea comment review. A rejected implementation with no diff remains a failed request because no pull request can be created. Execution, safety, and stale-state failures are never published. `/agent review` performs one fresh review of any open pull request and publishes a Gitea comment review with validated inline anchors. Only pull requests tied to a durable published implementation record can use `/agent fix` or `agent:fix-review`; marked reviews published after review exhaustion are available to the next fix attempt. `/agent cancel` sets a durable cancellation flag. The executor checks it while heartbeating and propagates cancellation into Gitea requests, Git subprocesses, and OpenCode prompts. Interrupted running jobs return to the queue on service restart; interrupted publications remain in the outbox and are retried. diff --git a/app/src/application/execution/orchestration/implementation.ts b/app/src/application/execution/orchestration/implementation.ts index 967c416..070a7be 100644 --- a/app/src/application/execution/orchestration/implementation.ts +++ b/app/src/application/execution/orchestration/implementation.ts @@ -135,12 +135,15 @@ export async function runImplementation(input: { input.signal, ), ); - if (review.verdict === "accept") { + if ( + review.verdict === "accept" || + (iteration === maximumIterations && files.length) + ) { const pendingFiles = await changedFiles( input.workspace, options, ); - return acceptedResult({ + return implementationResult({ input, accepted, prepared, @@ -149,6 +152,7 @@ export async function runImplementation(input: { diff, summary: summary.summary, rationale: review.rationale, + ...(review.verdict === "revise" ? { review } : {}), iteration, session, baseBranch: repository.default_branch, @@ -179,7 +183,7 @@ export async function runImplementation(input: { }; } -function acceptedResult(value: { +function implementationResult(value: { input: { issueNumber: number }; accepted: NonNullable>; prepared: { @@ -192,6 +196,7 @@ function acceptedResult(value: { diff: string; summary: string; rationale: string; + review?: ReturnType; iteration: number; session: string; baseBranch: string; @@ -205,7 +210,11 @@ function acceptedResult(value: { result: { version: 1, mode: "implement", - status: value.files.length ? "success" : "no-changes", + status: value.review + ? "review-failed" + : value.files.length + ? "success" + : "no-changes", message: value.files.length ? value.rationale || "Implementation accepted" : `${value.summary}\n\nReviewer: ${value.rationale}`, @@ -222,6 +231,7 @@ function acceptedResult(value: { pendingFiles: value.pendingFiles, summary: value.summary, iterations: value.iteration, + ...(value.review ? { review: value.review } : {}), }, }, }; diff --git a/app/src/application/publication/handlers/conversations/review.ts b/app/src/application/publication/handlers/conversations/review.ts index 2a7c621..ee34122 100644 --- a/app/src/application/publication/handlers/conversations/review.ts +++ b/app/src/application/publication/handlers/conversations/review.ts @@ -10,6 +10,7 @@ import { marker, parseMarker, protocolVersion, + type ReviewDecision, sha256, } from "../../../../core/contracts.js"; import { @@ -93,6 +94,53 @@ export async function publishPullReview( return { terminal: "succeeded", reviewId: published.id }; } +export async function publishRemainingImplementationReview( + context: PublicationContext, + input: { + job: Job; + pullRequestNumber: number; + commitSha: string; + review: ReviewDecision; + }, +): Promise { + const expected = { + v: protocolVersion, + kind: "review" as const, + issue: input.job.issueNumber, + mode: "implement" as const, + request: input.job.id, + pullRequest: input.pullRequestNumber, + }; + const existing = ( + await context.client.listPullReviews(input.pullRequestNumber) + ).find((candidate) => { + if ( + candidate.user.login.toLowerCase() !== + context.botLogin.toLowerCase() + ) + return false; + const found = parseMarker(candidate.body, "review"); + return ( + found?.request === input.job.id && + found.pullRequest === input.pullRequestNumber + ); + }); + if (existing) return existing.id; + const findings = input.review.findings.length + ? input.review.findings.map((finding) => `- ${finding}`).join("\n") + : "The reviewer requested revision without itemized findings."; + const published = await context.client.createPullReview( + input.pullRequestNumber, + { + event: "COMMENT", + body: `${marker(expected)}\n${input.review.rationale}\n\n## Remaining issues\n\n${findings}`, + commit_id: input.commitSha, + comments: [], + }, + ); + return published.id; +} + async function publishStatus( context: PublicationContext, job: Job, diff --git a/app/src/application/publication/handlers/implementation.ts b/app/src/application/publication/handlers/implementation.ts index c57c80a..984e1cc 100644 --- a/app/src/application/publication/handlers/implementation.ts +++ b/app/src/application/publication/handlers/implementation.ts @@ -27,6 +27,7 @@ import { type PublicationOutcome, upsertJobStatus, } from "../status.js"; +import { publishRemainingImplementationReview } from "./conversations/review.js"; export async function publishImplementation( context: PublicationContext, @@ -39,6 +40,13 @@ export async function publishImplementation( throw new Error( "Successful implementation job has no workspace result", ); + const remainingReview = + result.status === "review-failed" ? implementation.review : undefined; + if ( + result.status === "review-failed" && + remainingReview?.verdict !== "revise" + ) + throw new Error("Review-failed implementation has no final review"); const [issue, comments, currentBase] = await Promise.all([ context.client.getIssue(job.issueNumber), context.client.getComments(job.issueNumber), @@ -109,12 +117,23 @@ export async function publishImplementation( issue.title, ); await context.client.addLabelIfPresent(pull.number, generatedLabel); + if (remainingReview) + await publishRemainingImplementationReview(context, { + job, + pullRequestNumber: pull.number, + commitSha, + review: remainingReview, + }); await upsertJobStatus( context.client, context.botLogin, job, - "Agent implementation ready", - `[Pull request #${pull.number}](${pull.html_url}) was created or updated.`, + remainingReview + ? "Agent implementation published with review findings" + : "Agent implementation ready", + remainingReview + ? `[Pull request #${pull.number}](${pull.html_url}) was created or updated with the remaining issues posted as a review.` + : `[Pull request #${pull.number}](${pull.html_url}) was created or updated.`, ); return { terminal: "succeeded", commitSha, pullRequestNumber: pull.number }; } diff --git a/app/src/core/contracts.ts b/app/src/core/contracts.ts index a0a39ed..243f8ea 100644 --- a/app/src/core/contracts.ts +++ b/app/src/core/contracts.ts @@ -69,6 +69,7 @@ export interface ImplementationData { pendingFiles: string[]; summary: string; iterations: number; + review?: ReviewDecision; pullRequestNumber?: number; response?: string; feedback?: FeedbackReference[]; @@ -77,7 +78,7 @@ export interface ImplementationData { export interface Result { version: number; mode: Mode; - status: "success" | "no-changes" | "failed"; + status: "success" | "no-changes" | "review-failed" | "failed"; message: string; plan?: PlanData; implementation?: ImplementationData; diff --git a/app/src/tests/application/review.test.ts b/app/src/tests/application/review.test.ts index 774d74a..882aac2 100644 --- a/app/src/tests/application/review.test.ts +++ b/app/src/tests/application/review.test.ts @@ -18,7 +18,10 @@ import type { GiteaPullRequest, GiteaPullReview, } from "../../adapters/gitea/types.js"; -import { publishPullReview } from "../../application/publication/handlers/conversations/review.js"; +import { + publishPullReview, + publishRemainingImplementationReview, +} from "../../application/publication/handlers/conversations/review.js"; import type { PublicationContext } from "../../application/publication/status.js"; import { marker, protocolVersion, sha256 } from "../../core/contracts.js"; @@ -135,6 +138,73 @@ test("cleans a partial pending review before publishing inline findings", async } }); +test("publishes remaining implementation findings once and exposes them as fix feedback", async () => { + const reviews: GiteaPullReview[] = []; + const created: CreatePullReviewInput[] = []; + const client = { + listPullReviews: async () => reviews, + createPullReview: async ( + _pull: number, + input: CreatePullReviewInput, + ) => { + created.push(input); + const review = pullReview( + 80, + "agent", + "COMMENT", + input.body, + input.commit_id, + ); + reviews.push(review); + return review; + }, + } as unknown as GiteaClient; + const job = { + id: "implementation-job", + issueNumber: 2, + } as Job; + const input = { + job, + pullRequestNumber: 7, + commitSha: "c".repeat(40), + review: { + verdict: "revise" as const, + findings: ["Add a regression test.", "Handle the error path."], + rationale: "Two blocking issues remain.", + }, + }; + const first = await publishRemainingImplementationReview( + publicationContext(client), + input, + ); + const second = await publishRemainingImplementationReview( + publicationContext(client), + input, + ); + assert.equal(first, 80); + assert.equal(second, 80); + assert.equal(created.length, 1); + assert.equal(created[0]?.event, "COMMENT"); + assert.equal(created[0]?.commit_id, input.commitSha); + assert.deepEqual(created[0]?.comments, []); + assert.match(created[0]?.body || "", /## Remaining issues/); + assert.match(created[0]?.body || "", /Add a regression test\./); + assert.match(created[0]?.body || "", /Handle the error path\./); + + const feedback = await collectPullRequestFeedback( + { + getComments: async () => [], + listPullReviews: async () => reviews, + listPullReviewComments: async () => [], + }, + 7, + "agent", + ); + assert.equal(feedback.length, 1); + assert.equal(feedback[0]?.kind, "review-summary"); + assert.match(feedback[0]?.body || "", /Two blocking issues remain\./); +}); + function reviewJob( workspace: string, base: string,