From fa903db0a36c63bf0a17e47ec1437c5a90397239 Mon Sep 17 00:00:00 2001 From: brokemac79 Date: Wed, 24 Jun 2026 00:26:50 +0100 Subject: [PATCH 01/10] Add maintainer decision packets --- README.md | 6 + src/clawsweeper.ts | 158 +++++++- src/decision-packets.ts | 670 ++++++++++++++++++++++++++++++++++ test/clawsweeper.test.ts | 91 +++++ test/decision-packets.test.ts | 260 +++++++++++++ 5 files changed, 1168 insertions(+), 17 deletions(-) create mode 100644 src/decision-packets.ts create mode 100644 test/decision-packets.test.ts diff --git a/README.md b/README.md index 73b9a490aa..2393aaa949 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,12 @@ commenting or closing anything. Closed or already-closed reports move to `records//closed/.md`; reopened archived items move back to `items/` as stale work. +Apply and artifact replay also maintain label-derived decision packet JSON at +`records//decision-packets/.json` for reports that need a +maintainer ruling. The packet source of truth is the report frontmatter and +exact GitHub labels. Pass `--decision-packets-dir` to write those packet files +somewhere other than the profile's default records directory. + Generated state lives on the `state` branch of `openclaw/clawsweeper-state`: durable `records/`, `jobs/`, `results/`, audit output, workflow status JSON, repair ledgers, and the rendered dashboard. The state repo `main` branch is the diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index f7810d80c5..f961ef36fd 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -107,6 +107,11 @@ import { type Args, } from "./clawsweeper-args.js"; import { escapeRegExp, safeOutputTail, trimMiddle, truncateText } from "./clawsweeper-text.js"; +import { + renderDecisionPacketPublicBlock, + syncDecisionPacketRecord, + type DecisionPacketSubjectState, +} from "./decision-packets.js"; import { appendReviewHistoryCycle, neutralizeReviewControlMarkers, @@ -125,6 +130,10 @@ export { } from "./codex-env.js"; export { parseGhJson, parseGhJsonLines } from "./github-json.js"; export { itemNumbersArg } from "./clawsweeper-args.js"; +export { + buildDecisionPacketFromReport, + renderDecisionPacketPublicBlock, +} from "./decision-packets.js"; export { safeOutputTail } from "./clawsweeper-text.js"; export { ghRetryKind, @@ -2032,6 +2041,47 @@ function defaultPlansDir(profile = targetProfile()): string { return join(repoRecordsDir(profile), "plans"); } +function defaultDecisionPacketsDir(profile = targetProfile()): string { + return join(repoRecordsDir(profile), "decision-packets"); +} + +function siblingDecisionPacketsDir( + recordDir: string, + recordDirName: "items" | "closed", +): string | undefined { + return basename(recordDir) === recordDirName + ? join(dirname(recordDir), "decision-packets") + : undefined; +} + +function defaultDecisionPacketsDirForRecordDirs( + itemsDir: string, + closedDir: string, + profile = targetProfile(), +): string { + const itemsPacketsDir = siblingDecisionPacketsDir(itemsDir, "items"); + const closedPacketsDir = siblingDecisionPacketsDir(closedDir, "closed"); + if (itemsPacketsDir && (!closedPacketsDir || itemsPacketsDir === closedPacketsDir)) { + return itemsPacketsDir; + } + if (closedPacketsDir && !itemsPacketsDir) return closedPacketsDir; + return defaultDecisionPacketsDir(profile); +} + +function decisionPacketsDirFromArgs(args: Args, itemsDir: string, closedDir: string): string { + const explicitDecisionPacketsDir = stringArg(args.decision_packets_dir, ""); + if (explicitDecisionPacketsDir) return resolve(explicitDecisionPacketsDir); + if (typeof args.items_dir === "string") { + const itemsPacketsDir = siblingDecisionPacketsDir(itemsDir, "items"); + if (itemsPacketsDir) return resolve(itemsPacketsDir); + } + if (typeof args.closed_dir === "string") { + const closedPacketsDir = siblingDecisionPacketsDir(closedDir, "closed"); + if (closedPacketsDir) return resolve(closedPacketsDir); + } + return resolve(defaultDecisionPacketsDirForRecordDirs(itemsDir, closedDir)); +} + function reportFileName(repo: string, number: number): string { repositoryProfileFor(repo); return `${number}.md`; @@ -15176,6 +15226,10 @@ function renderKeepOpenCommentFromReport( isPullRequest ? "Next step before merge" : "Next step", publicNextStepLine, ); + const decisionPacketBlock = renderDecisionPacketPublicBlock(markdown); + if (decisionPacketBlock) { + appendPublicSection(lines, "Maintainer decision needed", decisionPacketBlock); + } const securityLine = publicSecurityReviewLine(securityReview); if (securityLine) appendPublicSection(lines, "Security", securityLine); if (isPullRequest && reviewFindings.length) { @@ -17729,6 +17783,7 @@ async function applyDecisionsCommand(args: Args): Promise { const itemsDir = resolve(stringArg(args.items_dir, defaultItemsDir())); const closedDir = resolve(stringArg(args.closed_dir, defaultClosedDir())); const plansDir = resolve(stringArg(args.plans_dir, defaultPlansDir())); + const decisionPacketsDir = decisionPacketsDirFromArgs(args, itemsDir, closedDir); const limit = numberArg(args.limit, 20); const processedLimit = numberArg(args.processed_limit, Math.max(limit * 2, 50)); const minAgeDays = numberArg(args.min_age_days, 0); @@ -17810,6 +17865,29 @@ async function applyDecisionsCommand(args: Args): Promise { location, ...applyQueueSortFields(entry.markdown, syncCommentsOnly, applyKind), })); + const syncDecisionPacketMarkdown = ( + reportPath: string, + nextMarkdown: string, + subjectState: DecisionPacketSubjectState = "open", + ): string => + syncDecisionPacketRecord({ + markdown: nextMarkdown, + reportPath, + packetsDir: decisionPacketsDir, + repoRoot: ROOT, + subjectState, + }).markdown; + const writeReportMarkdown = ( + reportPath: string, + nextMarkdown: string, + subjectState: DecisionPacketSubjectState = "open", + ): void => { + writeFileSync( + reportPath, + syncDecisionPacketMarkdown(reportPath, nextMarkdown, subjectState), + "utf8", + ); + }; const fileEntries = applyReportEntriesForDir(itemsDir, "items").sort( (left, right) => left.priority - right.priority || @@ -17866,17 +17944,19 @@ async function applyDecisionsCommand(args: Args): Promise { const archiveClosed = (nextMarkdown: string): void => { if (dryRun) return; ensureDir(closedDir); - writeFileSync(path, nextMarkdown, "utf8"); + const closedPath = join(closedDir, file); + const syncedMarkdown = syncDecisionPacketMarkdown(closedPath, nextMarkdown, "closed"); + writeFileSync(path, syncedMarkdown, "utf8"); syncWorkPlanFromReport({ - markdown: nextMarkdown, + markdown: syncedMarkdown, reportPath: path, plansDir, }); - renameSync(path, join(closedDir, file)); + renameSync(path, closedPath); }; const markApplyChecked = (): void => { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); }; const recordApplySkipped = (actionTaken: ActionTaken, reason: string): boolean => { markApplyChecked(); @@ -18513,7 +18593,7 @@ async function applyDecisionsCommand(args: Args): Promise { ); } markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: "skipped_changed_since_review", @@ -18665,7 +18745,7 @@ async function applyDecisionsCommand(args: Args): Promise { markdown = replaceFrontMatterValue(markdown, "action_taken", "skipped_changed_since_review"); markdown = replaceFrontMatterValue(markdown, "current_item_updated_at", item.updatedAt); markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: "skipped_changed_since_review", @@ -18686,7 +18766,7 @@ async function applyDecisionsCommand(args: Args): Promise { ); markdown = replaceFrontMatterValue(markdown, "current_item_snapshot_hash", currentHash); markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: "skipped_changed_since_review", @@ -18960,7 +19040,7 @@ async function applyDecisionsCommand(args: Args): Promise { : null; if (staleSyncReason) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: "skipped_stale_review_comment_sync", @@ -19011,7 +19091,7 @@ async function applyDecisionsCommand(args: Args): Promise { } markdown = updateReviewCommentMetadata(markdown, syncedComment, markedReviewComment); markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: proofBlockedForCommentSync?.actionTaken ?? "review_comment_synced", @@ -19026,7 +19106,7 @@ async function applyDecisionsCommand(args: Args): Promise { if (proofBlockedForCommentSync) { if (!needsReviewCommentSync) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: proofBlockedForCommentSync.actionTaken, @@ -19044,7 +19124,7 @@ async function applyDecisionsCommand(args: Args): Promise { (!isCloseProposal || syncCommentsOnly) ) { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown); results.push({ number, action: "kept_open", @@ -19880,6 +19960,7 @@ function applyArtifactsCommand(args: Args): void { const itemsDir = resolve(stringArg(args.items_dir, defaultItemsDir())); const closedDir = resolve(stringArg(args.closed_dir, defaultClosedDir())); const plansDir = resolve(stringArg(args.plans_dir, defaultPlansDir())); + const decisionPacketsDir = decisionPacketsDirFromArgs(args, itemsDir, closedDir); const skipReconcile = boolArg(args.skip_reconcile); const replayClosedArtifacts = boolArg(args.replay_closed_artifacts); const maxPages = numberArg(args.max_pages, 250); @@ -19914,12 +19995,19 @@ function applyArtifactsCommand(args: Args): void { const stalePath = join(destinationDir === itemsDir ? closedDir : itemsDir, destinationFile); if (existsSync(stalePath)) unlinkSync(stalePath); const reportPath = join(destinationDir, destinationFile); - writeFileSync(reportPath, markdown, "utf8"); + const syncedMarkdown = syncDecisionPacketRecord({ + markdown, + reportPath, + packetsDir: decisionPacketsDir, + repoRoot: ROOT, + subjectState: destination === "closed" ? "closed" : "open", + }).markdown; + writeFileSync(reportPath, syncedMarkdown, "utf8"); if (destination === "closed") { const planPath = workPlanPathForReport(reportPath, plansDir); if (existsSync(planPath)) unlinkSync(planPath); } else { - syncWorkPlanFromReport({ markdown, reportPath, plansDir }); + syncWorkPlanFromReport({ markdown: syncedMarkdown, reportPath, plansDir }); } appliedArtifacts += 1; } @@ -19927,7 +20015,7 @@ function applyArtifactsCommand(args: Args): void { console.error( `[apply-artifacts] applied=${appliedArtifacts} skipped_closed=${skippedClosedArtifacts}`, ); - if (!skipReconcile) reconcileFolders({ itemsDir, closedDir, plansDir }); + if (!skipReconcile) reconcileFolders({ itemsDir, closedDir, plansDir, decisionPacketsDir }); } function artifactTargetIsOpen(number: number, openNumbers: Set | null): boolean { @@ -20359,6 +20447,7 @@ function reconcileFolders(options: { itemsDir: string; closedDir: string; plansDir?: string; + decisionPacketsDir?: string; maxPages?: number; dryRun?: boolean; fetchClosedAt?: boolean; @@ -20368,6 +20457,20 @@ function reconcileFolders(options: { const dryRun = options.dryRun ?? false; const fetchClosedAt = options.fetchClosedAt ?? true; const plansDir = options.plansDir ?? defaultPlansDir(); + const syncReconciledDecisionPacket = ( + markdown: string, + reportPath: string, + subjectState: DecisionPacketSubjectState, + ): string => { + if (dryRun || !options.decisionPacketsDir) return markdown; + return syncDecisionPacketRecord({ + markdown, + reportPath, + packetsDir: options.decisionPacketsDir, + repoRoot: ROOT, + subjectState, + }).markdown; + }; ensureDir(options.itemsDir); ensureDir(options.closedDir); const { numbers: openNumbers, pagesScanned } = fetchOpenItemNumbers(maxPages); @@ -20401,7 +20504,11 @@ function reconcileFolders(options: { ); } } - const markdown = markReconciledState(sourceMarkdown, "closed", { closedAt }); + const markdown = syncReconciledDecisionPacket( + markReconciledState(sourceMarkdown, "closed", { closedAt }), + destinationPath, + "closed", + ); moveMarkdownFile({ sourcePath, destinationPath, markdown, dryRun }); if (!dryRun) { const planPath = workPlanPathForReport(sourcePath, plansDir); @@ -20418,11 +20525,26 @@ function reconcileFolders(options: { if (!openNumbers.has(number)) continue; const destinationPath = join(options.itemsDir, file); if (existsSync(destinationPath)) { - if (!dryRun) unlinkSync(sourcePath); + if (!dryRun) { + const destinationMarkdown = readFileSync(destinationPath, "utf8"); + const syncedDestinationMarkdown = syncReconciledDecisionPacket( + destinationMarkdown, + destinationPath, + "open", + ); + if (syncedDestinationMarkdown !== destinationMarkdown) { + writeFileSync(destinationPath, syncedDestinationMarkdown, "utf8"); + } + unlinkSync(sourcePath); + } removedStaleClosedCopies += 1; continue; } - const markdown = markReconciledState(sourceMarkdown, "open"); + const markdown = syncReconciledDecisionPacket( + markReconciledState(sourceMarkdown, "open"), + destinationPath, + "open", + ); moveMarkdownFile({ sourcePath, destinationPath, markdown, dryRun }); syncWorkPlanFromReport({ markdown, reportPath: destinationPath, plansDir, dryRun }); movedToItems += 1; @@ -20443,6 +20565,7 @@ function reconcileCommand(args: Args): void { const itemsDir = resolve(stringArg(args.items_dir, defaultItemsDir())); const closedDir = resolve(stringArg(args.closed_dir, defaultClosedDir())); const plansDir = resolve(stringArg(args.plans_dir, defaultPlansDir())); + const decisionPacketsDir = decisionPacketsDirFromArgs(args, itemsDir, closedDir); const maxPages = numberArg(args.max_pages, 250); const dryRun = boolArg(args.dry_run); const fetchClosedAt = !boolArg(args.skip_closed_at); @@ -20451,6 +20574,7 @@ function reconcileCommand(args: Args): void { itemsDir, closedDir, plansDir, + decisionPacketsDir, maxPages, dryRun, fetchClosedAt, diff --git a/src/decision-packets.ts b/src/decision-packets.ts new file mode 100644 index 0000000000..bc3659cfee --- /dev/null +++ b/src/decision-packets.ts @@ -0,0 +1,670 @@ +import { createHash } from "node:crypto"; +import { existsSync, mkdirSync, unlinkSync, writeFileSync } from "node:fs"; +import { dirname, relative } from "node:path"; + +export type DecisionPacketLane = + | "product_contract" + | "security_boundary" + | "maintainer_review" + | "release_inclusion" + | "proof_or_repro_decision"; + +export type DecisionPacketPriority = "P0" | "P1" | "P2" | "P3" | "none"; +export type DecisionPacketEvidenceStrength = "low" | "medium" | "high"; +export type DecisionPacketSubjectState = "open" | "closed" | "merged"; + +export interface DecisionPacketEvidence { + label: string; + detail: string; + url?: string; + file?: string; + line?: number; + sha?: string; +} + +export interface DecisionPacketLinkedItem { + repo: string; + kind: "issue" | "pull_request"; + number: number; + url: string; + state?: DecisionPacketSubjectState; + relationship: "fix" | "duplicate" | "dependency" | "prior_art" | "symptom" | "related"; +} + +export interface DecisionPacket { + version: 1; + generatedAt: string; + updatedAt: string; + subject: { + repo: string; + kind: "issue" | "pull_request"; + number: number; + title: string; + url: string; + state: DecisionPacketSubjectState; + labels: string[]; + createdAt?: string; + updatedAt?: string; + stateChangedAt?: string; + headSha?: string; + }; + lane: DecisionPacketLane; + priority: DecisionPacketPriority; + claim: string; + maintainerQuestion: string; + whyHuman: string; + evidenceStrength: DecisionPacketEvidenceStrength; + evidence: DecisionPacketEvidence[]; + linkedItems: DecisionPacketLinkedItem[]; + risks: string[]; + recommendedActions: string[]; + suggestedLabels: string[]; + source: { + reportPath: string; + reportUrl?: string; + reviewCommentUrl?: string; + reviewedAt?: string; + mainSha?: string; + }; +} + +export interface DecisionPacketBuildOptions { + generatedAt?: string; + reportPath?: string; + reportUrl?: string; + subjectState?: DecisionPacketSubjectState; +} + +export interface DecisionPacketSyncOptions extends DecisionPacketBuildOptions { + markdown: string; + reportPath: string; + packetsDir: string; + repoRoot: string; +} + +export interface DecisionPacketSyncResult { + markdown: string; + packet: DecisionPacket | null; + packetPath?: string; + packetSha256?: string; +} + +const DECISION_LABEL_PREFIXES = [ + "clawsweeper:", + "proof:", + "status:", + "app:", + "channel:", + "extensions:", + "impact:", + "merge-risk:", + "triage:", +] as const; +const DECISION_LABEL_MATCHES = ["release-blocker", "beta-blocker"] as const; + +export function buildDecisionPacketFromReport( + markdown: string, + options: DecisionPacketBuildOptions = {}, +): DecisionPacket | null { + const frontmatter = frontMatter(markdown); + const repo = frontmatter.repository; + const kind = frontmatter.type; + const number = numberValue(frontmatter.number); + if (!repo || (kind !== "issue" && kind !== "pull_request") || number === null) return null; + + const labels = stringArrayValue(frontmatter.labels); + const lane = packetLane(frontmatter, markdown, labels); + if (!lane) return null; + + const generatedAt = options.generatedAt ?? frontmatter.reviewed_at ?? new Date().toISOString(); + const updatedAt = frontmatter.reviewed_at ?? generatedAt; + const subjectUpdatedAt = + knownValue(frontmatter.current_item_updated_at) ?? knownValue(frontmatter.item_updated_at); + const title = frontmatter.title ?? reportHeadingTitle(markdown) ?? `#${number}`; + const url = + frontmatter.url ?? + `https://github.com/${repo}/${kind === "pull_request" ? "pull" : "issues"}/${number}`; + const reviewCommentUrl = knownValue(frontmatter.review_comment_url); + const mainSha = knownValue(frontmatter.main_sha); + const headSha = knownValue(frontmatter.pull_head_sha); + const claim = firstSentence(sectionValue(markdown, "Summary")) || title; + const recommendedActions = recommendedActionsFor(lane, markdown, frontmatter); + const source: DecisionPacket["source"] = { + reportPath: options.reportPath ?? "", + ...(options.reportUrl ? { reportUrl: options.reportUrl } : {}), + ...(reviewCommentUrl ? { reviewCommentUrl } : {}), + ...(frontmatter.reviewed_at ? { reviewedAt: frontmatter.reviewed_at } : {}), + ...(mainSha ? { mainSha } : {}), + }; + + return { + version: 1, + generatedAt, + updatedAt, + subject: { + repo, + kind, + number, + title, + url, + state: options.subjectState ?? stateFromReport(frontmatter), + labels, + ...(frontmatter.item_created_at ? { createdAt: frontmatter.item_created_at } : {}), + ...(subjectUpdatedAt ? { updatedAt: subjectUpdatedAt } : {}), + ...(frontmatter.current_item_closed_at + ? { stateChangedAt: frontmatter.current_item_closed_at } + : {}), + ...(headSha ? { headSha } : {}), + }, + lane, + priority: priorityFrom(frontmatter, labels), + claim, + maintainerQuestion: maintainerQuestionFor(lane, frontmatter), + whyHuman: whyHumanFor(lane), + evidenceStrength: evidenceStrengthFrom(frontmatter), + evidence: packetEvidence(markdown, frontmatter, labels), + linkedItems: linkedItemsFrom(frontmatter, markdown), + risks: risksFrom(markdown, frontmatter), + recommendedActions, + suggestedLabels: suggestedLabels(labels), + source, + }; +} + +export function renderDecisionPacketPublicBlock(markdown: string): string { + const packet = buildDecisionPacketFromReport(markdown); + if (!packet) return ""; + const action = packet.recommendedActions[0] ?? packet.maintainerQuestion; + return [ + `- Lane: ${publicLaneName(packet.lane)}.`, + `- Question: ${packet.maintainerQuestion}`, + `- Evidence strength: ${packet.evidenceStrength}.`, + `- Recommended action: ${action}`, + ].join("\n"); +} + +export function syncDecisionPacketRecord( + options: DecisionPacketSyncOptions, +): DecisionPacketSyncResult { + const buildOptions: DecisionPacketBuildOptions = { + reportPath: repoRelativePath(options.repoRoot, options.reportPath), + ...(options.generatedAt ? { generatedAt: options.generatedAt } : {}), + ...(options.reportUrl ? { reportUrl: options.reportUrl } : {}), + ...(options.subjectState ? { subjectState: options.subjectState } : {}), + }; + const packet = buildDecisionPacketFromReport(options.markdown, buildOptions); + const frontmatter = frontMatter(options.markdown); + const number = numberValue(frontmatter.number); + const packetPath = number === null ? undefined : `${options.packetsDir}/${number}.json`; + if (!packet || !packetPath) { + if (packetPath && existsSync(packetPath)) unlinkSync(packetPath); + return { + markdown: replacePacketFrontmatter(options.markdown, "none", "none"), + packet: null, + ...(packetPath ? { packetPath } : {}), + }; + } + + mkdirSync(dirname(packetPath), { recursive: true }); + const json = `${JSON.stringify(packet, null, 2)}\n`; + writeFileSync(packetPath, json, "utf8"); + const packetSha256 = sha256(json); + const markdown = replacePacketFrontmatter( + options.markdown, + repoRelativePath(options.repoRoot, packetPath), + packetSha256, + ); + return { markdown, packet, packetPath, packetSha256 }; +} + +function packetLane( + frontmatter: Record, + markdown: string, + labels: readonly string[], +): DecisionPacketLane | null { + const normalizedLabels = labels.map((label) => label.toLowerCase()); + const securityStatus = sectionLineValue(sectionValue(markdown, "Security Review"), "Status"); + if ( + frontmatter.item_category === "security" || + securityStatus === "needs_attention" || + normalizedLabels.includes("clawsweeper:needs-security-review") || + normalizedLabels.some((label) => label.startsWith("merge-risk:") && label.includes("security")) + ) { + return "security_boundary"; + } + if ( + frontmatter.requires_product_decision === "true" || + normalizedLabels.includes("clawsweeper:needs-product-decision") + ) { + return "product_contract"; + } + if ( + normalizedLabels.includes("clawsweeper:needs-live-repro") || + normalizedLabels.includes("triage: needs-real-behavior-proof") || + normalizedLabels.some( + (label) => label.startsWith("status:") && label.includes("needs proof"), + ) || + frontmatter.real_behavior_proof_needs_contributor_action === "true" + ) { + return "proof_or_repro_decision"; + } + if ( + frontmatter.work_status === "manual_review" || + frontmatter.work_candidate === "manual_review" || + normalizedLabels.includes("clawsweeper:needs-maintainer-review") + ) { + return "maintainer_review"; + } + if ( + normalizedLabels.some( + (label) => label.includes("release-blocker") || label.includes("beta-blocker"), + ) + ) { + return "release_inclusion"; + } + return null; +} + +function stateFromReport(frontmatter: Record): DecisionPacketSubjectState { + if ( + frontmatter.current_state === "open" || + frontmatter.current_state === "closed" || + frontmatter.current_state === "merged" + ) { + return frontmatter.current_state; + } + if ( + frontmatter.action_taken === "closed" || + frontmatter.action_taken === "skipped_already_closed" + ) { + return "closed"; + } + return "open"; +} + +function priorityFrom( + frontmatter: Record, + labels: readonly string[], +): DecisionPacketPriority { + if (isPriority(frontmatter.triage_priority)) return frontmatter.triage_priority; + const labelPriority = labels.find(isPriority); + return labelPriority ?? "none"; +} + +function evidenceStrengthFrom(frontmatter: Record): DecisionPacketEvidenceStrength { + const confidence = frontmatter.confidence; + return confidence === "high" || confidence === "medium" || confidence === "low" + ? confidence + : "medium"; +} + +function maintainerQuestionFor( + lane: DecisionPacketLane, + frontmatter: Record, +): string { + if (lane === "security_boundary") { + return "Should this security or trust-boundary change be accepted before merge?"; + } + if (lane === "product_contract") { + return "Should this product/API contract direction be accepted?"; + } + if (lane === "proof_or_repro_decision") { + return "Is the available proof or reproduction enough for maintainer action?"; + } + if (lane === "release_inclusion") { + return "Should this item block or be included in the release?"; + } + if (frontmatter.work_reason) return firstSentence(frontmatter.work_reason); + return "What maintainer action should happen next for this item?"; +} + +function whyHumanFor(lane: DecisionPacketLane): string { + switch (lane) { + case "security_boundary": + return "This affects a trust boundary or security-sensitive behavior that should not be decided by automation."; + case "product_contract": + return "This changes product/API direction and needs a maintainer ruling before automation can act safely."; + case "proof_or_repro_decision": + return "Automation can collect proof, but a maintainer must decide whether the proof is sufficient."; + case "release_inclusion": + return "Release inclusion and blocking decisions need maintainer judgment."; + case "maintainer_review": + return "ClawSweeper classified this as manual-review work."; + } +} + +function recommendedActionsFor( + lane: DecisionPacketLane, + markdown: string, + frontmatter: Record, +): string[] { + const candidates = [ + sectionValue(markdown, "Best Possible Solution"), + sectionValue(markdown, "Best Solution"), + sectionValue(markdown, "Repair Work Prompt"), + sectionLineValue(sectionValue(markdown, "Work Candidate"), "Reason"), + ] + .map(firstSentence) + .filter(Boolean); + if (candidates.length > 0) return uniqueStrings(candidates).slice(0, 3); + if (frontmatter.work_status === "manual_review") + return ["Assign a maintainer to review the item."]; + if (lane === "security_boundary") + return ["Ask the security or owning maintainer to rule before merge."]; + if (lane === "product_contract") + return ["Ask the owning maintainer to rule on the product/API contract."]; + if (lane === "proof_or_repro_decision") + return ["Ask a maintainer whether the proof/reproduction is sufficient."]; + return ["Ask a maintainer for the next decision."]; +} + +function packetEvidence( + markdown: string, + frontmatter: Record, + labels: readonly string[], +): DecisionPacketEvidence[] { + const evidence: DecisionPacketEvidence[] = []; + addEvidence(evidence, "Summary", sectionValue(markdown, "Summary")); + addEvidence(evidence, "Security review", compactLines(sectionValue(markdown, "Security Review"))); + addEvidence( + evidence, + "Real behavior proof", + compactLines(sectionValue(markdown, "Real Behavior Proof")), + ); + addEvidence(evidence, "Work candidate", compactLines(sectionValue(markdown, "Work Candidate"))); + const labelEvidence = suggestedLabels(labels).join(", "); + addEvidence(evidence, "Decision labels", labelEvidence); + if (frontmatter.review_comment_url && frontmatter.review_comment_url !== "unknown") { + evidence.push({ + label: "Durable review comment", + detail: "ClawSweeper durable review comment is available.", + url: frontmatter.review_comment_url, + }); + } + return evidence.slice(0, 8); +} + +function risksFrom(markdown: string, frontmatter: Record): string[] { + const risks = bulletLines( + sectionValue(markdown, "Risks / Open Questions") || sectionValue(markdown, "Risks"), + ); + const mergeRiskLabels = stringArrayValue(frontmatter.merge_risk_labels); + return uniqueStrings([...risks, ...mergeRiskLabels]).slice(0, 8); +} + +function linkedItemsFrom( + frontmatter: Record, + markdown: string, +): DecisionPacketLinkedItem[] { + const defaultRepo = subjectRepoFrom(frontmatter); + const refs = [ + ...stringArrayValue(frontmatter.work_cluster_refs), + ...rootCauseMemberRefs(frontmatter.root_cause_cluster), + ...textRefs(sectionValue(markdown, "Root-Cause Cluster"), defaultRepo), + ]; + const items: DecisionPacketLinkedItem[] = []; + const seen = new Set(); + for (const ref of refs) { + const parsed = parseRepoRef(ref, defaultRepo); + if (!parsed) continue; + const key = `${parsed.repo}:${parsed.kind}:${parsed.number}`; + if (seen.has(key)) continue; + seen.add(key); + items.push({ + repo: parsed.repo, + kind: parsed.kind, + number: parsed.number, + url: `https://github.com/${parsed.repo}/${parsed.kind === "pull_request" ? "pull" : "issues"}/${parsed.number}`, + relationship: "related", + }); + } + return items.slice(0, 12); +} + +function subjectRepoFrom(frontmatter: Record): string | undefined { + if (frontmatter.repository) return frontmatter.repository; + return frontmatter.url?.match(/\bgithub\.com\/([a-z0-9_.-]+\/[a-z0-9_.-]+)\b/i)?.[1]; +} + +function suggestedLabels(labels: readonly string[]): string[] { + return labels + .filter(isDecisionLabel) + .concat(labels.filter(isPriority)) + .filter((label, index, array) => array.indexOf(label) === index); +} + +function isDecisionLabel(label: string): boolean { + const normalized = label.toLowerCase(); + return ( + DECISION_LABEL_PREFIXES.some((prefix) => normalized.startsWith(prefix)) || + DECISION_LABEL_MATCHES.some((match) => normalized.includes(match)) + ); +} + +function frontMatter(markdown: string): Record { + const match = markdown.match(/^---\r?\n([\s\S]*?)\r?\n---/); + const block = match?.[1] ?? ""; + const values: Record = {}; + for (const line of block.split(/\r?\n/)) { + const separator = line.indexOf(":"); + if (separator <= 0) continue; + const key = line.slice(0, separator).trim(); + const value = line.slice(separator + 1).trim(); + values[key] = unquote(value); + } + return values; +} + +function replacePacketFrontmatter( + markdown: string, + packetPath: string, + packetSha256: string, +): string { + let next = replaceFrontMatterValue(markdown, "decision_packet_path", packetPath); + next = replaceFrontMatterValue(next, "decision_packet_sha256", packetSha256); + return next; +} + +function replaceFrontMatterValue(markdown: string, key: string, value: string): string { + const line = `${key}: ${value}`; + const pattern = new RegExp(`^${escapeRegExp(key)}:\\s*.*$`, "m"); + if (pattern.test(markdown)) return markdown.replace(pattern, line); + return markdown.replace(/^---\r?\n/, `---\n${line}\n`); +} + +function sectionValue(markdown: string, heading: string): string { + const normalized = markdown.replace(/\r\n?/g, "\n"); + const match = normalized.match( + new RegExp(`(?:^|\\n)## ${escapeRegExp(heading)}\\n\\n([\\s\\S]*?)(?=\\n## |\\n?$)`), + ); + return match?.[1]?.trim() ?? ""; +} + +function sectionLineValue(section: string, label: string): string { + const pattern = new RegExp(`^${escapeRegExp(label)}:\\s*(.+)$`, "im"); + return pattern.exec(section)?.[1]?.trim() ?? ""; +} + +function stringArrayValue(value: string | undefined): string[] { + if (!value || value === "unknown" || value === "none") return []; + try { + const parsed: unknown = JSON.parse(value); + if (Array.isArray(parsed)) { + return parsed.filter((item): item is string => typeof item === "string"); + } + } catch { + // Older reports used plain comma-separated labels. + } + return value + .split(",") + .map((item) => item.trim()) + .filter(Boolean); +} + +function numberValue(value: string | undefined): number | null { + const number = Number(value); + return Number.isInteger(number) && number > 0 ? number : null; +} + +function knownValue(value: string | undefined): string | undefined { + return value && value !== "unknown" && value !== "none" ? value : undefined; +} + +function firstSentence(value: string | undefined): string { + const normalized = compactLines(value ?? ""); + if (!normalized || normalized === "- none" || normalized === "_Not provided._") return ""; + const sentence = normalized.match(/^(.+?[.!?])(?:\s|$)/)?.[1]; + return (sentence ?? normalized).trim(); +} + +function compactLines(value: string): string { + return value + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean) + .join(" "); +} + +function addEvidence(items: DecisionPacketEvidence[], label: string, detail: string): void { + const text = firstSentence(detail); + if (text) items.push({ label, detail: text }); +} + +function bulletLines(value: string): string[] { + return value + .split(/\r?\n/) + .map((line) => line.trim().replace(/^-\s+/, "")) + .filter((line) => line && line !== "none"); +} + +function rootCauseMemberRefs(value: string | undefined): string[] { + if (!value || value === "unknown") return []; + try { + const parsed: unknown = JSON.parse(value); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return []; + const members = (parsed as { members?: unknown }).members; + if (!Array.isArray(members)) return []; + return members + .map((member) => + member && typeof member === "object" && "ref" in member + ? (member as { ref?: unknown }).ref + : undefined, + ) + .filter((ref): ref is string => typeof ref === "string"); + } catch { + return []; + } +} + +function textRefs(value: string, defaultRepo: string | undefined): string[] { + const refs = [ + ...[ + ...value.matchAll( + /\bhttps?:\/\/github\.com\/[a-z0-9_.-]+\/[a-z0-9_.-]+\/(?:issues|pull)\/\d+\b/gi, + ), + ].map((match) => match[0]), + ...[...value.matchAll(/\b([a-z0-9_.-]+\/[a-z0-9_.-]+)#(\d+)\b/gi)].map( + (match) => `${match[1]}#${match[2]}`, + ), + ]; + if (defaultRepo) { + refs.push( + ...[...value.matchAll(/(?:^|[^\w/])#(\d+)\b/g)].map((match) => `${defaultRepo}#${match[1]}`), + ); + } + return uniqueStrings(refs); +} + +function parseRepoRef( + value: string, + defaultRepo?: string, +): { repo: string; kind: "issue" | "pull_request"; number: number } | null { + const urlMatch = value.match( + /\bhttps?:\/\/github\.com\/([a-z0-9_.-]+\/[a-z0-9_.-]+)\/(issues|pull)\/(\d+)\b/i, + ); + if (urlMatch?.[1] && urlMatch[2] && urlMatch[3]) { + return { + repo: urlMatch[1], + kind: urlMatch[2] === "pull" ? "pull_request" : "issue", + number: Number(urlMatch[3]), + }; + } + const match = value.match(/\b([a-z0-9_.-]+\/[a-z0-9_.-]+)#(\d+)\b/i); + if (match?.[1] && match[2]) return { repo: match[1], kind: "issue", number: Number(match[2]) }; + const shorthandMatch = value.match(/(?:^|[^\w/])#(\d+)\b/); + if (defaultRepo && shorthandMatch?.[1]) { + return { repo: defaultRepo, kind: "issue", number: Number(shorthandMatch[1]) }; + } + return null; +} + +function reportHeadingTitle(markdown: string): string | undefined { + for (const rawLine of markdown.split("\n")) { + const line = rawLine.endsWith("\r") ? rawLine.slice(0, -1) : rawLine; + if (!line.startsWith("#")) continue; + const afterMarker = line.slice(1); + if (!afterMarker.startsWith(" ") && !afterMarker.startsWith("\t")) continue; + let heading = afterMarker.trimStart(); + if (heading.startsWith("#")) { + const colonIndex = heading.indexOf(":"); + const issueNumber = colonIndex === -1 ? "" : heading.slice(1, colonIndex); + if (issueNumber && isDecimalDigits(issueNumber)) + heading = heading.slice(colonIndex + 1).trimStart(); + } + const trimmed = heading.trim(); + if (trimmed) return trimmed; + } + return undefined; +} + +function isDecimalDigits(value: string): boolean { + for (const char of value) if (char < "0" || char > "9") return false; + return value.length > 0; +} + +function publicLaneName(lane: DecisionPacketLane): string { + switch (lane) { + case "product_contract": + return "Product/API contract"; + case "security_boundary": + return "Security boundary"; + case "maintainer_review": + return "Maintainer review"; + case "release_inclusion": + return "Release inclusion"; + case "proof_or_repro_decision": + return "Proof/repro decision"; + } +} + +function repoRelativePath(repoRoot: string, path: string): string { + return relative(repoRoot, path).replace(/\\/g, "/"); +} + +function isPriority(value: string | undefined): value is DecisionPacketPriority { + return value === "P0" || value === "P1" || value === "P2" || value === "P3"; +} + +function uniqueStrings(values: readonly string[]): string[] { + return [...new Set(values.filter(Boolean))]; +} + +function unquote(value: string): string { + if (value.startsWith('"') && value.endsWith('"')) { + try { + const parsed: unknown = JSON.parse(value); + if (typeof parsed === "string") return parsed; + } catch { + return value.slice(1, -1); + } + } + return value; +} + +function sha256(value: string): string { + return createHash("sha256").update(value).digest("hex"); +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index 19b1d0794b..587d616424 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -24,6 +24,7 @@ import { parseDecision, relatedGitHubIssueSearchQueryForTest, relatedTitleSearchTerms, + renderReviewCommentFromReport, renderReviewStartStatusComment, reviewArtifactDestination, reviewCodexForcedLoginMethodForTest, @@ -50,6 +51,22 @@ import { workPlanCandidateReport, } from "./helpers.ts"; +test("review comments include a compact maintainer decision packet block", () => { + const comment = renderReviewCommentFromReport( + workPlanCandidateReport({ + decision: "keep_open", + action_taken: "kept_open", + labels: JSON.stringify(["clawsweeper:needs-product-decision"]), + requires_product_decision: "true", + }), + "none", + ); + + assert.match(comment, /\*\*Maintainer decision needed\*\*/); + assert.match(comment, /Lane: Product\/API contract\./); + assert.match(comment, /Should this product\/API contract direction be accepted\?/); +}); + test("apply-decisions archives live-closed skipped records without reopening close gates", () => { const root = mkdtempSync(tmpPrefix); try { @@ -119,6 +136,80 @@ if (args[0] === "api" && /\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { } }); +test("apply-decisions writes decision packets for changed-since-review reports", () => { + const root = mkdtempSync(tmpPrefix); + try { + const itemsDir = join(root, "items"); + const closedDir = join(root, "closed"); + const plansDir = join(root, "plans"); + const reportPath = join(root, "apply-report.json"); + mkdirSync(itemsDir, { recursive: true }); + mkdirSync(plansDir, { recursive: true }); + writeFileSync( + join(itemsDir, "321.md"), + implementedCloseReport({ + labels: JSON.stringify(["clawsweeper:needs-product-decision"]), + requires_product_decision: "true", + item_snapshot_hash: "reviewed-snapshot-321", + item_updated_at: "2026-05-01T00:00:00Z", + }), + "utf8", + ); + + const ghMock = ` +const path = process.argv.includes("-i") + ? process.argv[process.argv.indexOf("-i") + 1] + : process.argv[3] || ""; +if (/\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { + console.log(JSON.stringify([[]])); +} else if (/\\/issues\\/321$/.test(path)) { + console.log(JSON.stringify({ + number: 321, + title: "Render work plans", + html_url: "https://github.com/openclaw/clawsweeper/issues/321", + created_at: "2026-05-01T00:00:00Z", + updated_at: "2026-05-02T00:00:00Z", + closed_at: null, + state: "open", + locked: false, + active_lock_reason: null, + author_association: "CONTRIBUTOR", + user: { login: "reporter" }, + labels: ["clawsweeper:needs-product-decision"], + comments: 0, + pull_request: null + })); +} else if (/\\/issues\\/321\\/timeline/.test(path)) { + console.log(JSON.stringify([[]])); +} else if (process.argv[2] === "issue" && process.argv[3] === "view") { + console.log(JSON.stringify({ closedByPullRequestsReferences: [] })); +} else if (process.argv[2] === "label" || process.argv[2] === "issue") { + console.log(""); +} else { + console.error("unexpected gh args", JSON.stringify(process.argv.slice(2))); + process.exit(1); +} +`; + withMockGh(root, ghMock, () => { + runApplyDecisionsForTest({ itemsDir, closedDir, plansDir, reportPath }); + }); + + assert.deepEqual(JSON.parse(readFileSync(reportPath, "utf8")), [ + { + number: 321, + action: "skipped_changed_since_review", + reason: "updated_at changed", + }, + ]); + assert.equal(existsSync(join(root, "decision-packets", "321.json")), true); + const updatedReport = readFileSync(join(itemsDir, "321.md"), "utf8"); + assert.match(updatedReport, /^decision_packet_path: .*decision-packets\/321\.json$/m); + assert.match(updatedReport, /^decision_packet_sha256: [a-f0-9]{64}$/m); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("apply-decisions skips advisory labels for failed or stale kept-open reports", () => { const root = mkdtempSync(tmpPrefix); try { diff --git a/test/decision-packets.test.ts b/test/decision-packets.test.ts new file mode 100644 index 0000000000..72ce635d3a --- /dev/null +++ b/test/decision-packets.test.ts @@ -0,0 +1,260 @@ +import assert from "node:assert/strict"; +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import test from "node:test"; + +import { + buildDecisionPacketFromReport, + syncDecisionPacketRecord, +} from "../dist/decision-packets.js"; +import { tmpPrefix } from "./helpers.ts"; + +test("decision packets derive product decision data from report labels and frontmatter", () => { + const report = `${workPlanCandidateReport({ + number: 81234, + repository: "openclaw/openclaw", + type: "pull_request", + title: "config.patch redacted array write", + url: "https://github.com/openclaw/openclaw/pull/81234", + labels: JSON.stringify(["clawsweeper:needs-product-decision", "app: web-ui", "P1"]), + requires_product_decision: "true", + confidence: "high", + item_updated_at: "2026-06-20T00:00:00Z", + current_item_updated_at: "2026-06-23T01:00:00Z", + pull_head_sha: "abc123", + main_sha: "main456", + review_comment_url: "https://github.com/openclaw/openclaw/pull/81234#issuecomment-99", + work_cluster_refs: JSON.stringify([ + "https://github.com/openclaw/openclaw/pull/81111", + "#81112", + "openclaw/clawhub#44", + ]), + root_cause_cluster: JSON.stringify({ + members: [{ ref: "https://github.com/openclaw/openclaw/issues/81113" }], + }), + })} + +## Best Possible Solution + +Ask the owning product maintainer to decide whether redacted full-array writes are valid. + +## Risks / Open Questions + +- The current implementation may overwrite redacted array entries. +`; + + const packet = buildDecisionPacketFromReport(report, { + generatedAt: "2026-06-23T12:00:00.000Z", + reportPath: "records/openclaw-openclaw/items/81234.md", + }); + + assert.ok(packet); + assert.equal(packet.lane, "product_contract"); + assert.equal(packet.priority, "P1"); + assert.equal(packet.evidenceStrength, "high"); + assert.equal(packet.subject.repo, "openclaw/openclaw"); + assert.equal(packet.subject.kind, "pull_request"); + assert.equal(packet.subject.headSha, "abc123"); + assert.equal(packet.subject.updatedAt, "2026-06-23T01:00:00Z"); + assert.deepEqual(packet.subject.labels, [ + "clawsweeper:needs-product-decision", + "app: web-ui", + "P1", + ]); + assert.deepEqual(packet.suggestedLabels, [ + "clawsweeper:needs-product-decision", + "app: web-ui", + "P1", + ]); + assert.equal( + packet.recommendedActions[0], + "Ask the owning product maintainer to decide whether redacted full-array writes are valid.", + ); + assert.deepEqual(packet.risks, [ + "The current implementation may overwrite redacted array entries.", + ]); + assert.deepEqual( + packet.linkedItems.map((item) => ({ + repo: item.repo, + kind: item.kind, + number: item.number, + })), + [ + { repo: "openclaw/openclaw", kind: "pull_request", number: 81111 }, + { repo: "openclaw/openclaw", kind: "issue", number: 81112 }, + { repo: "openclaw/clawhub", kind: "issue", number: 44 }, + { repo: "openclaw/openclaw", kind: "issue", number: 81113 }, + ], + ); + assert.equal( + packet.linkedItems.every((item) => !("state" in item)), + true, + ); + assert.equal("areaLabel" in packet, false); +}); + +test("decision packets preserve legacy comma-separated labels", () => { + const packet = buildDecisionPacketFromReport( + workPlanCandidateReport({ + number: 81235, + repository: "openclaw/openclaw", + labels: "clawsweeper:needs-product-decision, status: needs proof, P2", + }), + { + generatedAt: "2026-06-23T12:00:00.000Z", + reportPath: "records/openclaw-openclaw/items/81235.md", + }, + ); + + assert.ok(packet); + assert.equal(packet.lane, "product_contract"); + assert.deepEqual(packet.subject.labels, [ + "clawsweeper:needs-product-decision", + "status: needs proof", + "P2", + ]); +}); + +test("decision packets prefer reconciled current state", () => { + const packet = buildDecisionPacketFromReport( + workPlanCandidateReport({ + number: 81236, + repository: "openclaw/openclaw", + labels: "clawsweeper:needs-product-decision", + action_taken: "kept_open", + current_state: "closed", + }), + { + generatedAt: "2026-06-23T12:00:00.000Z", + reportPath: "records/openclaw-openclaw/closed/81236.md", + }, + ); + + assert.ok(packet); + assert.equal(packet.subject.state, "closed"); +}); + +test("decision packets read CRLF report sections", () => { + const report = `${workPlanCandidateReport({ + number: 81237, + repository: "openclaw/openclaw", + labels: JSON.stringify([]), + })} + +## Security Review + +Status: needs_attention +Concern: Requires maintainer security review. +`.replace(/\n/g, "\r\n"); + const packet = buildDecisionPacketFromReport(report, { + generatedAt: "2026-06-23T12:00:00.000Z", + reportPath: "records/openclaw-openclaw/items/81237.md", + }); + + assert.ok(packet); + assert.equal(packet.lane, "security_boundary"); +}); + +test("decision packets surface proof and release trigger labels", () => { + const packet = buildDecisionPacketFromReport( + workPlanCandidateReport({ + number: 81238, + repository: "openclaw/openclaw", + labels: "release-blocker, beta-blocker, triage: needs-real-behavior-proof", + }), + { + generatedAt: "2026-06-23T12:00:00.000Z", + reportPath: "records/openclaw-openclaw/items/81238.md", + }, + ); + + assert.ok(packet); + assert.equal(packet.lane, "proof_or_repro_decision"); + assert.deepEqual(packet.suggestedLabels, [ + "release-blocker", + "beta-blocker", + "triage: needs-real-behavior-proof", + ]); + assert.match( + packet.evidence.find((entry) => entry.label === "Decision labels")?.detail ?? "", + /triage: needs-real-behavior-proof/, + ); +}); + +test("decision packet sync writes packet JSON and frontmatter pointers", () => { + const root = mkdtempSync(tmpPrefix); + try { + const packetsDir = join(root, "records", "openclaw-openclaw", "decision-packets"); + const reportPath = join(root, "records", "openclaw-openclaw", "items", "321.md"); + const markdown = workPlanCandidateReport({ + repository: "openclaw/openclaw", + labels: JSON.stringify(["clawsweeper:needs-maintainer-review", "channel: telegram"]), + work_status: "manual_review", + confidence: "medium", + }); + + const result = syncDecisionPacketRecord({ + markdown, + reportPath, + packetsDir, + repoRoot: root, + generatedAt: "2026-06-23T12:00:00.000Z", + subjectState: "open", + }); + + assert.ok(result.packet); + assert.ok(result.packetPath); + assert.ok(existsSync(result.packetPath)); + assert.match( + result.markdown, + /^decision_packet_path: records\/openclaw-openclaw\/decision-packets\/321\.json$/m, + ); + assert.match(result.markdown, /^decision_packet_sha256: [a-f0-9]{64}$/m); + const packet = JSON.parse(readFileSync(result.packetPath, "utf8")); + assert.equal(packet.lane, "maintainer_review"); + assert.deepEqual(packet.subject.labels, [ + "clawsweeper:needs-maintainer-review", + "channel: telegram", + ]); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +function workPlanCandidateReport(overrides = {}) { + const frontmatter = { + number: 321, + repository: "openclaw/clawsweeper", + type: "issue", + title: "Render work plans", + reviewed_at: new Date().toISOString(), + review_status: "complete", + local_checkout_access: "verified", + decision: "keep_open", + action_taken: "kept_open", + work_candidate: "queue_fix_pr", + work_status: "candidate", + work_priority: "medium", + work_confidence: "high", + work_likely_files: JSON.stringify(["src/clawsweeper.ts", "test/clawsweeper.test.ts"]), + work_validation: JSON.stringify(["pnpm run check"]), + work_cluster_refs: JSON.stringify(["openclaw/clawsweeper#26"]), + ...overrides, + }; + return `--- +${Object.entries(frontmatter) + .map(([key, value]) => `${key}: ${value}`) + .join("\n")} +--- + +# #321: Render work plans + +## Summary + +The dashboard has queue_fix_pr candidates but no generated coding plan. + +## Repair Work Prompt + +Render generated plan markdown from existing report fields. +`; +} From 8928be1c53a4c53cfa230ef82cceb769b256f2d6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 20:17:42 +0100 Subject: [PATCH 02/10] feat(review): make decision packets Codex-authored --- CHANGELOG.md | 1 + README.md | 11 +- prompts/review-item.md | 12 + schema/clawsweeper-decision.schema.json | 76 +++ src/clawsweeper.ts | 63 +++ src/decision-packets.ts | 712 ++++++++---------------- test/clawsweeper.test.ts | 31 +- test/decision-packets.test.ts | 296 ++++------ test/decision-parser.test.ts | 43 ++ test/helpers.ts | 8 + 10 files changed, 586 insertions(+), 667 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f161f9b255..686d05aeb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ checkpoint, and status-only commits are intentionally omitted. ### Added - Added conservative, add-only `good first issue` labeling for unlocked, small, current-main reproduced bugs with a high-confidence repair prompt and validation steps and no linked-PR, feature, config, product, security, protected-label, or maintainer-opt-out blocker. +- Added durable maintainer decision packets whose exact question, rationale, options, recommendation, and likely owner come from Codex structured review output while deterministic code only validates and persists the result. Thanks @brokemac79. - Added close-candidate quality telemetry to apply status while keeping reporting separate from close eligibility and comment-only sync. Thanks @brokemac79. - Added the PR-only `stalled_unproven_pr` close reason: external D/F-rated pull requests whose requested real-behavior proof stayed missing, mock-only, or insufficient can close after 14 idle days, guarded by live checks that the proof request itself was visible for 14 days plus proof-label, draft, head-commit, and human-engagement gates. - Added the PR-only `abandoned_pr` close reason: external pull requests idle for 30 days that are still drafts, waiting on their author, or failing checks on the live head can close, while high-quality proven work stays open for repair/adopt paths. See `docs/stalled-pr-close-policies.md`. diff --git a/README.md b/README.md index 2393aaa949..964677880a 100644 --- a/README.md +++ b/README.md @@ -90,11 +90,14 @@ commenting or closing anything. Closed or already-closed reports move to `records//closed/.md`; reopened archived items move back to `items/` as stale work. -Apply and artifact replay also maintain label-derived decision packet JSON at +Apply and artifact replay also maintain Codex-authored decision packet JSON at `records//decision-packets/.json` for reports that need a -maintainer ruling. The packet source of truth is the report frontmatter and -exact GitHub labels. Pass `--decision-packets-dir` to write those packet files -somewhere other than the profile's default records directory. +maintainer ruling. Codex supplies the exact question, rationale, options, +recommendation, and likely owner as structured review output. Deterministic +code validates that intent, persists it, refreshes item state, and removes stale +packets; labels and report prose do not reconstruct the decision. Pass +`--decision-packets-dir` to write those packet files somewhere other than the +profile's default records directory. Generated state lives on the `state` branch of `openclaw/clawsweeper-state`: durable `records/`, `jobs/`, `results/`, audit output, workflow status JSON, diff --git a/prompts/review-item.md b/prompts/review-item.md index 352f5126c0..c1eea8d5be 100644 --- a/prompts/review-item.md +++ b/prompts/review-item.md @@ -85,6 +85,18 @@ one short sentence for `changeSummary`, `workReason`, `bestSolution`, and `changeSummary` or `workReason` into an automerge/autofix status update; merge automation is reported by the command/status comment and hidden markers. +Put maintainer-intent reasoning in `maintainerDecision`; do not expect labels, +report prose, or deterministic code to reconstruct it later. Set `required: +true` only when automation should pause for a real human choice. State the +exact item-specific question, why maintainer intent is required, one to three +concrete options, exactly one recommended option, and the most likely decision +owner. `likelyOwner.person` must exactly match a person in `likelyOwners`. +Choose the recommendation from the evidence even when the final authority stays +human. Use `kind: "none"`, empty question/rationale, `options: []`, and an empty +owner with low confidence when no maintainer decision is required. Do not use a +generic question such as “What should happen next?” and do not create a packet +for routine contributor follow-up that the review already specifies. + For PRs, do not let labels be the only place that merger risk is visible. If a merge can intentionally make an existing user's setup stop working, fail closed, lose a fallback path, require a migration, or require operator action, state that diff --git a/schema/clawsweeper-decision.schema.json b/schema/clawsweeper-decision.schema.json index 3b2fdea3a9..0c474e6d98 100644 --- a/schema/clawsweeper-decision.schema.json +++ b/schema/clawsweeper-decision.schema.json @@ -12,6 +12,7 @@ "likelyOwners", "risks", "bestSolution", + "maintainerDecision", "triagePriority", "impactLabels", "mergeRiskLabels", @@ -172,6 +173,81 @@ "type": "string", "description": "The best possible product/code/docs direction for this item, whether the item should close or stay open. Do not repeat the change summary, workReason, or risk list." }, + "maintainerDecision": { + "type": "object", + "additionalProperties": false, + "required": ["required", "kind", "question", "rationale", "options", "likelyOwner"], + "properties": { + "required": { + "type": "boolean", + "description": "True only when automation should pause for a maintainer choice that Codex can state precisely." + }, + "kind": { + "type": "string", + "enum": [ + "none", + "product_direction", + "security_boundary", + "proof_sufficiency", + "release_inclusion", + "merge_risk", + "manual_review" + ], + "description": "The human decision category. Use none when required is false." + }, + "question": { + "type": "string", + "description": "The exact item-specific question the maintainer should answer, or an empty string when required is false." + }, + "rationale": { + "type": "string", + "description": "Why model judgment cannot safely resolve this choice without maintainer intent, or an empty string when required is false." + }, + "options": { + "type": "array", + "maxItems": 3, + "description": "One to three item-specific choices when required is true; [] otherwise. Exactly one choice must be recommended when required is true.", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["title", "body", "recommended"], + "properties": { + "title": { + "type": "string", + "description": "Short decision option title." + }, + "body": { + "type": "string", + "description": "One concrete sentence explaining the outcome or tradeoff." + }, + "recommended": { + "type": "boolean", + "description": "True for exactly one option when a decision is required." + } + } + } + }, + "likelyOwner": { + "type": "object", + "additionalProperties": false, + "required": ["person", "reason", "confidence"], + "properties": { + "person": { + "type": "string", + "description": "The likely decision owner selected from likelyOwners, or an empty string when required is false." + }, + "reason": { + "type": "string", + "description": "Why this person is the best available owner for this choice, or an empty string when required is false." + }, + "confidence": { + "type": "string", + "enum": ["high", "medium", "low"] + } + } + } + } + }, "triagePriority": { "type": "string", "enum": ["P0", "P1", "P2", "P3", "none"], diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index f961ef36fd..1abe4723ad 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -108,9 +108,13 @@ import { } from "./clawsweeper-args.js"; import { escapeRegExp, safeOutputTail, trimMiddle, truncateText } from "./clawsweeper-text.js"; import { + emptyMaintainerDecision, + maintainerDecisionFromReport, + parseMaintainerDecision, renderDecisionPacketPublicBlock, syncDecisionPacketRecord, type DecisionPacketSubjectState, + type MaintainerDecision, } from "./decision-packets.js"; import { appendReviewHistoryCycle, @@ -517,6 +521,7 @@ interface Decision { likelyOwners: LikelyOwner[]; risks: string[]; bestSolution: string; + maintainerDecision: MaintainerDecision; triagePriority: TriagePriority; impactLabels: ImpactLabelName[]; mergeRiskLabels: MergeRiskLabelName[]; @@ -1759,6 +1764,7 @@ const DECISION_SCHEMA_KEYS = new Set([ "likelyOwners", "risks", "bestSolution", + "maintainerDecision", "triagePriority", "impactLabels", "mergeRiskLabels", @@ -1880,6 +1886,7 @@ const REVIEW_SECTIONS = { summary: "Summary", changeSummary: "What This Changes", bestSolution: "Best Possible Solution", + maintainerDecision: "Maintainer Decision", reproductionAssessment: "Reproduction Assessment", solutionAssessment: "Solution Assessment", visionFit: "Vision Fit", @@ -2664,6 +2671,18 @@ function validateMergeRiskOptions( } } +function validateMaintainerDecisionOwner( + decision: Pick, +): void { + if (!decision.maintainerDecision.required) return; + const selected = decision.maintainerDecision.likelyOwner.person; + if (!decision.likelyOwners.some((owner) => owner.person === selected)) { + throw new Error( + "decision.maintainerDecision.likelyOwner.person must match decision.likelyOwners", + ); + } +} + function parseLabelJustification(value: unknown, path: string): LabelJustification { const record = requireRecord(value, path); rejectUnexpectedKeys(record, LABEL_JUSTIFICATION_SCHEMA_KEYS, path); @@ -2833,6 +2852,7 @@ function normalizeDecisionForItem( ...decision, reviewFindings, bestSolution: CLEAN_OPENCLAW_PR_REVIEW_NEXT_STEP, + maintainerDecision: emptyMaintainerDecision(), triagePriority: decision.triagePriority, mergeRiskOptions: decision.mergeRiskOptions, labelJustifications: decision.labelJustifications, @@ -3168,6 +3188,10 @@ export function parseDecision(value: unknown, item?: DecisionNormalizationItem): (risk) => !isEnvironmentAccessCaveat(risk), ), bestSolution: requireString(record.bestSolution, "decision.bestSolution"), + maintainerDecision: parseMaintainerDecision( + record.maintainerDecision, + "decision.maintainerDecision", + ), triagePriority: requireEnum( record.triagePriority, TRIAGE_PRIORITIES, @@ -3265,6 +3289,7 @@ export function parseDecision(value: unknown, item?: DecisionNormalizationItem): workLikelyFiles: requireStringArray(record.workLikelyFiles, "decision.workLikelyFiles"), }; validateMergeRiskOptions(decision); + validateMaintainerDecisionOwner(decision); validateLabelJustifications(decision); return normalizeDecisionForItem(decision, item); } @@ -7510,6 +7535,7 @@ function codexFailureDecision( ], risks: ["No close action taken because the review did not complete."], bestSolution: "Retry the Codex review after fixing the execution failure.", + maintainerDecision: emptyMaintainerDecision(), triagePriority: "none", impactLabels: [], mergeRiskLabels: [], @@ -13004,6 +13030,7 @@ function reportDecision(markdown: string, closeReason: CloseReason): Decision { likelyOwners: reportLikelyOwners(markdown), risks: [], bestSolution: reviewSectionValue(markdown, "bestSolution"), + maintainerDecision: maintainerDecisionFromReport(markdown) ?? emptyMaintainerDecision(), triagePriority, impactLabels, mergeRiskLabels, @@ -16444,6 +16471,36 @@ function renderRepairWorkPromptReportSection(decision: Decision): string { return workPrompt ? `\n\n## ${REVIEW_SECTIONS.repairWorkPrompt}\n\n${workPrompt}` : ""; } +function renderMaintainerDecisionReportSection(decision: Decision): string { + const maintainerDecision = decision.maintainerDecision; + if (!maintainerDecision.required) return "Required: false"; + const options = maintainerDecision.options + .map( + (option) => + `- **${option.title}${option.recommended ? " (recommended)" : ""}:** ${option.body}`, + ) + .join("\n"); + return [ + "Required: true", + "", + `Kind: ${maintainerDecision.kind}`, + "", + `Question: ${maintainerDecision.question}`, + "", + `Rationale: ${maintainerDecision.rationale}`, + "", + `Likely owner: ${maintainerDecision.likelyOwner.person}`, + "", + `Owner reason: ${maintainerDecision.likelyOwner.reason}`, + "", + `Owner confidence: ${maintainerDecision.likelyOwner.confidence}`, + "", + "Options:", + "", + options, + ].join("\n"); +} + function renderVisionFitReportSection(decision: Decision): string { return [ `Status: ${decision.visionFit}`, @@ -16687,6 +16744,7 @@ function markdownFor(options: { .join("\n") : "- none"; const bestSolution = options.decision.bestSolution.trim() || "_Not provided._"; + const maintainerDecision = renderMaintainerDecisionReportSection(options.decision); const reproductionAssessment = options.decision.reproductionAssessment.trim() || "_Not provided._"; const solutionAssessment = options.decision.solutionAssessment.trim() || "_Not provided._"; @@ -16774,6 +16832,7 @@ work_cluster_refs: ${jsonFrontMatterValue(options.decision.workClusterRefs)} root_cause_cluster: ${JSON.stringify(options.decision.rootCauseCluster)} work_validation: ${jsonFrontMatterValue(options.decision.workValidation)} work_likely_files: ${jsonFrontMatterValue(options.decision.workLikelyFiles)} +maintainer_decision: ${JSON.stringify(options.decision.maintainerDecision)} triage_priority: ${options.decision.triagePriority} impact_labels: ${jsonFrontMatterValue(options.decision.impactLabels)} merge_risk_labels: ${jsonFrontMatterValue(options.decision.mergeRiskLabels)} @@ -16864,6 +16923,10 @@ ${options.decision.changeSummary} ${bestSolution} +## ${REVIEW_SECTIONS.maintainerDecision} + +${maintainerDecision} + ## ${REVIEW_SECTIONS.reproductionAssessment} ${reproductionAssessment} diff --git a/src/decision-packets.ts b/src/decision-packets.ts index bc3659cfee..a189b63f8d 100644 --- a/src/decision-packets.ts +++ b/src/decision-packets.ts @@ -2,33 +2,38 @@ import { createHash } from "node:crypto"; import { existsSync, mkdirSync, unlinkSync, writeFileSync } from "node:fs"; import { dirname, relative } from "node:path"; -export type DecisionPacketLane = - | "product_contract" +export type MaintainerDecisionKind = + | "none" + | "product_direction" | "security_boundary" - | "maintainer_review" + | "proof_sufficiency" | "release_inclusion" - | "proof_or_repro_decision"; + | "merge_risk" + | "manual_review"; +export type MaintainerDecisionConfidence = "high" | "medium" | "low"; export type DecisionPacketPriority = "P0" | "P1" | "P2" | "P3" | "none"; -export type DecisionPacketEvidenceStrength = "low" | "medium" | "high"; export type DecisionPacketSubjectState = "open" | "closed" | "merged"; -export interface DecisionPacketEvidence { - label: string; - detail: string; - url?: string; - file?: string; - line?: number; - sha?: string; +export interface MaintainerDecisionOption { + title: string; + body: string; + recommended: boolean; } -export interface DecisionPacketLinkedItem { - repo: string; - kind: "issue" | "pull_request"; - number: number; - url: string; - state?: DecisionPacketSubjectState; - relationship: "fix" | "duplicate" | "dependency" | "prior_art" | "symptom" | "related"; +export interface MaintainerDecisionOwner { + person: string; + reason: string; + confidence: MaintainerDecisionConfidence; +} + +export interface MaintainerDecision { + required: boolean; + kind: MaintainerDecisionKind; + question: string; + rationale: string; + options: MaintainerDecisionOption[]; + likelyOwner: MaintainerDecisionOwner; } export interface DecisionPacket { @@ -48,17 +53,13 @@ export interface DecisionPacket { stateChangedAt?: string; headSha?: string; }; - lane: DecisionPacketLane; + lane: Exclude; priority: DecisionPacketPriority; - claim: string; - maintainerQuestion: string; - whyHuman: string; - evidenceStrength: DecisionPacketEvidenceStrength; - evidence: DecisionPacketEvidence[]; - linkedItems: DecisionPacketLinkedItem[]; - risks: string[]; - recommendedActions: string[]; - suggestedLabels: string[]; + question: string; + rationale: string; + options: MaintainerDecisionOption[]; + recommendation: MaintainerDecisionOption; + likelyOwner: MaintainerDecisionOwner; source: { reportPath: string; reportUrl?: string; @@ -89,53 +90,119 @@ export interface DecisionPacketSyncResult { packetSha256?: string; } -const DECISION_LABEL_PREFIXES = [ - "clawsweeper:", - "proof:", - "status:", - "app:", - "channel:", - "extensions:", - "impact:", - "merge-risk:", - "triage:", -] as const; -const DECISION_LABEL_MATCHES = ["release-blocker", "beta-blocker"] as const; +const MAINTAINER_DECISION_KINDS = new Set([ + "none", + "product_direction", + "security_boundary", + "proof_sufficiency", + "release_inclusion", + "merge_risk", + "manual_review", +]); +const CONFIDENCES = new Set(["high", "medium", "low"]); +const DECISION_KEYS = new Set([ + "required", + "kind", + "question", + "rationale", + "options", + "likelyOwner", +]); +const OPTION_KEYS = new Set(["title", "body", "recommended"]); +const OWNER_KEYS = new Set(["person", "reason", "confidence"]); + +export function parseMaintainerDecision( + value: unknown, + path = "maintainerDecision", +): MaintainerDecision { + const record = objectValue(value, path); + rejectUnexpectedKeys(record, DECISION_KEYS, path); + const required = booleanValue(record.required, `${path}.required`); + const kind = enumValue(record.kind, MAINTAINER_DECISION_KINDS, `${path}.kind`); + const question = stringValue(record.question, `${path}.question`).trim(); + const rationale = stringValue(record.rationale, `${path}.rationale`).trim(); + if (!Array.isArray(record.options)) throw new Error(`${path}.options must be an array`); + const options = record.options.map((entry, index) => + parseMaintainerDecisionOption(entry, `${path}.options[${index}]`), + ); + if (options.length > 3) throw new Error(`${path}.options must contain at most 3 options`); + const likelyOwner = parseMaintainerDecisionOwner(record.likelyOwner, `${path}.likelyOwner`); + + if (!required) { + if (kind !== "none") throw new Error(`${path}.kind must be none when no decision is required`); + if (question || rationale || options.length || likelyOwner.person || likelyOwner.reason) { + throw new Error(`${path} must be empty when no decision is required`); + } + } else { + if (kind === "none") throw new Error(`${path}.kind must identify the required decision`); + if (!question) throw new Error(`${path}.question must not be empty`); + if (!rationale) throw new Error(`${path}.rationale must not be empty`); + if (options.length === 0) throw new Error(`${path}.options must contain at least 1 option`); + if (options.filter((option) => option.recommended).length !== 1) { + throw new Error(`${path}.options must contain exactly 1 recommended option`); + } + if (!likelyOwner.person) throw new Error(`${path}.likelyOwner.person must not be empty`); + if (!likelyOwner.reason) throw new Error(`${path}.likelyOwner.reason must not be empty`); + } + + return { required, kind, question, rationale, options, likelyOwner }; +} + +export function emptyMaintainerDecision(): MaintainerDecision { + return { + required: false, + kind: "none", + question: "", + rationale: "", + options: [], + likelyOwner: { person: "", reason: "", confidence: "low" }, + }; +} + +export function maintainerDecisionFromReport(markdown: string): MaintainerDecision | null { + const raw = frontMatter(markdown).maintainer_decision; + if (!raw || raw === "none" || raw === "unknown") return null; + try { + return parseMaintainerDecision(JSON.parse(raw), "maintainer_decision"); + } catch { + return null; + } +} export function buildDecisionPacketFromReport( markdown: string, options: DecisionPacketBuildOptions = {}, ): DecisionPacket | null { const frontmatter = frontMatter(markdown); + const decision = maintainerDecisionFromReport(markdown); const repo = frontmatter.repository; const kind = frontmatter.type; const number = numberValue(frontmatter.number); - if (!repo || (kind !== "issue" && kind !== "pull_request") || number === null) return null; - - const labels = stringArrayValue(frontmatter.labels); - const lane = packetLane(frontmatter, markdown, labels); - if (!lane) return null; + if ( + !decision?.required || + decision.kind === "none" || + !repo || + (kind !== "issue" && kind !== "pull_request") || + number === null + ) { + return null; + } const generatedAt = options.generatedAt ?? frontmatter.reviewed_at ?? new Date().toISOString(); - const updatedAt = frontmatter.reviewed_at ?? generatedAt; + const createdAt = knownValue(frontmatter.item_created_at); const subjectUpdatedAt = knownValue(frontmatter.current_item_updated_at) ?? knownValue(frontmatter.item_updated_at); - const title = frontmatter.title ?? reportHeadingTitle(markdown) ?? `#${number}`; - const url = - frontmatter.url ?? - `https://github.com/${repo}/${kind === "pull_request" ? "pull" : "issues"}/${number}`; + const updatedAt = subjectUpdatedAt ?? frontmatter.reviewed_at ?? generatedAt; + const stateChangedAt = knownValue(frontmatter.current_item_closed_at); + const headSha = knownValue(frontmatter.pull_head_sha); const reviewCommentUrl = knownValue(frontmatter.review_comment_url); + const reviewedAt = knownValue(frontmatter.reviewed_at); const mainSha = knownValue(frontmatter.main_sha); - const headSha = knownValue(frontmatter.pull_head_sha); - const claim = firstSentence(sectionValue(markdown, "Summary")) || title; - const recommendedActions = recommendedActionsFor(lane, markdown, frontmatter); - const source: DecisionPacket["source"] = { - reportPath: options.reportPath ?? "", - ...(options.reportUrl ? { reportUrl: options.reportUrl } : {}), - ...(reviewCommentUrl ? { reviewCommentUrl } : {}), - ...(frontmatter.reviewed_at ? { reviewedAt: frontmatter.reviewed_at } : {}), - ...(mainSha ? { mainSha } : {}), - }; + const url = + knownValue(frontmatter.url) ?? + `https://github.com/${repo}/${kind === "pull_request" ? "pull" : "issues"}/${number}`; + const recommendation = decision.options.find((entry) => entry.recommended); + if (!recommendation) return null; return { version: 1, @@ -145,56 +212,58 @@ export function buildDecisionPacketFromReport( repo, kind, number, - title, + title: frontmatter.title ?? `#${number}`, url, state: options.subjectState ?? stateFromReport(frontmatter), - labels, - ...(frontmatter.item_created_at ? { createdAt: frontmatter.item_created_at } : {}), + labels: stringArrayValue(frontmatter.labels), + ...(createdAt ? { createdAt } : {}), ...(subjectUpdatedAt ? { updatedAt: subjectUpdatedAt } : {}), - ...(frontmatter.current_item_closed_at - ? { stateChangedAt: frontmatter.current_item_closed_at } - : {}), + ...(stateChangedAt ? { stateChangedAt } : {}), ...(headSha ? { headSha } : {}), }, - lane, - priority: priorityFrom(frontmatter, labels), - claim, - maintainerQuestion: maintainerQuestionFor(lane, frontmatter), - whyHuman: whyHumanFor(lane), - evidenceStrength: evidenceStrengthFrom(frontmatter), - evidence: packetEvidence(markdown, frontmatter, labels), - linkedItems: linkedItemsFrom(frontmatter, markdown), - risks: risksFrom(markdown, frontmatter), - recommendedActions, - suggestedLabels: suggestedLabels(labels), - source, + lane: decision.kind, + priority: priorityValue(frontmatter.triage_priority), + question: decision.question, + rationale: decision.rationale, + options: decision.options, + recommendation, + likelyOwner: decision.likelyOwner, + source: { + reportPath: options.reportPath ?? "", + ...(options.reportUrl ? { reportUrl: options.reportUrl } : {}), + ...(reviewCommentUrl ? { reviewCommentUrl } : {}), + ...(reviewedAt ? { reviewedAt } : {}), + ...(mainSha ? { mainSha } : {}), + }, }; } export function renderDecisionPacketPublicBlock(markdown: string): string { const packet = buildDecisionPacketFromReport(markdown); if (!packet) return ""; - const action = packet.recommendedActions[0] ?? packet.maintainerQuestion; + const options = packet.options.map( + (option) => + ` - **${option.title}${option.recommended ? " (recommended)" : ""}:** ${option.body}`, + ); return [ - `- Lane: ${publicLaneName(packet.lane)}.`, - `- Question: ${packet.maintainerQuestion}`, - `- Evidence strength: ${packet.evidenceStrength}.`, - `- Recommended action: ${action}`, + `- Question: ${packet.question}`, + `- Rationale: ${packet.rationale}`, + `- Likely owner: ${packet.likelyOwner.person} — ${packet.likelyOwner.reason}`, + "- Options:", + ...options, ].join("\n"); } export function syncDecisionPacketRecord( options: DecisionPacketSyncOptions, ): DecisionPacketSyncResult { - const buildOptions: DecisionPacketBuildOptions = { + const packet = buildDecisionPacketFromReport(options.markdown, { reportPath: repoRelativePath(options.repoRoot, options.reportPath), ...(options.generatedAt ? { generatedAt: options.generatedAt } : {}), ...(options.reportUrl ? { reportUrl: options.reportUrl } : {}), ...(options.subjectState ? { subjectState: options.subjectState } : {}), - }; - const packet = buildDecisionPacketFromReport(options.markdown, buildOptions); - const frontmatter = frontMatter(options.markdown); - const number = numberValue(frontmatter.number); + }); + const number = numberValue(frontMatter(options.markdown).number); const packetPath = number === null ? undefined : `${options.packetsDir}/${number}.json`; if (!packet || !packetPath) { if (packetPath && existsSync(packetPath)) unlinkSync(packetPath); @@ -208,261 +277,87 @@ export function syncDecisionPacketRecord( mkdirSync(dirname(packetPath), { recursive: true }); const json = `${JSON.stringify(packet, null, 2)}\n`; writeFileSync(packetPath, json, "utf8"); - const packetSha256 = sha256(json); - const markdown = replacePacketFrontmatter( - options.markdown, - repoRelativePath(options.repoRoot, packetPath), + const packetSha256 = createHash("sha256").update(json).digest("hex"); + return { + markdown: replacePacketFrontmatter( + options.markdown, + repoRelativePath(options.repoRoot, packetPath), + packetSha256, + ), + packet, + packetPath, packetSha256, - ); - return { markdown, packet, packetPath, packetSha256 }; -} - -function packetLane( - frontmatter: Record, - markdown: string, - labels: readonly string[], -): DecisionPacketLane | null { - const normalizedLabels = labels.map((label) => label.toLowerCase()); - const securityStatus = sectionLineValue(sectionValue(markdown, "Security Review"), "Status"); - if ( - frontmatter.item_category === "security" || - securityStatus === "needs_attention" || - normalizedLabels.includes("clawsweeper:needs-security-review") || - normalizedLabels.some((label) => label.startsWith("merge-risk:") && label.includes("security")) - ) { - return "security_boundary"; - } - if ( - frontmatter.requires_product_decision === "true" || - normalizedLabels.includes("clawsweeper:needs-product-decision") - ) { - return "product_contract"; - } - if ( - normalizedLabels.includes("clawsweeper:needs-live-repro") || - normalizedLabels.includes("triage: needs-real-behavior-proof") || - normalizedLabels.some( - (label) => label.startsWith("status:") && label.includes("needs proof"), - ) || - frontmatter.real_behavior_proof_needs_contributor_action === "true" - ) { - return "proof_or_repro_decision"; - } - if ( - frontmatter.work_status === "manual_review" || - frontmatter.work_candidate === "manual_review" || - normalizedLabels.includes("clawsweeper:needs-maintainer-review") - ) { - return "maintainer_review"; - } - if ( - normalizedLabels.some( - (label) => label.includes("release-blocker") || label.includes("beta-blocker"), - ) - ) { - return "release_inclusion"; - } - return null; -} - -function stateFromReport(frontmatter: Record): DecisionPacketSubjectState { - if ( - frontmatter.current_state === "open" || - frontmatter.current_state === "closed" || - frontmatter.current_state === "merged" - ) { - return frontmatter.current_state; - } - if ( - frontmatter.action_taken === "closed" || - frontmatter.action_taken === "skipped_already_closed" - ) { - return "closed"; - } - return "open"; -} - -function priorityFrom( - frontmatter: Record, - labels: readonly string[], -): DecisionPacketPriority { - if (isPriority(frontmatter.triage_priority)) return frontmatter.triage_priority; - const labelPriority = labels.find(isPriority); - return labelPriority ?? "none"; -} - -function evidenceStrengthFrom(frontmatter: Record): DecisionPacketEvidenceStrength { - const confidence = frontmatter.confidence; - return confidence === "high" || confidence === "medium" || confidence === "low" - ? confidence - : "medium"; -} - -function maintainerQuestionFor( - lane: DecisionPacketLane, - frontmatter: Record, -): string { - if (lane === "security_boundary") { - return "Should this security or trust-boundary change be accepted before merge?"; - } - if (lane === "product_contract") { - return "Should this product/API contract direction be accepted?"; - } - if (lane === "proof_or_repro_decision") { - return "Is the available proof or reproduction enough for maintainer action?"; - } - if (lane === "release_inclusion") { - return "Should this item block or be included in the release?"; - } - if (frontmatter.work_reason) return firstSentence(frontmatter.work_reason); - return "What maintainer action should happen next for this item?"; + }; } -function whyHumanFor(lane: DecisionPacketLane): string { - switch (lane) { - case "security_boundary": - return "This affects a trust boundary or security-sensitive behavior that should not be decided by automation."; - case "product_contract": - return "This changes product/API direction and needs a maintainer ruling before automation can act safely."; - case "proof_or_repro_decision": - return "Automation can collect proof, but a maintainer must decide whether the proof is sufficient."; - case "release_inclusion": - return "Release inclusion and blocking decisions need maintainer judgment."; - case "maintainer_review": - return "ClawSweeper classified this as manual-review work."; - } +function parseMaintainerDecisionOption(value: unknown, path: string): MaintainerDecisionOption { + const record = objectValue(value, path); + rejectUnexpectedKeys(record, OPTION_KEYS, path); + const title = stringValue(record.title, `${path}.title`).trim(); + const body = stringValue(record.body, `${path}.body`).trim(); + if (!title) throw new Error(`${path}.title must not be empty`); + if (!body) throw new Error(`${path}.body must not be empty`); + return { title, body, recommended: booleanValue(record.recommended, `${path}.recommended`) }; } -function recommendedActionsFor( - lane: DecisionPacketLane, - markdown: string, - frontmatter: Record, -): string[] { - const candidates = [ - sectionValue(markdown, "Best Possible Solution"), - sectionValue(markdown, "Best Solution"), - sectionValue(markdown, "Repair Work Prompt"), - sectionLineValue(sectionValue(markdown, "Work Candidate"), "Reason"), - ] - .map(firstSentence) - .filter(Boolean); - if (candidates.length > 0) return uniqueStrings(candidates).slice(0, 3); - if (frontmatter.work_status === "manual_review") - return ["Assign a maintainer to review the item."]; - if (lane === "security_boundary") - return ["Ask the security or owning maintainer to rule before merge."]; - if (lane === "product_contract") - return ["Ask the owning maintainer to rule on the product/API contract."]; - if (lane === "proof_or_repro_decision") - return ["Ask a maintainer whether the proof/reproduction is sufficient."]; - return ["Ask a maintainer for the next decision."]; +function parseMaintainerDecisionOwner(value: unknown, path: string): MaintainerDecisionOwner { + const record = objectValue(value, path); + rejectUnexpectedKeys(record, OWNER_KEYS, path); + return { + person: stringValue(record.person, `${path}.person`).trim(), + reason: stringValue(record.reason, `${path}.reason`).trim(), + confidence: enumValue(record.confidence, CONFIDENCES, `${path}.confidence`), + }; } -function packetEvidence( - markdown: string, - frontmatter: Record, - labels: readonly string[], -): DecisionPacketEvidence[] { - const evidence: DecisionPacketEvidence[] = []; - addEvidence(evidence, "Summary", sectionValue(markdown, "Summary")); - addEvidence(evidence, "Security review", compactLines(sectionValue(markdown, "Security Review"))); - addEvidence( - evidence, - "Real behavior proof", - compactLines(sectionValue(markdown, "Real Behavior Proof")), - ); - addEvidence(evidence, "Work candidate", compactLines(sectionValue(markdown, "Work Candidate"))); - const labelEvidence = suggestedLabels(labels).join(", "); - addEvidence(evidence, "Decision labels", labelEvidence); - if (frontmatter.review_comment_url && frontmatter.review_comment_url !== "unknown") { - evidence.push({ - label: "Durable review comment", - detail: "ClawSweeper durable review comment is available.", - url: frontmatter.review_comment_url, - }); +function objectValue(value: unknown, path: string): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error(`${path} must be an object`); } - return evidence.slice(0, 8); -} - -function risksFrom(markdown: string, frontmatter: Record): string[] { - const risks = bulletLines( - sectionValue(markdown, "Risks / Open Questions") || sectionValue(markdown, "Risks"), - ); - const mergeRiskLabels = stringArrayValue(frontmatter.merge_risk_labels); - return uniqueStrings([...risks, ...mergeRiskLabels]).slice(0, 8); + return value as Record; } -function linkedItemsFrom( - frontmatter: Record, - markdown: string, -): DecisionPacketLinkedItem[] { - const defaultRepo = subjectRepoFrom(frontmatter); - const refs = [ - ...stringArrayValue(frontmatter.work_cluster_refs), - ...rootCauseMemberRefs(frontmatter.root_cause_cluster), - ...textRefs(sectionValue(markdown, "Root-Cause Cluster"), defaultRepo), - ]; - const items: DecisionPacketLinkedItem[] = []; - const seen = new Set(); - for (const ref of refs) { - const parsed = parseRepoRef(ref, defaultRepo); - if (!parsed) continue; - const key = `${parsed.repo}:${parsed.kind}:${parsed.number}`; - if (seen.has(key)) continue; - seen.add(key); - items.push({ - repo: parsed.repo, - kind: parsed.kind, - number: parsed.number, - url: `https://github.com/${parsed.repo}/${parsed.kind === "pull_request" ? "pull" : "issues"}/${parsed.number}`, - relationship: "related", - }); - } - return items.slice(0, 12); +function rejectUnexpectedKeys( + record: Record, + allowed: ReadonlySet, + path: string, +): void { + const unexpected = Object.keys(record).filter((key) => !allowed.has(key)); + if (unexpected.length) throw new Error(`${path} has unexpected keys: ${unexpected.join(", ")}`); } -function subjectRepoFrom(frontmatter: Record): string | undefined { - if (frontmatter.repository) return frontmatter.repository; - return frontmatter.url?.match(/\bgithub\.com\/([a-z0-9_.-]+\/[a-z0-9_.-]+)\b/i)?.[1]; +function stringValue(value: unknown, path: string): string { + if (typeof value !== "string") throw new Error(`${path} must be a string`); + return value; } -function suggestedLabels(labels: readonly string[]): string[] { - return labels - .filter(isDecisionLabel) - .concat(labels.filter(isPriority)) - .filter((label, index, array) => array.indexOf(label) === index); +function booleanValue(value: unknown, path: string): boolean { + if (typeof value !== "boolean") throw new Error(`${path} must be a boolean`); + return value; } -function isDecisionLabel(label: string): boolean { - const normalized = label.toLowerCase(); - return ( - DECISION_LABEL_PREFIXES.some((prefix) => normalized.startsWith(prefix)) || - DECISION_LABEL_MATCHES.some((match) => normalized.includes(match)) - ); +function enumValue(value: unknown, values: ReadonlySet, path: string): T { + if (typeof value === "string" && values.has(value as T)) return value as T; + throw new Error(`${path} has invalid value`); } function frontMatter(markdown: string): Record { const match = markdown.match(/^---\r?\n([\s\S]*?)\r?\n---/); - const block = match?.[1] ?? ""; const values: Record = {}; - for (const line of block.split(/\r?\n/)) { + for (const line of (match?.[1] ?? "").split(/\r?\n/)) { const separator = line.indexOf(":"); if (separator <= 0) continue; - const key = line.slice(0, separator).trim(); - const value = line.slice(separator + 1).trim(); - values[key] = unquote(value); + values[line.slice(0, separator).trim()] = unquote(line.slice(separator + 1).trim()); } return values; } -function replacePacketFrontmatter( - markdown: string, - packetPath: string, - packetSha256: string, -): string { - let next = replaceFrontMatterValue(markdown, "decision_packet_path", packetPath); - next = replaceFrontMatterValue(next, "decision_packet_sha256", packetSha256); - return next; +function replacePacketFrontmatter(markdown: string, path: string, sha256: string): string { + return replaceFrontMatterValue( + replaceFrontMatterValue(markdown, "decision_packet_path", path), + "decision_packet_sha256", + sha256, + ); } function replaceFrontMatterValue(markdown: string, key: string, value: string): string { @@ -472,181 +367,46 @@ function replaceFrontMatterValue(markdown: string, key: string, value: string): return markdown.replace(/^---\r?\n/, `---\n${line}\n`); } -function sectionValue(markdown: string, heading: string): string { - const normalized = markdown.replace(/\r\n?/g, "\n"); - const match = normalized.match( - new RegExp(`(?:^|\\n)## ${escapeRegExp(heading)}\\n\\n([\\s\\S]*?)(?=\\n## |\\n?$)`), - ); - return match?.[1]?.trim() ?? ""; -} - -function sectionLineValue(section: string, label: string): string { - const pattern = new RegExp(`^${escapeRegExp(label)}:\\s*(.+)$`, "im"); - return pattern.exec(section)?.[1]?.trim() ?? ""; +function numberValue(value: string | undefined): number | null { + const number = Number(value); + return Number.isInteger(number) && number > 0 ? number : null; } function stringArrayValue(value: string | undefined): string[] { - if (!value || value === "unknown" || value === "none") return []; + if (!value || value === "none" || value === "unknown") return []; try { const parsed: unknown = JSON.parse(value); - if (Array.isArray(parsed)) { - return parsed.filter((item): item is string => typeof item === "string"); - } + if (Array.isArray(parsed)) + return parsed.filter((entry): entry is string => typeof entry === "string"); } catch { - // Older reports used plain comma-separated labels. + // Legacy report labels were comma separated. } return value .split(",") - .map((item) => item.trim()) + .map((entry) => entry.trim()) .filter(Boolean); } -function numberValue(value: string | undefined): number | null { - const number = Number(value); - return Number.isInteger(number) && number > 0 ? number : null; -} - function knownValue(value: string | undefined): string | undefined { - return value && value !== "unknown" && value !== "none" ? value : undefined; + return value && value !== "none" && value !== "unknown" ? value : undefined; } -function firstSentence(value: string | undefined): string { - const normalized = compactLines(value ?? ""); - if (!normalized || normalized === "- none" || normalized === "_Not provided._") return ""; - const sentence = normalized.match(/^(.+?[.!?])(?:\s|$)/)?.[1]; - return (sentence ?? normalized).trim(); +function priorityValue(value: string | undefined): DecisionPacketPriority { + return value === "P0" || value === "P1" || value === "P2" || value === "P3" ? value : "none"; } -function compactLines(value: string): string { - return value - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean) - .join(" "); -} - -function addEvidence(items: DecisionPacketEvidence[], label: string, detail: string): void { - const text = firstSentence(detail); - if (text) items.push({ label, detail: text }); -} - -function bulletLines(value: string): string[] { - return value - .split(/\r?\n/) - .map((line) => line.trim().replace(/^-\s+/, "")) - .filter((line) => line && line !== "none"); -} - -function rootCauseMemberRefs(value: string | undefined): string[] { - if (!value || value === "unknown") return []; - try { - const parsed: unknown = JSON.parse(value); - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return []; - const members = (parsed as { members?: unknown }).members; - if (!Array.isArray(members)) return []; - return members - .map((member) => - member && typeof member === "object" && "ref" in member - ? (member as { ref?: unknown }).ref - : undefined, - ) - .filter((ref): ref is string => typeof ref === "string"); - } catch { - return []; - } -} - -function textRefs(value: string, defaultRepo: string | undefined): string[] { - const refs = [ - ...[ - ...value.matchAll( - /\bhttps?:\/\/github\.com\/[a-z0-9_.-]+\/[a-z0-9_.-]+\/(?:issues|pull)\/\d+\b/gi, - ), - ].map((match) => match[0]), - ...[...value.matchAll(/\b([a-z0-9_.-]+\/[a-z0-9_.-]+)#(\d+)\b/gi)].map( - (match) => `${match[1]}#${match[2]}`, - ), - ]; - if (defaultRepo) { - refs.push( - ...[...value.matchAll(/(?:^|[^\w/])#(\d+)\b/g)].map((match) => `${defaultRepo}#${match[1]}`), - ); - } - return uniqueStrings(refs); -} - -function parseRepoRef( - value: string, - defaultRepo?: string, -): { repo: string; kind: "issue" | "pull_request"; number: number } | null { - const urlMatch = value.match( - /\bhttps?:\/\/github\.com\/([a-z0-9_.-]+\/[a-z0-9_.-]+)\/(issues|pull)\/(\d+)\b/i, - ); - if (urlMatch?.[1] && urlMatch[2] && urlMatch[3]) { - return { - repo: urlMatch[1], - kind: urlMatch[2] === "pull" ? "pull_request" : "issue", - number: Number(urlMatch[3]), - }; - } - const match = value.match(/\b([a-z0-9_.-]+\/[a-z0-9_.-]+)#(\d+)\b/i); - if (match?.[1] && match[2]) return { repo: match[1], kind: "issue", number: Number(match[2]) }; - const shorthandMatch = value.match(/(?:^|[^\w/])#(\d+)\b/); - if (defaultRepo && shorthandMatch?.[1]) { - return { repo: defaultRepo, kind: "issue", number: Number(shorthandMatch[1]) }; - } - return null; -} - -function reportHeadingTitle(markdown: string): string | undefined { - for (const rawLine of markdown.split("\n")) { - const line = rawLine.endsWith("\r") ? rawLine.slice(0, -1) : rawLine; - if (!line.startsWith("#")) continue; - const afterMarker = line.slice(1); - if (!afterMarker.startsWith(" ") && !afterMarker.startsWith("\t")) continue; - let heading = afterMarker.trimStart(); - if (heading.startsWith("#")) { - const colonIndex = heading.indexOf(":"); - const issueNumber = colonIndex === -1 ? "" : heading.slice(1, colonIndex); - if (issueNumber && isDecimalDigits(issueNumber)) - heading = heading.slice(colonIndex + 1).trimStart(); - } - const trimmed = heading.trim(); - if (trimmed) return trimmed; - } - return undefined; -} - -function isDecimalDigits(value: string): boolean { - for (const char of value) if (char < "0" || char > "9") return false; - return value.length > 0; -} - -function publicLaneName(lane: DecisionPacketLane): string { - switch (lane) { - case "product_contract": - return "Product/API contract"; - case "security_boundary": - return "Security boundary"; - case "maintainer_review": - return "Maintainer review"; - case "release_inclusion": - return "Release inclusion"; - case "proof_or_repro_decision": - return "Proof/repro decision"; +function stateFromReport(frontmatter: Record): DecisionPacketSubjectState { + if ( + frontmatter.current_state === "open" || + frontmatter.current_state === "closed" || + frontmatter.current_state === "merged" + ) { + return frontmatter.current_state; } -} - -function repoRelativePath(repoRoot: string, path: string): string { - return relative(repoRoot, path).replace(/\\/g, "/"); -} - -function isPriority(value: string | undefined): value is DecisionPacketPriority { - return value === "P0" || value === "P1" || value === "P2" || value === "P3"; -} - -function uniqueStrings(values: readonly string[]): string[] { - return [...new Set(values.filter(Boolean))]; + return frontmatter.action_taken === "closed" || + frontmatter.action_taken === "skipped_already_closed" + ? "closed" + : "open"; } function unquote(value: string): string { @@ -661,8 +421,8 @@ function unquote(value: string): string { return value; } -function sha256(value: string): string { - return createHash("sha256").update(value).digest("hex"); +function repoRelativePath(repoRoot: string, path: string): string { + return relative(repoRoot, path).replace(/\\/g, "/"); } function escapeRegExp(value: string): string { diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index 587d616424..0a13060e5e 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -51,6 +51,30 @@ import { workPlanCandidateReport, } from "./helpers.ts"; +const maintainerDecision = { + required: true, + kind: "product_direction", + question: "Should this product contract be accepted?", + rationale: "The implementation is valid only if maintainers choose this public behavior.", + options: [ + { + title: "Accept the contract", + body: "Adopt and document the proposed behavior.", + recommended: true, + }, + { + title: "Keep the current contract", + body: "Close the proposal without changing current behavior.", + recommended: false, + }, + ], + likelyOwner: { + person: "@owner", + reason: "Recent history shows ownership of this contract.", + confidence: "high", + }, +}; + test("review comments include a compact maintainer decision packet block", () => { const comment = renderReviewCommentFromReport( workPlanCandidateReport({ @@ -58,13 +82,15 @@ test("review comments include a compact maintainer decision packet block", () => action_taken: "kept_open", labels: JSON.stringify(["clawsweeper:needs-product-decision"]), requires_product_decision: "true", + maintainer_decision: JSON.stringify(maintainerDecision), }), "none", ); assert.match(comment, /\*\*Maintainer decision needed\*\*/); - assert.match(comment, /Lane: Product\/API contract\./); - assert.match(comment, /Should this product\/API contract direction be accepted\?/); + assert.match(comment, /Should this product contract be accepted\?/); + assert.match(comment, /Accept the contract \(recommended\)/); + assert.match(comment, /Likely owner: @owner/); }); test("apply-decisions archives live-closed skipped records without reopening close gates", () => { @@ -150,6 +176,7 @@ test("apply-decisions writes decision packets for changed-since-review reports", implementedCloseReport({ labels: JSON.stringify(["clawsweeper:needs-product-decision"]), requires_product_decision: "true", + maintainer_decision: JSON.stringify(maintainerDecision), item_snapshot_hash: "reviewed-snapshot-321", item_updated_at: "2026-05-01T00:00:00Z", }), diff --git a/test/decision-packets.test.ts b/test/decision-packets.test.ts index 72ce635d3a..2e5baa8c12 100644 --- a/test/decision-packets.test.ts +++ b/test/decision-packets.test.ts @@ -5,43 +5,53 @@ import test from "node:test"; import { buildDecisionPacketFromReport, + emptyMaintainerDecision, + parseMaintainerDecision, syncDecisionPacketRecord, } from "../dist/decision-packets.js"; import { tmpPrefix } from "./helpers.ts"; -test("decision packets derive product decision data from report labels and frontmatter", () => { - const report = `${workPlanCandidateReport({ +const productDecision = { + required: true, + kind: "product_direction", + question: "Should config.patch replace redacted array entries or preserve them?", + rationale: + "Both behaviors are coherent, but choosing one defines the public configuration contract.", + options: [ + { + title: "Preserve redacted entries", + body: "Merge visible values into the stored array without deleting redacted entries.", + recommended: true, + }, + { + title: "Replace the array", + body: "Treat the supplied array as authoritative and document the destructive behavior.", + recommended: false, + }, + ], + likelyOwner: { + person: "@config-owner", + reason: "Recent history shows ownership of config.patch semantics.", + confidence: "high", + }, +}; + +test("decision packets preserve the exact Codex-authored maintainer decision", () => { + const report = decisionReport({ number: 81234, repository: "openclaw/openclaw", type: "pull_request", - title: "config.patch redacted array write", + title: JSON.stringify("config.patch redacted array write"), url: "https://github.com/openclaw/openclaw/pull/81234", - labels: JSON.stringify(["clawsweeper:needs-product-decision", "app: web-ui", "P1"]), - requires_product_decision: "true", - confidence: "high", + labels: JSON.stringify(["clawsweeper:needs-product-decision", "P1"]), + triage_priority: "P1", item_updated_at: "2026-06-20T00:00:00Z", current_item_updated_at: "2026-06-23T01:00:00Z", pull_head_sha: "abc123", main_sha: "main456", review_comment_url: "https://github.com/openclaw/openclaw/pull/81234#issuecomment-99", - work_cluster_refs: JSON.stringify([ - "https://github.com/openclaw/openclaw/pull/81111", - "#81112", - "openclaw/clawhub#44", - ]), - root_cause_cluster: JSON.stringify({ - members: [{ ref: "https://github.com/openclaw/openclaw/issues/81113" }], - }), - })} - -## Best Possible Solution - -Ask the owning product maintainer to decide whether redacted full-array writes are valid. - -## Risks / Open Questions - -- The current implementation may overwrite redacted array entries. -`; + maintainer_decision: JSON.stringify(productDecision), + }); const packet = buildDecisionPacketFromReport(report, { generatedAt: "2026-06-23T12:00:00.000Z", @@ -49,152 +59,71 @@ Ask the owning product maintainer to decide whether redacted full-array writes a }); assert.ok(packet); - assert.equal(packet.lane, "product_contract"); + assert.equal(packet.lane, "product_direction"); assert.equal(packet.priority, "P1"); - assert.equal(packet.evidenceStrength, "high"); - assert.equal(packet.subject.repo, "openclaw/openclaw"); - assert.equal(packet.subject.kind, "pull_request"); + assert.equal(packet.question, productDecision.question); + assert.equal(packet.rationale, productDecision.rationale); + assert.deepEqual(packet.options, productDecision.options); + assert.deepEqual(packet.recommendation, productDecision.options[0]); + assert.deepEqual(packet.likelyOwner, productDecision.likelyOwner); assert.equal(packet.subject.headSha, "abc123"); assert.equal(packet.subject.updatedAt, "2026-06-23T01:00:00Z"); - assert.deepEqual(packet.subject.labels, [ - "clawsweeper:needs-product-decision", - "app: web-ui", - "P1", - ]); - assert.deepEqual(packet.suggestedLabels, [ - "clawsweeper:needs-product-decision", - "app: web-ui", - "P1", - ]); - assert.equal( - packet.recommendedActions[0], - "Ask the owning product maintainer to decide whether redacted full-array writes are valid.", - ); - assert.deepEqual(packet.risks, [ - "The current implementation may overwrite redacted array entries.", - ]); - assert.deepEqual( - packet.linkedItems.map((item) => ({ - repo: item.repo, - kind: item.kind, - number: item.number, - })), - [ - { repo: "openclaw/openclaw", kind: "pull_request", number: 81111 }, - { repo: "openclaw/openclaw", kind: "issue", number: 81112 }, - { repo: "openclaw/clawhub", kind: "issue", number: 44 }, - { repo: "openclaw/openclaw", kind: "issue", number: 81113 }, - ], - ); - assert.equal( - packet.linkedItems.every((item) => !("state" in item)), - true, - ); - assert.equal("areaLabel" in packet, false); + assert.equal(packet.updatedAt, "2026-06-23T01:00:00Z"); }); -test("decision packets preserve legacy comma-separated labels", () => { - const packet = buildDecisionPacketFromReport( - workPlanCandidateReport({ - number: 81235, - repository: "openclaw/openclaw", - labels: "clawsweeper:needs-product-decision, status: needs proof, P2", - }), - { - generatedAt: "2026-06-23T12:00:00.000Z", - reportPath: "records/openclaw-openclaw/items/81235.md", - }, - ); +test("labels and report prose cannot invent a maintainer decision", () => { + const report = `${decisionReport({ + labels: JSON.stringify([ + "clawsweeper:needs-product-decision", + "clawsweeper:needs-security-review", + "release-blocker", + ]), + requires_product_decision: "true", + })}\n\n## Best Possible Solution\n\nAsk a maintainer what should happen next.\n`; - assert.ok(packet); - assert.equal(packet.lane, "product_contract"); - assert.deepEqual(packet.subject.labels, [ - "clawsweeper:needs-product-decision", - "status: needs proof", - "P2", - ]); + assert.equal(buildDecisionPacketFromReport(report), null); }); -test("decision packets prefer reconciled current state", () => { - const packet = buildDecisionPacketFromReport( - workPlanCandidateReport({ - number: 81236, - repository: "openclaw/openclaw", - labels: "clawsweeper:needs-product-decision", - action_taken: "kept_open", - current_state: "closed", - }), - { - generatedAt: "2026-06-23T12:00:00.000Z", - reportPath: "records/openclaw-openclaw/closed/81236.md", - }, +test("maintainer decision validation requires one recommendation and an exact owner", () => { + assert.throws( + () => + parseMaintainerDecision({ + ...productDecision, + options: productDecision.options.map((option) => ({ ...option, recommended: false })), + }), + /exactly 1 recommended option/, + ); + assert.throws( + () => + parseMaintainerDecision({ + ...emptyMaintainerDecision(), + question: "A label-derived question", + }), + /must be empty when no decision is required/, ); - - assert.ok(packet); - assert.equal(packet.subject.state, "closed"); -}); - -test("decision packets read CRLF report sections", () => { - const report = `${workPlanCandidateReport({ - number: 81237, - repository: "openclaw/openclaw", - labels: JSON.stringify([]), - })} - -## Security Review - -Status: needs_attention -Concern: Requires maintainer security review. -`.replace(/\n/g, "\r\n"); - const packet = buildDecisionPacketFromReport(report, { - generatedAt: "2026-06-23T12:00:00.000Z", - reportPath: "records/openclaw-openclaw/items/81237.md", - }); - - assert.ok(packet); - assert.equal(packet.lane, "security_boundary"); }); -test("decision packets surface proof and release trigger labels", () => { +test("decision packets prefer reconciled subject state", () => { const packet = buildDecisionPacketFromReport( - workPlanCandidateReport({ - number: 81238, - repository: "openclaw/openclaw", - labels: "release-blocker, beta-blocker, triage: needs-real-behavior-proof", + decisionReport({ + action_taken: "kept_open", + current_state: "closed", + maintainer_decision: JSON.stringify(productDecision), }), - { - generatedAt: "2026-06-23T12:00:00.000Z", - reportPath: "records/openclaw-openclaw/items/81238.md", - }, + { generatedAt: "2026-06-23T12:00:00.000Z" }, ); assert.ok(packet); - assert.equal(packet.lane, "proof_or_repro_decision"); - assert.deepEqual(packet.suggestedLabels, [ - "release-blocker", - "beta-blocker", - "triage: needs-real-behavior-proof", - ]); - assert.match( - packet.evidence.find((entry) => entry.label === "Decision labels")?.detail ?? "", - /triage: needs-real-behavior-proof/, - ); + assert.equal(packet.subject.state, "closed"); }); -test("decision packet sync writes packet JSON and frontmatter pointers", () => { +test("decision packet sync writes pointers and removes stale generated state", () => { const root = mkdtempSync(tmpPrefix); try { - const packetsDir = join(root, "records", "openclaw-openclaw", "decision-packets"); - const reportPath = join(root, "records", "openclaw-openclaw", "items", "321.md"); - const markdown = workPlanCandidateReport({ - repository: "openclaw/openclaw", - labels: JSON.stringify(["clawsweeper:needs-maintainer-review", "channel: telegram"]), - work_status: "manual_review", - confidence: "medium", - }); - - const result = syncDecisionPacketRecord({ - markdown, + const packetsDir = join(root, "records", "openclaw-clawsweeper", "decision-packets"); + const reportPath = join(root, "records", "openclaw-clawsweeper", "items", "321.md"); + const first = syncDecisionPacketRecord({ + markdown: decisionReport({ maintainer_decision: JSON.stringify(productDecision) }), reportPath, packetsDir, repoRoot: root, @@ -202,59 +131,56 @@ test("decision packet sync writes packet JSON and frontmatter pointers", () => { subjectState: "open", }); - assert.ok(result.packet); - assert.ok(result.packetPath); - assert.ok(existsSync(result.packetPath)); + assert.ok(first.packetPath); + assert.ok(existsSync(first.packetPath)); assert.match( - result.markdown, - /^decision_packet_path: records\/openclaw-openclaw\/decision-packets\/321\.json$/m, + first.markdown, + /^decision_packet_path: records\/openclaw-clawsweeper\/decision-packets\/321\.json$/m, ); - assert.match(result.markdown, /^decision_packet_sha256: [a-f0-9]{64}$/m); - const packet = JSON.parse(readFileSync(result.packetPath, "utf8")); - assert.equal(packet.lane, "maintainer_review"); - assert.deepEqual(packet.subject.labels, [ - "clawsweeper:needs-maintainer-review", - "channel: telegram", - ]); + assert.match(first.markdown, /^decision_packet_sha256: [a-f0-9]{64}$/m); + const stored = JSON.parse(readFileSync(first.packetPath, "utf8")); + assert.equal(stored.question, productDecision.question); + + const second = syncDecisionPacketRecord({ + markdown: first.markdown.replace( + /^maintainer_decision: .*$/m, + `maintainer_decision: ${JSON.stringify(emptyMaintainerDecision())}`, + ), + reportPath, + packetsDir, + repoRoot: root, + }); + + assert.equal(second.packet, null); + assert.equal(existsSync(first.packetPath), false); + assert.match(second.markdown, /^decision_packet_path: none$/m); + assert.match(second.markdown, /^decision_packet_sha256: none$/m); } finally { rmSync(root, { recursive: true, force: true }); } }); -function workPlanCandidateReport(overrides = {}) { +function decisionReport(overrides: Record = {}): string { const frontmatter = { number: 321, repository: "openclaw/clawsweeper", type: "issue", - title: "Render work plans", - reviewed_at: new Date().toISOString(), - review_status: "complete", - local_checkout_access: "verified", - decision: "keep_open", + title: JSON.stringify("Render maintainer decision"), + url: "https://github.com/openclaw/clawsweeper/issues/321", + reviewed_at: "2026-06-23T10:00:00.000Z", + item_created_at: "2026-06-20T00:00:00Z", + item_updated_at: "2026-06-21T00:00:00Z", + labels: JSON.stringify([]), + triage_priority: "P2", action_taken: "kept_open", - work_candidate: "queue_fix_pr", - work_status: "candidate", - work_priority: "medium", - work_confidence: "high", - work_likely_files: JSON.stringify(["src/clawsweeper.ts", "test/clawsweeper.test.ts"]), - work_validation: JSON.stringify(["pnpm run check"]), - work_cluster_refs: JSON.stringify(["openclaw/clawsweeper#26"]), ...overrides, }; return `--- ${Object.entries(frontmatter) - .map(([key, value]) => `${key}: ${value}`) + .map(([key, value]) => `${key}: ${String(value)}`) .join("\n")} --- -# #321: Render work plans - -## Summary - -The dashboard has queue_fix_pr candidates but no generated coding plan. - -## Repair Work Prompt - -Render generated plan markdown from existing report fields. +# #321: Render maintainer decision `; } diff --git a/test/decision-parser.test.ts b/test/decision-parser.test.ts index 61e3ccf0ac..b4d896fa09 100644 --- a/test/decision-parser.test.ts +++ b/test/decision-parser.test.ts @@ -394,6 +394,49 @@ test("decision parser enforces required schema-shaped evidence", () => { assert.deepEqual(workCandidate.workClusterRefs, ["#123", "#456"]); }); +test("decision parser keeps maintainer intent model-authored and owner-consistent", () => { + const maintainerDecision = { + required: true, + kind: "product_direction", + question: "Should this configuration contract change?", + rationale: "Both behaviors are technically valid, so maintainer intent is authoritative.", + options: [ + { + title: "Keep compatibility", + body: "Preserve the current contract and close the proposal.", + recommended: true, + }, + { + title: "Adopt the proposal", + body: "Accept the new contract and document the migration.", + recommended: false, + }, + ], + likelyOwner: { + person: "@alice", + reason: "Git history identifies @alice as the feature owner.", + confidence: "high", + }, + }; + + assert.deepEqual( + parseDecision(closeDecision({ maintainerDecision })).maintainerDecision, + maintainerDecision, + ); + assert.throws( + () => + parseDecision( + closeDecision({ + maintainerDecision: { + ...maintainerDecision, + likelyOwner: { ...maintainerDecision.likelyOwner, person: "@not-in-history" }, + }, + }), + ), + /likelyOwner\.person must match decision\.likelyOwners/, + ); +}); + test("decision parser validates typed root-cause clusters", () => { const canonicalRef = "https://github.com/openclaw/openclaw/pull/456"; const canonicalIssueRef = "https://github.com/openclaw/openclaw/issues/456"; diff --git a/test/helpers.ts b/test/helpers.ts index 8adf93157b..be78e14d62 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -82,6 +82,14 @@ export function closeDecision(overrides = {}) { ], risks: [], bestSolution: "Keep the implementation as-is.", + maintainerDecision: { + required: false, + kind: "none", + question: "", + rationale: "", + options: [], + likelyOwner: { person: "", reason: "", confidence: "low" }, + }, triagePriority: "P2", impactLabels: [], maturityLabels: [], From 650a28a2eb8888103af69d7d733f9d8846b82820 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 20:23:16 +0100 Subject: [PATCH 03/10] fix(review): preserve closed packet state --- src/clawsweeper.ts | 8 ++--- test/clawsweeper.test.ts | 72 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 1abe4723ad..96ce86f4ce 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -18017,9 +18017,9 @@ async function applyDecisionsCommand(args: Args): Promise { }); renameSync(path, closedPath); }; - const markApplyChecked = (): void => { + const markApplyChecked = (subjectState: DecisionPacketSubjectState = "open"): void => { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeReportMarkdown(path, markdown); + if (!dryRun) writeReportMarkdown(path, markdown, subjectState); }; const recordApplySkipped = (actionTaken: ActionTaken, reason: string): boolean => { markApplyChecked(); @@ -18071,7 +18071,7 @@ async function applyDecisionsCommand(args: Args): Promise { // inaccessible. Confirm repo access before treating this as an item miss. ghJson(["api", `repos/${targetRepo()}`]); if (syncCommentsOnly) { - markApplyChecked(); + markApplyChecked("closed"); results.push({ number, action: "skipped_already_closed", @@ -18349,7 +18349,7 @@ async function applyDecisionsCommand(args: Args): Promise { return result; }; if (syncCommentsOnly && state !== "open") { - markApplyChecked(); + markApplyChecked("closed"); results.push({ number, action: "skipped_already_closed", reason: `state is ${state}` }); processedCount += 1; maybeLogProgress(`skipped comment sync #${number}: already ${state}`); diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index 0a13060e5e..e7b57e50c7 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -162,6 +162,78 @@ if (args[0] === "api" && /\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { } }); +test("apply-decisions records closed decision packet state during comment-only sync", () => { + const root = mkdtempSync(tmpPrefix); + try { + const itemsDir = join(root, "items"); + const closedDir = join(root, "closed"); + const plansDir = join(root, "plans"); + const reportPath = join(root, "apply-report.json"); + const packetPath = join(root, "decision-packets", "321.json"); + mkdirSync(itemsDir, { recursive: true }); + mkdirSync(plansDir, { recursive: true }); + writeFileSync( + join(itemsDir, "321.md"), + implementedCloseReport({ + action_taken: "skipped_open_closing_pr", + close_reason: "duplicate_or_superseded", + labels: JSON.stringify(["clawsweeper:needs-product-decision"]), + maintainer_decision: JSON.stringify(maintainerDecision), + }), + "utf8", + ); + + const ghMock = ` +const path = process.argv[3] || ""; +if (/\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { + console.log(JSON.stringify([[]])); +} else if (/\\/issues\\/321$/.test(path)) { + console.log(JSON.stringify({ + number: 321, + title: "Render work plans", + html_url: "https://github.com/openclaw/clawsweeper/issues/321", + created_at: "2026-05-01T00:00:00Z", + updated_at: "2026-05-02T00:00:00Z", + closed_at: "2026-05-02T00:00:00Z", + state: "closed", + locked: false, + active_lock_reason: null, + author_association: "CONTRIBUTOR", + user: { login: "reporter" }, + labels: ["clawsweeper:needs-product-decision"], + comments: 0, + pull_request: null + })); +} else { + console.error("unexpected gh args", JSON.stringify(process.argv.slice(2))); + process.exit(1); +} +`; + withMockGh(root, ghMock, () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: ["--sync-comments-only", "--comment-sync-min-age-days", "0"], + }); + }); + + assert.equal(existsSync(join(itemsDir, "321.md")), true); + assert.equal(existsSync(join(closedDir, "321.md")), false); + assert.equal(JSON.parse(readFileSync(packetPath, "utf8")).subject.state, "closed"); + assert.deepEqual(JSON.parse(readFileSync(reportPath, "utf8")), [ + { + number: 321, + action: "skipped_already_closed", + reason: "state is closed", + }, + ]); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("apply-decisions writes decision packets for changed-since-review reports", () => { const root = mkdtempSync(tmpPrefix); try { From 9ef0de5842fba07e0255c2a2f5b18b2565787a63 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 20:39:31 +0100 Subject: [PATCH 04/10] fix(apply): pause required maintainer decisions --- src/clawsweeper.ts | 11 +++++ test/clawsweeper.test.ts | 100 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 96ce86f4ce..5dd3891f78 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -19201,6 +19201,17 @@ async function applyDecisionsCommand(args: Args): Promise { if (!isCloseProposal || !closeReason) { continue; } + const requiredMaintainerDecision = maintainerDecisionFromReport(markdown); + if (requiredMaintainerDecision?.required) { + if ( + markApplySkipped( + "kept_open", + `maintainer decision required: ${requiredMaintainerDecision.question}`, + ) + ) + break; + continue; + } if (closedCount >= limit) break; if (applyKind !== "all" && item.kind !== applyKind) { if (recordApplySkipped("kept_open", `type is ${item.kind}; apply kind is ${applyKind}`)) diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index e7b57e50c7..e83e142b9c 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -309,6 +309,106 @@ if (/\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { } }); +test("apply-decisions keeps required maintainer decisions open", () => { + const root = mkdtempSync(tmpPrefix); + try { + const itemsDir = join(root, "items"); + const closedDir = join(root, "closed"); + const plansDir = join(root, "plans"); + const reportPath = join(root, "apply-report.json"); + const packetPath = join(root, "decision-packets", "321.json"); + mkdirSync(itemsDir, { recursive: true }); + mkdirSync(plansDir, { recursive: true }); + const synced = reportWithSyncedReviewComment( + implementedCloseReport({ + labels: JSON.stringify(["clawsweeper:needs-product-decision"]), + requires_product_decision: "true", + maintainer_decision: JSON.stringify(maintainerDecision), + }), + 321, + ); + writeFileSync(join(itemsDir, "321.md"), synced.report, "utf8"); + const existingComment = { + id: 9321, + html_url: "https://github.com/openclaw/clawsweeper/issues/321#issuecomment-9321", + created_at: "2026-05-01T01:00:00Z", + updated_at: "2026-05-01T01:00:00Z", + user: { login: "clawsweeper[bot]" }, + body: synced.comment, + }; + + const ghMock = ` +const { readFileSync } = require("fs"); +const rawArgs = process.argv.slice(2); +const args = rawArgs[0] === "--repo" ? rawArgs.slice(2) : rawArgs; +const path = args.includes("-i") ? args[args.indexOf("-i") + 1] : args[1] || ""; +if (/\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { + console.log(JSON.stringify([[${JSON.stringify(existingComment)}]])); +} else if (/\\/issues\\/comments\\/9321$/.test(path) && args.includes("--method")) { + const inputPath = args[args.indexOf("--input") + 1]; + const body = JSON.parse(readFileSync(inputPath, "utf8")).body; + console.log(JSON.stringify({ ...${JSON.stringify(existingComment)}, body })); +} else if (/\\/issues\\/321$/.test(path)) { + console.log(JSON.stringify({ + number: 321, + title: "Render work plans", + html_url: "https://github.com/openclaw/clawsweeper/issues/321", + created_at: "2026-05-01T00:00:00Z", + updated_at: "2026-05-01T00:00:00Z", + closed_at: null, + state: "open", + locked: false, + active_lock_reason: null, + author_association: "CONTRIBUTOR", + user: { login: "reporter" }, + labels: ["clawsweeper:needs-product-decision"], + comments: 1, + pull_request: null + })); +} else if (/\\/issues\\/321\\/timeline/.test(path)) { + console.log(JSON.stringify([[]])); +} else if (args[0] === "issue" && args[1] === "view") { + console.log(JSON.stringify({ closedByPullRequestsReferences: [] })); +} else if (args[0] === "issue" && args[1] === "close") { + console.error("required maintainer decision reached close mutation"); + process.exit(1); +} else if (args[0] === "label" || args[0] === "issue") { + console.log(""); +} else { + console.error("unexpected gh args", JSON.stringify(args)); + process.exit(1); +} +`; + withMockGh(root, ghMock, () => { + runApplyDecisionsForTest({ + itemsDir, + closedDir, + plansDir, + reportPath, + extraArgs: ["--processed-limit", "2"], + }); + }); + + assert.equal(existsSync(join(itemsDir, "321.md")), true); + assert.equal(existsSync(join(closedDir, "321.md")), false); + assert.equal(JSON.parse(readFileSync(packetPath, "utf8")).subject.state, "open"); + assert.deepEqual(JSON.parse(readFileSync(reportPath, "utf8")), [ + { + number: 321, + action: "review_comment_synced", + reason: "updated durable Codex review comment", + }, + { + number: 321, + action: "kept_open", + reason: `maintainer decision required: ${maintainerDecision.question}`, + }, + ]); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("apply-decisions skips advisory labels for failed or stale kept-open reports", () => { const root = mkdtempSync(tmpPrefix); try { From 8c28836f863b4c16dc9c939ba98604c940d98939 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 20:43:19 +0100 Subject: [PATCH 05/10] fix(review): surface required close decisions --- src/clawsweeper.ts | 3 ++- test/clawsweeper.test.ts | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 5dd3891f78..c85bbef9b5 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -15376,8 +15376,9 @@ export function renderReviewCommentFromReport( options: ReviewCommentRenderOptions = {}, ): string { const decision = frontMatterValue(markdown, "decision"); + const requiresMaintainerDecision = maintainerDecisionFromReport(markdown)?.required === true; const body = - decision === "close" && reason !== "none" + decision === "close" && reason !== "none" && !requiresMaintainerDecision ? renderCloseCommentFromReport(markdown, reason) : renderKeepOpenCommentFromReport(markdown, options); const markers = reviewAutomationMarkersFromReport(markdown); diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index e83e142b9c..7c59d4e20f 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -93,6 +93,23 @@ test("review comments include a compact maintainer decision packet block", () => assert.match(comment, /Likely owner: @owner/); }); +test("close proposals that require maintainer decisions render as kept open", () => { + const comment = renderReviewCommentFromReport( + implementedCloseReport({ + labels: JSON.stringify(["clawsweeper:needs-product-decision"]), + requires_product_decision: "true", + maintainer_decision: JSON.stringify(maintainerDecision), + }), + "implemented_or_shipped", + ); + + assert.match(comment, /\*\*Maintainer decision needed\*\*/); + assert.match(comment, /Should this product contract be accepted\?/); + assert.match(comment, /Accept the contract \(recommended\)/); + assert.match(comment, /Likely owner: @owner/); + assert.doesNotMatch(comment, /Closing this issue/); +}); + test("apply-decisions archives live-closed skipped records without reopening close gates", () => { const root = mkdtempSync(tmpPrefix); try { From 388bbbd74f17edc1904feedbe9261153a9baa10c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 20:54:19 +0100 Subject: [PATCH 06/10] fix(apply): reject malformed decision intent --- src/clawsweeper.ts | 13 +++++++++++- src/decision-packets.ts | 6 ++++-- test/clawsweeper.test.ts | 37 +++++++++++++++++++++++++++++++++++ test/decision-packets.test.ts | 15 ++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index c85bbef9b5..679781950a 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -17992,6 +17992,7 @@ async function applyDecisionsCommand(args: Args): Promise { let storedHash = frontMatterValue(markdown, "item_snapshot_hash"); let storedUpdatedAt = frontMatterValue(markdown, "item_updated_at"); const storedAuthorAssociation = frontMatterValue(markdown, "author_association"); + let requiredMaintainerDecision: MaintainerDecision | null; const shouldProbeClosedState = shouldProbeClosedStateReport(markdown); const isRetryableSkippedClose = isRetryableCloseSkipReport(markdown); const isUpgradedCloseCandidate = @@ -18038,6 +18039,17 @@ async function applyDecisionsCommand(args: Args): Promise { "kept_open", `GitHub rejected ${labelKind} label sync with Requires authentication`, ); + try { + requiredMaintainerDecision = maintainerDecisionFromReport(markdown); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + const reason = `invalid maintainer_decision: ${detail}`; + results.push({ number, action: "kept_open", reason }); + processedCount += 1; + maybeLogProgress(`skipped #${number}: ${reason}`); + if (processedCount >= processedLimit) break; + continue; + } if (!verifiedLocalCheckout && !shouldProbeClosedState) { if (markApplySkipped("kept_open", "review lacks verified local checkout access")) break; continue; @@ -19202,7 +19214,6 @@ async function applyDecisionsCommand(args: Args): Promise { if (!isCloseProposal || !closeReason) { continue; } - const requiredMaintainerDecision = maintainerDecisionFromReport(markdown); if (requiredMaintainerDecision?.required) { if ( markApplySkipped( diff --git a/src/decision-packets.ts b/src/decision-packets.ts index a189b63f8d..e07d80ec37 100644 --- a/src/decision-packets.ts +++ b/src/decision-packets.ts @@ -162,11 +162,13 @@ export function emptyMaintainerDecision(): MaintainerDecision { export function maintainerDecisionFromReport(markdown: string): MaintainerDecision | null { const raw = frontMatter(markdown).maintainer_decision; if (!raw || raw === "none" || raw === "unknown") return null; + let parsed: unknown; try { - return parseMaintainerDecision(JSON.parse(raw), "maintainer_decision"); + parsed = JSON.parse(raw); } catch { - return null; + throw new Error("maintainer_decision must contain valid JSON"); } + return parseMaintainerDecision(parsed, "maintainer_decision"); } export function buildDecisionPacketFromReport( diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index 7c59d4e20f..ecdcd8cf84 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -426,6 +426,43 @@ if (/\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { } }); +test("apply-decisions makes no GitHub mutation for malformed maintainer decisions", () => { + const root = mkdtempSync(tmpPrefix); + try { + const itemsDir = join(root, "items"); + const closedDir = join(root, "closed"); + const plansDir = join(root, "plans"); + const reportPath = join(root, "apply-report.json"); + mkdirSync(itemsDir, { recursive: true }); + mkdirSync(plansDir, { recursive: true }); + writeFileSync( + join(itemsDir, "321.md"), + implementedCloseReport({ maintainer_decision: "{" }), + "utf8", + ); + + const ghMock = ` +console.error("malformed maintainer decision reached GitHub"); +process.exit(1); +`; + withMockGh(root, ghMock, () => { + runApplyDecisionsForTest({ itemsDir, closedDir, plansDir, reportPath }); + }); + + assert.equal(existsSync(join(itemsDir, "321.md")), true); + assert.equal(existsSync(join(closedDir, "321.md")), false); + assert.deepEqual(JSON.parse(readFileSync(reportPath, "utf8")), [ + { + number: 321, + action: "kept_open", + reason: "invalid maintainer_decision: maintainer_decision must contain valid JSON", + }, + ]); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("apply-decisions skips advisory labels for failed or stale kept-open reports", () => { const root = mkdtempSync(tmpPrefix); try { diff --git a/test/decision-packets.test.ts b/test/decision-packets.test.ts index 2e5baa8c12..9bb80fa106 100644 --- a/test/decision-packets.test.ts +++ b/test/decision-packets.test.ts @@ -6,6 +6,7 @@ import test from "node:test"; import { buildDecisionPacketFromReport, emptyMaintainerDecision, + maintainerDecisionFromReport, parseMaintainerDecision, syncDecisionPacketRecord, } from "../dist/decision-packets.js"; @@ -103,6 +104,20 @@ test("maintainer decision validation requires one recommendation and an exact ow ); }); +test("present malformed maintainer decisions fail closed", () => { + assert.throws( + () => maintainerDecisionFromReport(decisionReport({ maintainer_decision: "{" })), + /must contain valid JSON/, + ); + assert.throws( + () => + maintainerDecisionFromReport( + decisionReport({ maintainer_decision: JSON.stringify({ required: true }) }), + ), + /maintainer_decision/, + ); +}); + test("decision packets prefer reconciled subject state", () => { const packet = buildDecisionPacketFromReport( decisionReport({ From d98ca463ef01f7de435d0ccc9e2a73916be71db1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 20:59:03 +0100 Subject: [PATCH 07/10] fix(review): propagate maintainer decision state --- src/clawsweeper.ts | 4 +++- test/clawsweeper.test.ts | 8 +++++++- test/pr-proof-automation.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 679781950a..4b13142d64 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -2852,7 +2852,6 @@ function normalizeDecisionForItem( ...decision, reviewFindings, bestSolution: CLEAN_OPENCLAW_PR_REVIEW_NEXT_STEP, - maintainerDecision: emptyMaintainerDecision(), triagePriority: decision.triagePriority, mergeRiskOptions: decision.mergeRiskOptions, labelJustifications: decision.labelJustifications, @@ -15840,6 +15839,9 @@ export function reviewAutomationMarkersFromReport(markdown: string): string { return markers.join("\n"); }; + if (maintainerDecisionFromReport(markdown)?.required) { + return humanReviewMarkers(); + } if (frontMatterValue(markdown, "review_status") === "failed") { return humanReviewMarkers(); } diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index ecdcd8cf84..75b9450ddb 100644 --- a/test/clawsweeper.test.ts +++ b/test/clawsweeper.test.ts @@ -96,6 +96,9 @@ test("review comments include a compact maintainer decision packet block", () => test("close proposals that require maintainer decisions render as kept open", () => { const comment = renderReviewCommentFromReport( implementedCloseReport({ + repository: "openclaw/openclaw", + type: "pull_request", + pull_head_sha: "abc123def456", labels: JSON.stringify(["clawsweeper:needs-product-decision"]), requires_product_decision: "true", maintainer_decision: JSON.stringify(maintainerDecision), @@ -107,7 +110,10 @@ test("close proposals that require maintainer decisions render as kept open", () assert.match(comment, /Should this product contract be accepted\?/); assert.match(comment, /Accept the contract \(recommended\)/); assert.match(comment, /Likely owner: @owner/); - assert.doesNotMatch(comment, /Closing this issue/); + assert.match(comment, /clawsweeper-verdict:needs-human/); + assert.doesNotMatch(comment, /Closing this PR/); + assert.doesNotMatch(comment, /clawsweeper-verdict:close/); + assert.doesNotMatch(comment, /clawsweeper-action:close-required/); }); test("apply-decisions archives live-closed skipped records without reopening close gates", () => { diff --git a/test/pr-proof-automation.test.ts b/test/pr-proof-automation.test.ts index be47ce7844..d55a2887ff 100644 --- a/test/pr-proof-automation.test.ts +++ b/test/pr-proof-automation.test.ts @@ -367,8 +367,33 @@ Full review comments: }); test("OpenClaw contributor changelog-entry findings are normalized", () => { + const maintainerDecision = { + required: true, + kind: "product_direction", + question: "Should this public behavior become the supported contract?", + rationale: "The patch is correct, but the behavior still needs an explicit product choice.", + options: [ + { + title: "Accept the behavior", + body: "Adopt and document this behavior as the supported contract.", + recommended: true, + }, + { + title: "Keep the existing behavior", + body: "Decline this contract change while retaining current behavior.", + recommended: false, + }, + ], + likelyOwner: { + person: "@alice", + reason: "Recent implementation history identifies Alice as the likely product owner.", + confidence: "high", + }, + } as const; const decision = parseDecision( changelogReviewDecision({ + maintainerDecision, + requiresProductDecision: true, realBehaviorProof: { status: "sufficient", summary: "Terminal output from a real OpenClaw checkout shows the changed behavior.", @@ -394,6 +419,7 @@ test("OpenClaw contributor changelog-entry findings are normalized", () => { assert.deepEqual(decision.prRating.nextSteps, []); assert.equal(decision.workCandidate, "none"); assert.equal(decision.workReason, ""); + assert.deepEqual(decision.maintainerDecision, maintainerDecision); const comment = renderReviewCommentFromReport( `${reportFrontMatter({ From e428ee4b688613ed42fdd652b038bc46e8f1f5f3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 21:04:34 +0100 Subject: [PATCH 08/10] fix(apply): gate paired maintainer decisions --- src/clawsweeper.ts | 2 ++ test/apply-same-author-pair-close.test.ts | 37 +++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 4b13142d64..c3caa9e70e 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -18154,6 +18154,7 @@ async function applyDecisionsCommand(args: Args): Promise { }; const sameAuthorPairStartCloseable = new Map(); const currentCloseGatesPassed = (): boolean => { + if (requiredMaintainerDecision?.required) return false; if (!closeReason || !closeReasonEnabled(closeReason, applyCloseReasons)) return false; if (needsReviewCommentSync) return false; if ( @@ -18326,6 +18327,7 @@ async function applyDecisionsCommand(args: Args): Promise { result = counterpartState === "open" && counterpartItem.kind === counterpartKind && + !maintainerDecisionFromReport(counterpartMarkdown)?.required && applyBlockingProtectedLabels(counterpartItem.labels, counterpartReason).length === 0 && (isVerifiedFixedCloseReason(counterpartReason) || diff --git a/test/apply-same-author-pair-close.test.ts b/test/apply-same-author-pair-close.test.ts index d1ca1df6a8..7cd308b7bd 100644 --- a/test/apply-same-author-pair-close.test.ts +++ b/test/apply-same-author-pair-close.test.ts @@ -652,7 +652,7 @@ if (args[0] === "api" && args[1] === "-i" && /\\/issues\\/(320|321)\\/timeline(? } }); -test("apply-decisions keeps same-author PR blocked when counterpart comment needs sync", () => { +test("apply-decisions keeps same-author PR blocked when counterpart needs a maintainer decision", () => { const root = mkdtempSync(tmpPrefix); try { const itemsDir = join(root, "items"); @@ -669,6 +669,29 @@ test("apply-decisions keeps same-author PR blocked when counterpart comment need title: "Paired issue", author: "reporter", action_taken: "skipped_same_author_pair", + maintainer_decision: JSON.stringify({ + required: true, + kind: "product_direction", + question: "Should this issue be closed with its paired PR?", + rationale: "Closing the pair would settle a product contract that maintainers must own.", + options: [ + { + title: "Keep the pair open", + body: "Retain both items until the product contract is decided.", + recommended: true, + }, + { + title: "Close the pair", + body: "Accept the proposed contract and close both items.", + recommended: false, + }, + ], + likelyOwner: { + person: "@owner", + reason: "Recent history identifies this maintainer as the contract owner.", + confidence: "high", + }, + }), }), 320, "implemented_on_main", @@ -689,6 +712,7 @@ test("apply-decisions keeps same-author PR blocked when counterpart comment need writeFileSync(join(itemsDir, "321.md"), pullSynced.report, "utf8"); const ghMock = ` +const issueComment = ${JSON.stringify(issueSynced.comment)}; const pullComment = ${JSON.stringify(pullSynced.comment)}; const rawArgs = process.argv.slice(2); const args = rawArgs[0] === "--repo" ? rawArgs.slice(2) : rawArgs; @@ -696,7 +720,14 @@ const path = args[1] || ""; if (args[0] === "api" && args[1] === "-i" && /\\/issues\\/(320|321)\\/timeline(?:\\?|$)/.test(args[2] || "")) { console.log("HTTP/2 200\\n\\n[]"); } else if (args[0] === "api" && /\\/issues\\/320\\/comments(?:\\?|$)/.test(path)) { - console.log(JSON.stringify([[]])); + console.log(JSON.stringify([[{ + id: 9320, + html_url: "https://github.com/openclaw/openclaw/issues/320#issuecomment-9320", + created_at: "2026-05-01T01:00:00Z", + updated_at: "2026-05-01T01:00:00Z", + user: { login: "clawsweeper[bot]" }, + body: issueComment + }]])); } else if (args[0] === "api" && /\\/issues\\/321\\/comments(?:\\?|$)/.test(path)) { console.log(JSON.stringify([[{ id: 9321, @@ -723,7 +754,7 @@ if (args[0] === "api" && args[1] === "-i" && /\\/issues\\/(320|321)\\/timeline(? author_association: "CONTRIBUTOR", user: { login: "reporter" }, labels: [], - comments: 0, + comments: 1, pull_request: null })); } else if (args[0] === "api" && /\\/issues\\/321$/.test(path)) { From afbf0f11a797c203f6b0101155ca6e907722210c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 21:08:34 +0100 Subject: [PATCH 09/10] fix(apply): fail closed on invalid pair intent --- src/clawsweeper.ts | 5 ++++- src/decision-packets.ts | 8 ++++++++ test/decision-packets.test.ts | 15 +++++++++++---- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index c3caa9e70e..e48eae3de3 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -109,6 +109,7 @@ import { import { escapeRegExp, safeOutputTail, trimMiddle, truncateText } from "./clawsweeper-text.js"; import { emptyMaintainerDecision, + maintainerDecisionBlocksClose, maintainerDecisionFromReport, parseMaintainerDecision, renderDecisionPacketPublicBlock, @@ -18233,12 +18234,15 @@ async function applyDecisionsCommand(args: Args): Promise { const counterpartEntry = openFileEntryByNumber.get(counterpartNumber); if (counterpartEntry) { const counterpartMarkdown = readFileSync(counterpartEntry.path, "utf8"); + const counterpartMaintainerDecisionBlocked = + maintainerDecisionBlocksClose(counterpartMarkdown); const counterpartRepo = markdownRepository(counterpartMarkdown, counterpartEntry.path); const counterpartReason = reportCloseReason(counterpartMarkdown); if ( counterpartRepo === repo && reportItemKind(counterpartMarkdown) === counterpartKind && counterpartReason && + !counterpartMaintainerDecisionBlocked && closeReasonEnabled(counterpartReason, applyCloseReasons) && isApplyCloseCandidateReport(counterpartMarkdown) && hasAutoCloseAllowedMetadata(counterpartMarkdown) && @@ -18327,7 +18331,6 @@ async function applyDecisionsCommand(args: Args): Promise { result = counterpartState === "open" && counterpartItem.kind === counterpartKind && - !maintainerDecisionFromReport(counterpartMarkdown)?.required && applyBlockingProtectedLabels(counterpartItem.labels, counterpartReason).length === 0 && (isVerifiedFixedCloseReason(counterpartReason) || diff --git a/src/decision-packets.ts b/src/decision-packets.ts index e07d80ec37..4aaba68aac 100644 --- a/src/decision-packets.ts +++ b/src/decision-packets.ts @@ -171,6 +171,14 @@ export function maintainerDecisionFromReport(markdown: string): MaintainerDecisi return parseMaintainerDecision(parsed, "maintainer_decision"); } +export function maintainerDecisionBlocksClose(markdown: string): boolean { + try { + return maintainerDecisionFromReport(markdown)?.required === true; + } catch { + return true; + } +} + export function buildDecisionPacketFromReport( markdown: string, options: DecisionPacketBuildOptions = {}, diff --git a/test/decision-packets.test.ts b/test/decision-packets.test.ts index 9bb80fa106..cde0397b91 100644 --- a/test/decision-packets.test.ts +++ b/test/decision-packets.test.ts @@ -6,6 +6,7 @@ import test from "node:test"; import { buildDecisionPacketFromReport, emptyMaintainerDecision, + maintainerDecisionBlocksClose, maintainerDecisionFromReport, parseMaintainerDecision, syncDecisionPacketRecord, @@ -105,10 +106,8 @@ test("maintainer decision validation requires one recommendation and an exact ow }); test("present malformed maintainer decisions fail closed", () => { - assert.throws( - () => maintainerDecisionFromReport(decisionReport({ maintainer_decision: "{" })), - /must contain valid JSON/, - ); + const malformed = decisionReport({ maintainer_decision: "{" }); + assert.throws(() => maintainerDecisionFromReport(malformed), /must contain valid JSON/); assert.throws( () => maintainerDecisionFromReport( @@ -116,6 +115,14 @@ test("present malformed maintainer decisions fail closed", () => { ), /maintainer_decision/, ); + assert.equal(maintainerDecisionBlocksClose(malformed), true); + assert.equal( + maintainerDecisionBlocksClose( + decisionReport({ maintainer_decision: JSON.stringify(productDecision) }), + ), + true, + ); + assert.equal(maintainerDecisionBlocksClose(decisionReport()), false); }); test("decision packets prefer reconciled subject state", () => { From 927668a34ea3721c9d78cff9090f9455849dad00 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 21:27:07 +0100 Subject: [PATCH 10/10] docs(vision): codify model-authored decisions --- VISION.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/VISION.md b/VISION.md index a8e1421c14..63c9388ac8 100644 --- a/VISION.md +++ b/VISION.md @@ -96,6 +96,14 @@ Durable records, dashboard output, labels, and comments are operational history. They should be reproducible, link back to source records, and never expose secrets, private artifacts, or unredacted security-sensitive details. +### 7. Models propose maintainer decisions; deterministic code enforces them + +Models decide whether a maintainer choice exists and produce the question, +rationale, options, recommendation, and likely owner. Deterministic code +validates and persists that structured intent, refreshes live state, and blocks +unsafe or stale mutations. It must fail closed when required intent is missing +or malformed; it must not invent product judgment from hard-coded heuristics. + ## Security ClawSweeper is a maintenance automation tool, not the security response owner.