diff --git a/docs/pr-review-comments.md b/docs/pr-review-comments.md index f14332b01d..94eb765582 100644 --- a/docs/pr-review-comments.md +++ b/docs/pr-review-comments.md @@ -27,10 +27,16 @@ Each synced comment includes the durable identity marker: ClawSweeper edits that comment in place instead of posting repeated comments. Report front matter stores the synced comment id, URL, hash, and sync time. -When review starts and no ClawSweeper-owned comment exists yet, the review -shard posts a short status placeholder with the same durable identity marker. -The placeholder is intentionally light and crustacean-friendly, then the final -review sync edits that exact comment in place. +The review shard uses a separate transient status comment as its coordination +lease. When a durable review already exists, acquiring that lease also edits +the durable comment to show that a fresh review is in progress, marks the old +review stale, and collapses it as previous context. The status projection has +no active verdict or action markers, so downstream automation cannot act on the +displaced review. A matching apply run replaces the projection with the new +completed review; an abandoned or expired lease marks the refresh interrupted. + +When no durable review exists yet, the transient lease comment remains the only +start status until the first completed review is published. For a PR that needs work, the visible comment starts with: diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 7a8d494708..effc3b4638 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -187,6 +187,11 @@ import { type ReviewHistoryCycle, type ReviewHistoryLedger, } from "./review-history.js"; +import { + parseDurableReviewStatusProjection, + renderDurableReviewRefreshProjection, + renderInterruptedDurableReviewProjection, +} from "./review-comment-status.js"; import { trailingHtmlComments } from "./review-comment-markers.js"; import { MAX_REVIEWED_PR_ACTIVITY, @@ -5683,29 +5688,41 @@ function extractLatestClawSweeperReview( if (!latest) return null; const comment = asRecord(latest); const body = rawCommentBody(latest); - const verdictMarker = htmlMarkerWithPrefix(body, "clawsweeper-verdict:"); - const actionMarker = htmlMarkerWithPrefix(body, "clawsweeper-action:"); - const history = parseReviewHistory(body); - const currentCycle = reviewHistoryCycleFromCommentBody(body); + const projection = parseDurableReviewStatusProjection(body, number); + const reviewBody = projection?.previousBody ?? body; + const verdictMarker = projection + ? null + : htmlMarkerWithPrefix(reviewBody, "clawsweeper-verdict:"); + const actionMarker = projection ? null : htmlMarkerWithPrefix(reviewBody, "clawsweeper-action:"); + const history = parseReviewHistory(reviewBody); + const currentCycle = reviewHistoryCycleFromCommentBody(reviewBody, { + reviewedAt: projection?.previousReviewedAt, + sha: projection?.previousSha, + }); const latestCompletedCycle = currentCycle ?? history.cycles.at(-1); const earlierReviewCycles = currentCycle ? history.cycles : history.cycles.slice(0, -1); return { - status: previousReviewStatus(body), - verdictDigest: reviewCommentBodyDigest(body), - reviewedAt: previousReviewReviewedAt(body) ?? latestCompletedCycle?.reviewedAt ?? null, + status: previousReviewStatus(reviewBody), + verdictDigest: projection?.previousDigest ?? reviewCommentBodyDigest(reviewBody), + reviewedAt: + projection?.previousReviewedAt ?? + previousReviewReviewedAt(reviewBody) ?? + latestCompletedCycle?.reviewedAt ?? + null, reviewedSha: + projection?.previousSha ?? markerAttribute(verdictMarker, "sha") ?? markerAttribute(actionMarker, "sha") ?? latestCompletedCycle?.sha ?? null, verdictMarker, actionMarker, - summary: firstNonEmptyLine(markdownSection(body, "Summary")), - proofStatus: previousReviewProofStatus(body), - rating: previousReviewRating(body), + summary: firstNonEmptyLine(markdownSection(reviewBody, "Summary")), + proofStatus: previousReviewProofStatus(reviewBody), + rating: previousReviewRating(reviewBody), nextStep: - firstNonEmptyLine(markdownSection(body, "Next step before merge")) || - firstNonEmptyLine(markdownSection(body, "Next step")), + firstNonEmptyLine(markdownSection(reviewBody, "Next step before merge")) || + firstNonEmptyLine(markdownSection(reviewBody, "Next step")), findings: reviewHistoryFindings(latestCompletedCycle), earlierReviewCycles, completedReviewCycles: history.totalCompletedCycles + (currentCycle ? 1 : 0), @@ -18506,8 +18523,13 @@ function reviewHistoryForRender( } const body = previousReviewCommentBody ?? ""; if (!body.trim()) return { cycles: [], totalCompletedCycles: 0 }; - const history = parseReviewHistory(body); - const previousCycle = reviewHistoryCycleFromCommentBody(body); + const projection = parseDurableReviewStatusProjection(body); + const reviewBody = projection?.previousBody ?? body; + const history = parseReviewHistory(reviewBody); + const previousCycle = reviewHistoryCycleFromCommentBody(reviewBody, { + reviewedAt: projection?.previousReviewedAt, + sha: projection?.previousSha, + }); if (!previousCycle) return history; const reviewedAt = frontMatterValue(markdown, "reviewed_at"); if (reviewedAt && previousCycle.reviewedAt === reviewedAt) return history; @@ -18518,8 +18540,16 @@ function reviewHistoryForStaleComment( previousReviewCommentBody: string | undefined, ): ReviewHistoryLedger { const body = previousReviewCommentBody ?? ""; - const history = parseReviewHistory(body); - return appendReviewHistoryCycle(history, reviewHistoryCycleFromCommentBody(body)); + const projection = parseDurableReviewStatusProjection(body); + const reviewBody = projection?.previousBody ?? body; + const history = parseReviewHistory(reviewBody); + return appendReviewHistoryCycle( + history, + reviewHistoryCycleFromCommentBody(reviewBody, { + reviewedAt: projection?.previousReviewedAt, + sha: projection?.previousSha, + }), + ); } function renderKeepOpenCommentFromReport( @@ -19848,6 +19878,18 @@ function durableReviewVersion( if (!canPatchReviewComment(comment)) return null; const body = commentBody(comment); if (!body) return null; + const projection = parseDurableReviewStatusProjection(body, number); + if (projection) { + // The projection's lease tuple is the publication fence: only the report + // produced by that exact refresh may replace the visible pending state. + return { + reviewedAt: projection.startedAt, + headSha: projection.targetRevision, + sourceRevision: projection.targetRevision, + leaseOwner: projection.state === "refreshing" ? projection.leaseOwner : null, + leaseCommentId: String(projection.leaseCommentId), + }; + } const identity = reviewCommentMarker(number); const identityIndex = body.lastIndexOf(identity); if (identityIndex < 0 || body.slice(identityIndex + identity.length).trim()) return null; @@ -20437,13 +20479,14 @@ function postReviewStartStatusComment(options: { bulkFilerLabelApplied?: boolean; }): ReviewStartStatusCommentResult { const startedAtMs = Date.now(); + const startedAt = new Date(startedAtMs).toISOString(); const leaseOwner = newReviewStartLeaseOwner(); const leaseOptions: ReviewStartStatusCommentOptions = { number: options.item.number, kind: options.item.kind, title: options.item.title, ...(options.headSha ? { headSha: options.headSha } : {}), - startedAt: new Date(startedAtMs).toISOString(), + startedAt, leaseExpiresAt: new Date(startedAtMs + options.reviewTimeoutMs + 10 * 60 * 1000).toISOString(), leaseOwner, position: options.position, @@ -20526,6 +20569,13 @@ function postReviewStartStatusComment(options: { deleteOwnedDedicatedReviewStartLease(options.item.number, acquired); return heldReviewStartStatusCommentResult(winner.expiresAt, true); } + projectExistingDurableReviewForLease({ + itemNumber: options.item.number, + targetRevision: normalizedHead, + startedAt, + leaseOwner, + leaseCommentId: createdCommentId, + }); return { status: "posted", lease: { ...acquired, comment: winner.comment }, @@ -20533,6 +20583,120 @@ function postReviewStartStatusComment(options: { }; } +function currentWorkflowRunUrl(): string | null { + const serverUrl = String(process.env.GITHUB_SERVER_URL ?? "https://github.com").replace( + /\/$/, + "", + ); + const repository = String(process.env.GITHUB_REPOSITORY ?? "").trim(); + const runId = String(process.env.GITHUB_RUN_ID ?? "").trim(); + if (!/^https:\/\/[^\s]+$/.test(serverUrl) || !/^[\w.-]+\/[\w.-]+$/.test(repository)) { + return null; + } + return /^[1-9]\d*$/.test(runId) ? `${serverUrl}/${repository}/actions/runs/${runId}` : null; +} + +function patchDurableReviewStatusBody(options: { + itemNumber: number; + comment: Record | undefined; + nextBody: string | null; + identity: string; +}): void { + const currentBody = commentBody(options.comment); + const id = commentId(options.comment); + if (!currentBody || id === null || !options.nextBody || options.nextBody === currentBody) return; + const endpoint = `repos/${targetRepo()}/issues/comments/${id}`; + const snapshot = ghWithRetry(["api", "--include", endpoint]); + const etag = snapshot.match(/^etag:\s*(.+)\r?$/im)?.[1]?.trim(); + const jsonStart = snapshot.lastIndexOf("\n{"); + const liveComment = + jsonStart >= 0 ? asRecord(JSON.parse(snapshot.slice(jsonStart + 1)) as unknown) : {}; + if (!etag || commentBody(liveComment) !== currentBody) return; + const payload = writeCommentPayload(options.itemNumber, options.nextBody); + const args = [ + "api", + endpoint, + "--method", + "PATCH", + "-H", + `If-Match: ${etag}`, + "--input", + payload, + ]; + ghObservedMutationCommand({ + identity: options.identity, + args, + knownNoMutation: (error) => + /(?:\b412\b|precondition failed)/i.test(mutationErrorMessage(error)), + }); +} + +function projectExistingDurableReviewForLease(options: { + itemNumber: number; + targetRevision: string; + startedAt: string; + leaseOwner: string; + leaseCommentId: number; +}): void { + try { + const durable = issueReviewCommentState(options.itemNumber).reviewComment; + const body = commentBody(durable); + if (!body || !canPatchReviewComment(durable) || commentId(durable) === options.leaseCommentId) { + return; + } + const previous = extractLatestClawSweeperReview([durable], options.itemNumber); + const nextBody = renderDurableReviewRefreshProjection(body, { + ...options, + previousReviewedAt: previous?.reviewedAt, + previousSha: previous?.reviewedSha, + workflowUrl: currentWorkflowRunUrl(), + }); + patchDurableReviewStatusBody({ + itemNumber: options.itemNumber, + comment: durable, + nextBody, + identity: `durable_review_refresh:${options.itemNumber}:${options.leaseCommentId}`, + }); + } catch (error) { + // The dedicated lease remains authoritative. A presentation-only patch must + // never prevent the already-coordinated review from producing a new verdict. + console.error( + `[review] could not project durable review refresh for #${options.itemNumber}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +} + +function interruptDurableReviewProjectionForLease( + itemNumber: number, + lease: Pick, +): void { + try { + const durable = issueReviewCommentState(itemNumber).reviewComment; + const body = commentBody(durable); + if (!body || !canPatchReviewComment(durable)) return; + const nextBody = renderInterruptedDurableReviewProjection(body, { + itemNumber, + leaseOwner: lease.owner, + leaseCommentId: lease.commentId, + targetRevision: lease.headSha, + }); + patchDurableReviewStatusBody({ + itemNumber, + comment: durable, + nextBody, + identity: `durable_review_interrupted:${itemNumber}:${lease.commentId}`, + }); + } catch (error) { + console.error( + `[review] could not mark durable review refresh interrupted for #${itemNumber}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +} + function patchOwnedBulkFilerReviewStartStatusComment( itemNumber: number, lease: AcquiredReviewStartLease, @@ -20599,6 +20763,7 @@ function deleteOwnedDedicatedReviewStartLease( (commentBody(comment) ?? "").includes(`sha=${lease.headSha}`), ); if (!matching) return false; + interruptDurableReviewProjectionForLease(itemNumber, lease); ghObservedMutationCommand({ identity: `review_lease_delete:${itemNumber}:${lease.commentId}`, args: [ @@ -20635,6 +20800,20 @@ function reapExpiredDedicatedReviewStartLeases( }); for (const lease of expired) { try { + const matchingComment = dedicatedLeaseComments.find( + (comment) => commentId(comment) === lease.commentId, + ); + const owner = reviewStartLeaseOwner(matchingComment); + const headSha = (commentBody(matchingComment) ?? "").match( + /"; +const PREVIOUS_END = ""; +const IDENTITY_PATTERN = /\s*$/i; +const STATUS_PATTERN = /\s*$/i; + +export interface DurableReviewStatusProjection { + state: "refreshing" | "interrupted"; + itemNumber: number; + targetRevision: string; + startedAt: string; + leaseOwner: string; + leaseCommentId: number; + previousDigest: string; + previousVisibleDigest: string; + previousReviewedAt: string | null; + previousSha: string | null; + previousBody: string; +} + +export interface DurableReviewRefreshOptions { + itemNumber: number; + targetRevision: string; + startedAt: string; + leaseOwner: string; + leaseCommentId: number; + previousReviewedAt?: string | null | undefined; + previousSha?: string | null | undefined; + workflowUrl?: string | null; +} + +function digest(body: string): string { + return createHash("sha256").update(body.trim()).digest("hex"); +} + +function markerValue(value: string): string { + return value.trim().replace(/[^\w./:@-]/g, "_") || "unknown"; +} + +function markerAttributes(source: string): Map { + const attributes = new Map(); + for (const token of source.trim().split(/\s+/)) { + const separator = token.indexOf("="); + if (separator > 0) attributes.set(token.slice(0, separator), token.slice(separator + 1)); + } + return attributes; +} + +function validTimestamp(value: string): boolean { + return Boolean(value) && Number.isFinite(Date.parse(value)); +} + +function sanitizePreviousBody(body: string): string { + return body + .replace(//gi, "") + .replace(/\n{3,}/g, "\n\n") + .trim(); +} + +function truncateUtf8(value: string, maxBytes: number): string { + const bytes = Buffer.from(value, "utf8"); + if (bytes.length <= maxBytes) return value; + return `${bytes + .subarray(0, maxBytes) + .toString("utf8") + .replace(/\uFFFD$/, "") + .trimEnd()}\n\n_[Previous review truncated while a fresh review runs.]_`; +} + +function safeWorkflowUrl(value: string | null | undefined): string | null { + const trimmed = value?.trim() ?? ""; + return trimmed.length <= 512 && /^https:\/\/[^\s()[\]]+$/.test(trimmed) ? trimmed : null; +} + +export function parseDurableReviewStatusProjection( + body: string, + expectedItemNumber?: number, +): DurableReviewStatusProjection | null { + const normalized = body.replace(/\r\n?/g, "\n").trim(); + const identity = normalized.match(IDENTITY_PATTERN); + const itemNumber = Number(identity?.[1]); + if (!identity || !Number.isInteger(itemNumber) || itemNumber <= 0) return null; + if (expectedItemNumber !== undefined && itemNumber !== expectedItemNumber) return null; + + const beforeIdentity = normalized.slice(0, identity.index).trimEnd(); + const status = beforeIdentity.match(STATUS_PATTERN); + if (!status?.[1]) return null; + const attributes = markerAttributes(status[2] ?? ""); + const leaseCommentId = Number(attributes.get("lease_comment_id")); + const targetRevision = attributes.get("target_revision") ?? ""; + const startedAt = attributes.get("started_at") ?? ""; + const leaseOwner = attributes.get("lease_owner") ?? ""; + const previousDigest = attributes.get("previous_digest") ?? ""; + const previousVisibleDigest = attributes.get("previous_visible_digest") ?? ""; + if ( + attributes.get("v") !== "1" || + Number(attributes.get("item")) !== itemNumber || + !/^[0-9a-f]{40}(?:[0-9a-f]{24})?$/.test(targetRevision) || + !validTimestamp(startedAt) || + !/^[\w./:@-]+$/.test(leaseOwner) || + !Number.isSafeInteger(leaseCommentId) || + leaseCommentId <= 0 || + !/^[0-9a-f]{64}$/.test(previousDigest) || + !/^[0-9a-f]{64}$/.test(previousVisibleDigest) + ) { + return null; + } + + const visible = beforeIdentity.slice(0, status.index).trimEnd(); + const previousStart = visible.indexOf(PREVIOUS_START); + const previousEnd = visible.indexOf(PREVIOUS_END); + if ( + previousStart < 0 || + previousEnd <= previousStart || + visible.indexOf(PREVIOUS_START, previousStart + PREVIOUS_START.length) >= 0 || + visible.indexOf(PREVIOUS_END, previousEnd + PREVIOUS_END.length) >= 0 + ) { + return null; + } + const previousBody = visible.slice(previousStart + PREVIOUS_START.length, previousEnd).trim(); + if (!previousBody || digest(previousBody) !== previousVisibleDigest) return null; + + const nullableAttribute = (name: string): string | null => { + const value = attributes.get(name); + return !value || value === "na" || value === "unknown" ? null : value; + }; + const previousReviewedAt = nullableAttribute("previous_reviewed_at"); + const previousSha = nullableAttribute("previous_sha"); + if (previousReviewedAt && !validTimestamp(previousReviewedAt)) return null; + if (previousSha && !/^[\w./:@-]+$/.test(previousSha)) return null; + return { + state: status[1].toLowerCase() as DurableReviewStatusProjection["state"], + itemNumber, + targetRevision, + startedAt, + leaseOwner, + leaseCommentId, + previousDigest, + previousVisibleDigest, + previousReviewedAt, + previousSha, + previousBody, + }; +} + +function renderProjection( + projection: Omit & { + state: DurableReviewStatusProjection["state"]; + }, + workflowUrl?: string | null, +): string | null { + const previousVisibleDigest = digest(projection.previousBody); + const interrupted = projection.state === "interrupted"; + const title = interrupted + ? `Review refresh for \`${projection.targetRevision.slice(0, 12)}\` was interrupted.` + : `Fresh ClawSweeper review in progress for \`${projection.targetRevision.slice(0, 12)}\`.`; + const explanation = interrupted + ? "The previous review below is stale. A new review must complete before it can be used for merge decisions." + : "The previous review below is stale and must not be used for merge decisions."; + const workflow = !interrupted && workflowUrl ? ` [View workflow](${workflowUrl})` : ""; + const attributes = [ + `item=${projection.itemNumber}`, + `target_revision=${projection.targetRevision}`, + `started_at=${markerValue(projection.startedAt)}`, + `lease_owner=${markerValue(projection.leaseOwner)}`, + `lease_comment_id=${projection.leaseCommentId}`, + `previous_digest=${projection.previousDigest}`, + `previous_visible_digest=${previousVisibleDigest}`, + `previous_reviewed_at=${markerValue(projection.previousReviewedAt ?? "na")}`, + `previous_sha=${markerValue(projection.previousSha ?? "na")}`, + "v=1", + ].join(" "); + const body = [ + interrupted ? "> [!CAUTION]" : "> [!WARNING]", + `> **${title}**`, + `> ${explanation}`, + `> Started ${projection.startedAt}.${workflow}`, + "", + "
", + `Previous review (stale${interrupted ? " after interrupted refresh" : " while refresh runs"})`, + "", + PREVIOUS_START, + projection.previousBody, + PREVIOUS_END, + "", + "
", + "", + ``, + "", + ``, + ].join("\n"); + return Buffer.byteLength(body, "utf8") <= COMMENT_MAX_BYTES ? body : null; +} + +export function renderDurableReviewRefreshProjection( + previousBody: string, + options: DurableReviewRefreshOptions, +): string | null { + const existing = parseDurableReviewStatusProjection(previousBody, options.itemNumber); + const originalPreviousBody = existing?.previousBody ?? previousBody; + const visiblePreviousBody = sanitizePreviousBody(originalPreviousBody); + if (!visiblePreviousBody) return null; + const projection = { + state: "refreshing" as const, + itemNumber: options.itemNumber, + targetRevision: options.targetRevision.trim().toLowerCase(), + startedAt: options.startedAt, + leaseOwner: options.leaseOwner, + leaseCommentId: options.leaseCommentId, + previousDigest: existing?.previousDigest ?? digest(originalPreviousBody), + previousReviewedAt: existing?.previousReviewedAt ?? options.previousReviewedAt ?? null, + previousSha: existing?.previousSha ?? options.previousSha ?? null, + previousBody: existing?.previousBody ?? visiblePreviousBody, + }; + const workflowUrl = safeWorkflowUrl(options.workflowUrl); + return ( + renderProjection(projection, workflowUrl) ?? + renderProjection({ ...projection, previousBody: truncateUtf8(projection.previousBody, 48_000) }) + ); +} + +export function renderInterruptedDurableReviewProjection( + body: string, + expected: { + itemNumber: number; + leaseOwner: string; + leaseCommentId: number; + targetRevision: string; + }, +): string | null { + const projection = parseDurableReviewStatusProjection(body, expected.itemNumber); + if ( + !projection || + projection.state !== "refreshing" || + projection.leaseOwner !== expected.leaseOwner || + projection.leaseCommentId !== expected.leaseCommentId || + projection.targetRevision !== expected.targetRevision.trim().toLowerCase() + ) { + return null; + } + return renderProjection({ ...projection, state: "interrupted" }); +} + +export function isDurableReviewStatusProjection(body: string): boolean { + return parseDurableReviewStatusProjection(body) !== null; +} diff --git a/src/review-history.ts b/src/review-history.ts index 12530839d9..bdebfc4a2b 100644 --- a/src/review-history.ts +++ b/src/review-history.ts @@ -300,7 +300,10 @@ function commentBodyFindings(body: string): string[] { return [...new Set(findings)].slice(0, MAX_CYCLE_FINDINGS); } -export function reviewHistoryCycleFromCommentBody(body: string): ReviewHistoryCycle | null { +export function reviewHistoryCycleFromCommentBody( + body: string, + metadata: { reviewedAt?: string | null | undefined; sha?: string | null | undefined } = {}, +): ReviewHistoryCycle | null { if ( !body.trim() || body.includes(REVIEW_START_PLACEHOLDER) || @@ -322,7 +325,11 @@ export function reviewHistoryCycleFromCommentBody(body: string): ReviewHistoryCy if (freshnessIndex >= 0) verdict = verdict.slice(0, freshnessIndex).trim(); if (!verdict || verdict === FAILED_REVIEW_VERDICT) return null; const inlineReviewedAt = body.match(/_reviewed ([^_]+?)\.?_/i)?.[1]?.trim(); - const reviewedAt = reviewMarkerAttribute(body, "reviewed_at") ?? inlineReviewedAt ?? "unknown"; - const sha = reviewMarkerAttribute(body, "sha") ?? "unknown"; + const reviewedAt = + metadata.reviewedAt ?? + reviewMarkerAttribute(body, "reviewed_at") ?? + inlineReviewedAt ?? + "unknown"; + const sha = metadata.sha ?? reviewMarkerAttribute(body, "sha") ?? "unknown"; return { reviewedAt, sha, verdict, findings: commentBodyFindings(body) }; } diff --git a/test/repair/comment-router-core.test.ts b/test/repair/comment-router-core.test.ts index b28d9a9bc8..4ff09fa736 100644 --- a/test/repair/comment-router-core.test.ts +++ b/test/repair/comment-router-core.test.ts @@ -1301,6 +1301,41 @@ test("parseTrustedAutomation accepts only trusted ClawSweeper repair signals", ( ); }); +test("parseTrustedAutomation ignores durable review refresh projections", async () => { + const { renderDurableReviewRefreshProjection, renderInterruptedDurableReviewProjection } = + await import("../../dist/review-comment-status.js"); + const trustedAuthors = new Set(["clawsweeper[bot]"]); + const projection = renderDurableReviewRefreshProjection( + [ + "Codex review: needs changes before merge.", + "", + "- **[P1] Old finding:** must not trigger repair while stale.", + "", + "", + "", + "", + ].join("\n"), + { + itemNumber: 42, + targetRevision: "0123456789abcdef0123456789abcdef01234567", + startedAt: "2026-07-17T03:50:00.000Z", + leaseOwner: "github-run-123-1", + leaseCommentId: 987, + }, + ); + assert.ok(projection); + const comment = { user: { login: "clawsweeper[bot]" }, body: projection }; + assert.equal(parseTrustedAutomation(comment, { trustedAuthors }), null); + const interrupted = renderInterruptedDurableReviewProjection(projection, { + itemNumber: 42, + targetRevision: "0123456789abcdef0123456789abcdef01234567", + leaseOwner: "github-run-123-1", + leaseCommentId: 987, + }); + assert.ok(interrupted); + assert.equal(parseTrustedAutomation({ ...comment, body: interrupted }, { trustedAuthors }), null); +}); + test("parseRoutedCommentCommand ignores proof-nudge marker comments", () => { const trustedAuthors = new Set(["clawsweeper[bot]"]); const comment = { diff --git a/test/review-comment-rendering.test.ts b/test/review-comment-rendering.test.ts index 08bbe49469..71506e1f81 100644 --- a/test/review-comment-rendering.test.ts +++ b/test/review-comment-rendering.test.ts @@ -300,6 +300,11 @@ test("spoofed durable markers cannot suppress a bot-owned start lease", () => { assert.match(postStart, /heldReviewStartStatusCommentResult\(initialLease\.expiresAt, false\)/); assert.match(postStart, /heldReviewStartStatusCommentResult\(winner\.expiresAt, true\)/); assert.match(postStart, /issues\/\$\{options\.item\.number\}\/comments/); + assert.match(postStart, /projectExistingDurableReviewForLease\(\{/); + assert.match(postStart, /durable_review_refresh/); + assert.match(postStart, /renderInterruptedDurableReviewProjection/); + assert.match(postStart, /If-Match/); + assert.match(postStart, /precondition failed/); }); test("review start status comment is marker-backed and crustacean-friendly", () => { diff --git a/test/review-comment-status.test.ts b/test/review-comment-status.test.ts new file mode 100644 index 0000000000..f0ac23d8c1 --- /dev/null +++ b/test/review-comment-status.test.ts @@ -0,0 +1,130 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + parseDurableReviewStatusProjection, + renderDurableReviewRefreshProjection, + renderInterruptedDurableReviewProjection, +} from "../dist/review-comment-status.js"; + +const options = { + itemNumber: 74453, + targetRevision: "0123456789abcdef0123456789abcdef01234567", + startedAt: "2026-07-17T03:50:00.000Z", + leaseOwner: "github-run-123-1", + leaseCommentId: 987, + previousReviewedAt: "2026-07-16T12:00:00.000Z", + previousSha: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + workflowUrl: "https://github.com/openclaw/clawsweeper/actions/runs/123", +}; + +function previousReview(): string { + return [ + "Codex review: needs changes before merge.", + "", + "**Review findings**", + "- [P1] Fix the stale cache", + "", + "
", + "Review history (1 earlier review cycle)", + "", + "", + "- reviewed 2026-07-15T10:00:00.000Z sha bbb222 :: passed. :: none", + "", + "
", + "", + "", + "", + "", + "", + "", + ].join("\n"); +} + +test("refresh projection visibly stales the previous review without actionable markers", () => { + const body = renderDurableReviewRefreshProjection(previousReview(), options); + assert.ok(body); + assert.match(body, /Fresh ClawSweeper review in progress/); + assert.match(body, /Previous review \(stale while refresh runs\)/); + assert.match(body, /Started 2026-07-17T03:50:00.000Z/); + assert.match(body, /View workflow/); + assert.match(body, /clawsweeper-review-history/); + assert.doesNotMatch(body, /clawsweeper-verdict:/); + assert.doesNotMatch(body, /clawsweeper-action:/); + assert.doesNotMatch(body, /clawsweeper-security:/); + assert.doesNotMatch(body, /clawsweeper-review-version/); + assert.equal(body.match(//g)?.length, 1); + + const parsed = parseDurableReviewStatusProjection(body, 74453); + assert.equal(parsed?.state, "refreshing"); + assert.equal(parsed?.previousReviewedAt, options.previousReviewedAt); + assert.equal(parsed?.previousSha, options.previousSha); +}); + +test("refresh projection unwraps an existing projection instead of nesting it", () => { + const first = renderDurableReviewRefreshProjection(previousReview(), options); + assert.ok(first); + const second = renderDurableReviewRefreshProjection(first, { + ...options, + targetRevision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + leaseCommentId: 988, + }); + assert.ok(second); + assert.equal(second.match(/clawsweeper-review-previous:start/g)?.length, 1); + assert.equal( + parseDurableReviewStatusProjection(second)?.previousDigest, + parseDurableReviewStatusProjection(first)?.previousDigest, + ); +}); + +test("parser rejects malformed, mismatched, and forged projections", () => { + const body = renderDurableReviewRefreshProjection(previousReview(), options); + assert.ok(body); + assert.equal(parseDurableReviewStatusProjection(body, 1), null); + assert.equal( + parseDurableReviewStatusProjection(body.replace("Fix the stale cache", "Forged")), + null, + ); + assert.equal(parseDurableReviewStatusProjection("ordinary prose"), null); +}); + +test("marker-like prior prose cannot forge projection delimiters", () => { + const body = renderDurableReviewRefreshProjection( + `${previousReview()}\n\nSpoof `, + options, + ); + assert.ok(body); + assert.equal(body.match(/clawsweeper-review-previous:end/g)?.length, 1); + assert.ok(parseDurableReviewStatusProjection(body)); +}); + +test("interrupted transition is tuple-bound and keeps the stale prior review inert", () => { + const refreshing = renderDurableReviewRefreshProjection(previousReview(), options); + assert.ok(refreshing); + assert.equal( + renderInterruptedDurableReviewProjection(refreshing, { + itemNumber: options.itemNumber, + leaseOwner: options.leaseOwner, + leaseCommentId: 999, + targetRevision: options.targetRevision, + }), + null, + ); + const interrupted = renderInterruptedDurableReviewProjection(refreshing, options); + assert.ok(interrupted); + assert.match(interrupted, /was interrupted/); + assert.equal(parseDurableReviewStatusProjection(interrupted)?.state, "interrupted"); + assert.doesNotMatch(interrupted, /clawsweeper-verdict:/); +}); + +test("over-limit prior reviews produce a bounded inert projection", () => { + const body = renderDurableReviewRefreshProjection( + `Codex review: passed.\n${"x".repeat(70_000)}\n`, + options, + ); + assert.ok(body); + assert.ok(Buffer.byteLength(body, "utf8") <= 65_536); + assert.match(body, /Previous review truncated/); + assert.doesNotMatch(body, /clawsweeper-verdict:/); + assert.ok(parseDurableReviewStatusProjection(body)); +}); diff --git a/test/review-history.test.ts b/test/review-history.test.ts index cbbaef8dfe..473a26c02a 100644 --- a/test/review-history.test.ts +++ b/test/review-history.test.ts @@ -19,6 +19,7 @@ import { renderReviewCommentFromReport, } from "../dist/clawsweeper.js"; import { reviewSemanticPriorReviewDigest } from "../dist/review-semantic-cache.js"; +import { renderDurableReviewRefreshProjection } from "../dist/review-comment-status.js"; import { changelogReviewDecision, markedReviewCommentForTest, @@ -564,6 +565,27 @@ test("keep-open PR comment carries the previous review as an earlier cycle", () assert.equal(parsed.cycles.length, 1); }); +test("publishing after a refresh projection carries the displaced review exactly once", () => { + const projection = renderDurableReviewRefreshProjection(previousDurableComment(), { + itemNumber: 101, + targetRevision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + startedAt: "2026-07-17T03:50:00.000Z", + leaseOwner: "github-run-123-1", + leaseCommentId: 987, + previousReviewedAt: "2026-06-20T10:00:00.000Z", + previousSha: "abc1234def", + }); + assert.ok(projection); + const comment = renderReviewCommentFromReport(keepOpenPullReport(), "none", { + prStatusKind: "ready_for_maintainer_look", + previousReviewCommentBody: projection, + }); + const parsed = parseReviewHistory(comment); + assert.equal(parsed.totalCompletedCycles, 1); + assert.equal(parsed.cycles.length, 1); + assert.equal(parsed.cycles[0]?.sha, "abc1234def"); +}); + test("re-syncing the same review does not add a duplicate cycle", () => { const reviewedAt = "2026-06-24T12:00:00.000Z"; const comment = renderReviewCommentFromReport( @@ -669,6 +691,46 @@ test("latest review extraction exposes earlier cycles and a cycle count", () => ]); }); +test("refresh projections preserve prior review identity and history context", () => { + const previousBody = previousDurableComment({ + reviewedAt: "2026-06-20T10:00:00.000Z", + sha: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }); + const projection = renderDurableReviewRefreshProjection(previousBody, { + itemNumber: 101, + targetRevision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + startedAt: "2026-07-17T03:50:00.000Z", + leaseOwner: "github-run-123-1", + leaseCommentId: 987, + previousReviewedAt: "2026-06-20T10:00:00.000Z", + previousSha: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }); + assert.ok(projection); + const review = extractLatestClawSweeperReviewForTest( + [ + { + id: 11, + user: { login: "clawsweeper" }, + body: projection, + created_at: "2026-06-20T10:05:00Z", + updated_at: "2026-07-17T03:50:00Z", + }, + ], + 101, + ); + assert.ok(review); + assert.equal( + review.verdictDigest, + createHash("sha256").update(previousBody.trim()).digest("hex"), + ); + assert.equal(review.reviewedAt, "2026-06-20T10:00:00.000Z"); + assert.equal(review.reviewedSha, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + assert.equal(review.completedReviewCycles, 1); + assert.deepEqual(review.findings, [ + { priority: "P1", title: "Drop the stale cache before rebuild" }, + ]); +}); + test("stale durable comments expose the latest completed cycle from preserved history", () => { const ledger = renderReviewHistorySection({ cycles: [