fix(mcp): preserve coveragePreflightRan across checkout_pr refreshes (#576)

checkout_pr unconditionally rebuilds ctx.toolState.diffCoverage via
createDiffCoverageState, which initialised coveragePreflightRan to false.
a second checkout_pr therefore reset the "one-time nudge per review
session" guarantee in runDiffCoveragePreflight, and the next
create_pull_request_review threw the diff-coverage pre-flight error
again — even after the agent had already gone through the
read-and-resubmit dance once.

createDiffCoverageState now accepts an optional previous state and
carries forward coveragePreflightRan. coveredRanges are intentionally
not carried because their line numbers are tied to the previous diff's
content (especially under incremental diffs).

closes #566

Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
Co-authored-by: David Blass <david@arktype.io>
This commit is contained in:
pullfrog[bot]
2026-05-06 20:54:34 +00:00
committed by pullfrog[bot]
parent 3c8b493aee
commit 6db4a6d02e
3 changed files with 24 additions and 1 deletions
+1
View File
@@ -545,6 +545,7 @@ export function CheckoutPrTool(ctx: ToolContext) {
diffPath, diffPath,
totalLines: countLines({ content: formatResult.content }), totalLines: countLines({ content: formatResult.content }),
toc: formatResult.toc, toc: formatResult.toc,
previous: ctx.toolState.diffCoverage,
}); });
log.debug( log.debug(
`» diff coverage initialized: diffPath=${diffPath}, totalLines=${ctx.toolState.diffCoverage.totalLines}, tocEntries=${ctx.toolState.diffCoverage.tocEntries.length}` `» diff coverage initialized: diffPath=${diffPath}, totalLines=${ctx.toolState.diffCoverage.totalLines}, tocEntries=${ctx.toolState.diffCoverage.tocEntries.length}`
+18
View File
@@ -98,6 +98,24 @@ describe("diff coverage line checker", () => {
]); ]);
}); });
it("carries forward coveragePreflightRan from a previous state across checkout refreshes", () => {
const previous = createDiffCoverageState({ diffPath, totalLines: 30, toc });
previous.coveragePreflightRan = true;
previous.coveredRanges = [{ startLine: 5, endLine: 10 }];
const next = createDiffCoverageState({ diffPath, totalLines: 50, toc, previous });
expect(next.coveragePreflightRan).toBe(true);
// coveredRanges are tied to the previous diff content and must not leak forward
expect(next.coveredRanges).toEqual([]);
expect(next.totalLines).toBe(50);
});
it("defaults coveragePreflightRan to false when no previous state is provided", () => {
const state = createDiffCoverageState({ diffPath, totalLines: 30, toc });
expect(state.coveragePreflightRan).toBe(false);
});
it("computes per-file unread ranges from tracked reads", () => { it("computes per-file unread ranges from tracked reads", () => {
const state = createDiffCoverageState({ const state = createDiffCoverageState({
diffPath, diffPath,
+5 -1
View File
@@ -78,13 +78,17 @@ export function createDiffCoverageState(params: {
diffPath: string; diffPath: string;
totalLines: number; totalLines: number;
toc: string; toc: string;
previous?: DiffCoverageState | undefined;
}): DiffCoverageState { }): DiffCoverageState {
return { return {
diffPath: params.diffPath, diffPath: params.diffPath,
totalLines: params.totalLines, totalLines: params.totalLines,
tocEntries: parseDiffTocEntries({ toc: params.toc }), tocEntries: parseDiffTocEntries({ toc: params.toc }),
coveredRanges: [], coveredRanges: [],
coveragePreflightRan: false, // carry forward across checkout_pr refreshes so the nudge stays "once per
// review session". coveredRanges are intentionally not carried because
// line numbers are tied to the previous diff's content.
coveragePreflightRan: params.previous?.coveragePreflightRan ?? false,
}; };
} }