Archived
feat: publish implementations with review findings
This commit is contained in:
@@ -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.
|
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-<number>-p<digest>`. 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-<number>-p<digest>`. 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.
|
`/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.
|
||||||
|
|
||||||
|
|||||||
@@ -135,12 +135,15 @@ export async function runImplementation(input: {
|
|||||||
input.signal,
|
input.signal,
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
if (review.verdict === "accept") {
|
if (
|
||||||
|
review.verdict === "accept" ||
|
||||||
|
(iteration === maximumIterations && files.length)
|
||||||
|
) {
|
||||||
const pendingFiles = await changedFiles(
|
const pendingFiles = await changedFiles(
|
||||||
input.workspace,
|
input.workspace,
|
||||||
options,
|
options,
|
||||||
);
|
);
|
||||||
return acceptedResult({
|
return implementationResult({
|
||||||
input,
|
input,
|
||||||
accepted,
|
accepted,
|
||||||
prepared,
|
prepared,
|
||||||
@@ -149,6 +152,7 @@ export async function runImplementation(input: {
|
|||||||
diff,
|
diff,
|
||||||
summary: summary.summary,
|
summary: summary.summary,
|
||||||
rationale: review.rationale,
|
rationale: review.rationale,
|
||||||
|
...(review.verdict === "revise" ? { review } : {}),
|
||||||
iteration,
|
iteration,
|
||||||
session,
|
session,
|
||||||
baseBranch: repository.default_branch,
|
baseBranch: repository.default_branch,
|
||||||
@@ -179,7 +183,7 @@ export async function runImplementation(input: {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
function acceptedResult(value: {
|
function implementationResult(value: {
|
||||||
input: { issueNumber: number };
|
input: { issueNumber: number };
|
||||||
accepted: NonNullable<ReturnType<typeof findAcceptedPlan>>;
|
accepted: NonNullable<ReturnType<typeof findAcceptedPlan>>;
|
||||||
prepared: {
|
prepared: {
|
||||||
@@ -192,6 +196,7 @@ function acceptedResult(value: {
|
|||||||
diff: string;
|
diff: string;
|
||||||
summary: string;
|
summary: string;
|
||||||
rationale: string;
|
rationale: string;
|
||||||
|
review?: ReturnType<typeof assertReviewDecision>;
|
||||||
iteration: number;
|
iteration: number;
|
||||||
session: string;
|
session: string;
|
||||||
baseBranch: string;
|
baseBranch: string;
|
||||||
@@ -205,7 +210,11 @@ function acceptedResult(value: {
|
|||||||
result: {
|
result: {
|
||||||
version: 1,
|
version: 1,
|
||||||
mode: "implement",
|
mode: "implement",
|
||||||
status: value.files.length ? "success" : "no-changes",
|
status: value.review
|
||||||
|
? "review-failed"
|
||||||
|
: value.files.length
|
||||||
|
? "success"
|
||||||
|
: "no-changes",
|
||||||
message: value.files.length
|
message: value.files.length
|
||||||
? value.rationale || "Implementation accepted"
|
? value.rationale || "Implementation accepted"
|
||||||
: `${value.summary}\n\nReviewer: ${value.rationale}`,
|
: `${value.summary}\n\nReviewer: ${value.rationale}`,
|
||||||
@@ -222,6 +231,7 @@ function acceptedResult(value: {
|
|||||||
pendingFiles: value.pendingFiles,
|
pendingFiles: value.pendingFiles,
|
||||||
summary: value.summary,
|
summary: value.summary,
|
||||||
iterations: value.iteration,
|
iterations: value.iteration,
|
||||||
|
...(value.review ? { review: value.review } : {}),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import {
|
|||||||
marker,
|
marker,
|
||||||
parseMarker,
|
parseMarker,
|
||||||
protocolVersion,
|
protocolVersion,
|
||||||
|
type ReviewDecision,
|
||||||
sha256,
|
sha256,
|
||||||
} from "../../../../core/contracts.js";
|
} from "../../../../core/contracts.js";
|
||||||
import {
|
import {
|
||||||
@@ -93,6 +94,53 @@ export async function publishPullReview(
|
|||||||
return { terminal: "succeeded", reviewId: published.id };
|
return { terminal: "succeeded", reviewId: published.id };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function publishRemainingImplementationReview(
|
||||||
|
context: PublicationContext,
|
||||||
|
input: {
|
||||||
|
job: Job;
|
||||||
|
pullRequestNumber: number;
|
||||||
|
commitSha: string;
|
||||||
|
review: ReviewDecision;
|
||||||
|
},
|
||||||
|
): Promise<number> {
|
||||||
|
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(
|
async function publishStatus(
|
||||||
context: PublicationContext,
|
context: PublicationContext,
|
||||||
job: Job,
|
job: Job,
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import {
|
|||||||
type PublicationOutcome,
|
type PublicationOutcome,
|
||||||
upsertJobStatus,
|
upsertJobStatus,
|
||||||
} from "../status.js";
|
} from "../status.js";
|
||||||
|
import { publishRemainingImplementationReview } from "./conversations/review.js";
|
||||||
|
|
||||||
export async function publishImplementation(
|
export async function publishImplementation(
|
||||||
context: PublicationContext,
|
context: PublicationContext,
|
||||||
@@ -39,6 +40,13 @@ export async function publishImplementation(
|
|||||||
throw new Error(
|
throw new Error(
|
||||||
"Successful implementation job has no workspace result",
|
"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([
|
const [issue, comments, currentBase] = await Promise.all([
|
||||||
context.client.getIssue(job.issueNumber),
|
context.client.getIssue(job.issueNumber),
|
||||||
context.client.getComments(job.issueNumber),
|
context.client.getComments(job.issueNumber),
|
||||||
@@ -109,12 +117,23 @@ export async function publishImplementation(
|
|||||||
issue.title,
|
issue.title,
|
||||||
);
|
);
|
||||||
await context.client.addLabelIfPresent(pull.number, generatedLabel);
|
await context.client.addLabelIfPresent(pull.number, generatedLabel);
|
||||||
|
if (remainingReview)
|
||||||
|
await publishRemainingImplementationReview(context, {
|
||||||
|
job,
|
||||||
|
pullRequestNumber: pull.number,
|
||||||
|
commitSha,
|
||||||
|
review: remainingReview,
|
||||||
|
});
|
||||||
await upsertJobStatus(
|
await upsertJobStatus(
|
||||||
context.client,
|
context.client,
|
||||||
context.botLogin,
|
context.botLogin,
|
||||||
job,
|
job,
|
||||||
"Agent implementation ready",
|
remainingReview
|
||||||
`[Pull request #${pull.number}](${pull.html_url}) was created or updated.`,
|
? "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 };
|
return { terminal: "succeeded", commitSha, pullRequestNumber: pull.number };
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -69,6 +69,7 @@ export interface ImplementationData {
|
|||||||
pendingFiles: string[];
|
pendingFiles: string[];
|
||||||
summary: string;
|
summary: string;
|
||||||
iterations: number;
|
iterations: number;
|
||||||
|
review?: ReviewDecision;
|
||||||
pullRequestNumber?: number;
|
pullRequestNumber?: number;
|
||||||
response?: string;
|
response?: string;
|
||||||
feedback?: FeedbackReference[];
|
feedback?: FeedbackReference[];
|
||||||
@@ -77,7 +78,7 @@ export interface ImplementationData {
|
|||||||
export interface Result {
|
export interface Result {
|
||||||
version: number;
|
version: number;
|
||||||
mode: Mode;
|
mode: Mode;
|
||||||
status: "success" | "no-changes" | "failed";
|
status: "success" | "no-changes" | "review-failed" | "failed";
|
||||||
message: string;
|
message: string;
|
||||||
plan?: PlanData;
|
plan?: PlanData;
|
||||||
implementation?: ImplementationData;
|
implementation?: ImplementationData;
|
||||||
|
|||||||
@@ -18,7 +18,10 @@ import type {
|
|||||||
GiteaPullRequest,
|
GiteaPullRequest,
|
||||||
GiteaPullReview,
|
GiteaPullReview,
|
||||||
} from "../../adapters/gitea/types.js";
|
} 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 type { PublicationContext } from "../../application/publication/status.js";
|
||||||
import { marker, protocolVersion, sha256 } from "../../core/contracts.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(
|
function reviewJob(
|
||||||
workspace: string,
|
workspace: string,
|
||||||
base: string,
|
base: string,
|
||||||
|
|||||||
Reference in New Issue
Block a user