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 73b9a490aa..964677880a 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,15 @@ 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 Codex-authored decision packet JSON at +`records//decision-packets/.json` for reports that need a +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, repair ledgers, and the rendered dashboard. The state repo `main` branch is the 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. 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 f7810d80c5..e48eae3de3 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -107,6 +107,16 @@ import { type Args, } from "./clawsweeper-args.js"; import { escapeRegExp, safeOutputTail, trimMiddle, truncateText } from "./clawsweeper-text.js"; +import { + emptyMaintainerDecision, + maintainerDecisionBlocksClose, + maintainerDecisionFromReport, + parseMaintainerDecision, + renderDecisionPacketPublicBlock, + syncDecisionPacketRecord, + type DecisionPacketSubjectState, + type MaintainerDecision, +} from "./decision-packets.js"; import { appendReviewHistoryCycle, neutralizeReviewControlMarkers, @@ -125,6 +135,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, @@ -508,6 +522,7 @@ interface Decision { likelyOwners: LikelyOwner[]; risks: string[]; bestSolution: string; + maintainerDecision: MaintainerDecision; triagePriority: TriagePriority; impactLabels: ImpactLabelName[]; mergeRiskLabels: MergeRiskLabelName[]; @@ -1750,6 +1765,7 @@ const DECISION_SCHEMA_KEYS = new Set([ "likelyOwners", "risks", "bestSolution", + "maintainerDecision", "triagePriority", "impactLabels", "mergeRiskLabels", @@ -1871,6 +1887,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", @@ -2032,6 +2049,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`; @@ -2614,6 +2672,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); @@ -3118,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, @@ -3215,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); } @@ -7460,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: [], @@ -12954,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, @@ -15176,6 +15253,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) { @@ -15295,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); @@ -15758,6 +15840,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(); } @@ -16390,6 +16475,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}`, @@ -16633,6 +16748,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._"; @@ -16720,6 +16836,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)} @@ -16810,6 +16927,10 @@ ${options.decision.changeSummary} ${bestSolution} +## ${REVIEW_SECTIONS.maintainerDecision} + +${maintainerDecision} + ## ${REVIEW_SECTIONS.reproductionAssessment} ${reproductionAssessment} @@ -17729,6 +17850,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 +17932,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 || @@ -17850,6 +17995,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 = @@ -17866,17 +18012,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 => { + const markApplyChecked = (subjectState: DecisionPacketSubjectState = "open"): void => { markdown = replaceFrontMatterValue(markdown, "apply_checked_at", new Date().toISOString()); - if (!dryRun) writeFileSync(path, markdown, "utf8"); + if (!dryRun) writeReportMarkdown(path, markdown, subjectState); }; const recordApplySkipped = (actionTaken: ActionTaken, reason: string): boolean => { markApplyChecked(); @@ -17894,6 +18042,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; @@ -17928,7 +18087,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", @@ -17996,6 +18155,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 ( @@ -18074,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) && @@ -18206,7 +18369,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}`); @@ -18513,7 +18676,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 +18828,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 +18849,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 +19123,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 +19174,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 +19189,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 +19207,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", @@ -19058,6 +19221,16 @@ async function applyDecisionsCommand(args: Args): Promise { if (!isCloseProposal || !closeReason) { continue; } + 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}`)) @@ -19880,6 +20053,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 +20088,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 +20108,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 +20540,7 @@ function reconcileFolders(options: { itemsDir: string; closedDir: string; plansDir?: string; + decisionPacketsDir?: string; maxPages?: number; dryRun?: boolean; fetchClosedAt?: boolean; @@ -20368,6 +20550,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 +20597,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 +20618,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 +20658,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 +20667,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..4aaba68aac --- /dev/null +++ b/src/decision-packets.ts @@ -0,0 +1,440 @@ +import { createHash } from "node:crypto"; +import { existsSync, mkdirSync, unlinkSync, writeFileSync } from "node:fs"; +import { dirname, relative } from "node:path"; + +export type MaintainerDecisionKind = + | "none" + | "product_direction" + | "security_boundary" + | "proof_sufficiency" + | "release_inclusion" + | "merge_risk" + | "manual_review"; + +export type MaintainerDecisionConfidence = "high" | "medium" | "low"; +export type DecisionPacketPriority = "P0" | "P1" | "P2" | "P3" | "none"; +export type DecisionPacketSubjectState = "open" | "closed" | "merged"; + +export interface MaintainerDecisionOption { + title: string; + body: string; + recommended: boolean; +} + +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 { + 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: Exclude; + priority: DecisionPacketPriority; + question: string; + rationale: string; + options: MaintainerDecisionOption[]; + recommendation: MaintainerDecisionOption; + likelyOwner: MaintainerDecisionOwner; + 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 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; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error("maintainer_decision must contain valid JSON"); + } + 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 = {}, +): DecisionPacket | null { + const frontmatter = frontMatter(markdown); + const decision = maintainerDecisionFromReport(markdown); + const repo = frontmatter.repository; + const kind = frontmatter.type; + const number = numberValue(frontmatter.number); + 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 createdAt = knownValue(frontmatter.item_created_at); + const subjectUpdatedAt = + knownValue(frontmatter.current_item_updated_at) ?? knownValue(frontmatter.item_updated_at); + 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 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, + generatedAt, + updatedAt, + subject: { + repo, + kind, + number, + title: frontmatter.title ?? `#${number}`, + url, + state: options.subjectState ?? stateFromReport(frontmatter), + labels: stringArrayValue(frontmatter.labels), + ...(createdAt ? { createdAt } : {}), + ...(subjectUpdatedAt ? { updatedAt: subjectUpdatedAt } : {}), + ...(stateChangedAt ? { stateChangedAt } : {}), + ...(headSha ? { headSha } : {}), + }, + 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 options = packet.options.map( + (option) => + ` - **${option.title}${option.recommended ? " (recommended)" : ""}:** ${option.body}`, + ); + return [ + `- 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 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 number = numberValue(frontMatter(options.markdown).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 = createHash("sha256").update(json).digest("hex"); + return { + markdown: replacePacketFrontmatter( + options.markdown, + repoRelativePath(options.repoRoot, packetPath), + packetSha256, + ), + packet, + packetPath, + packetSha256, + }; +} + +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 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 objectValue(value: unknown, path: string): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error(`${path} must be an object`); + } + return value as Record; +} + +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 stringValue(value: unknown, path: string): string { + if (typeof value !== "string") throw new Error(`${path} must be a string`); + return value; +} + +function booleanValue(value: unknown, path: string): boolean { + if (typeof value !== "boolean") throw new Error(`${path} must be a boolean`); + return value; +} + +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 values: Record = {}; + for (const line of (match?.[1] ?? "").split(/\r?\n/)) { + const separator = line.indexOf(":"); + if (separator <= 0) continue; + values[line.slice(0, separator).trim()] = unquote(line.slice(separator + 1).trim()); + } + return values; +} + +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 { + 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 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 === "none" || value === "unknown") return []; + try { + const parsed: unknown = JSON.parse(value); + if (Array.isArray(parsed)) + return parsed.filter((entry): entry is string => typeof entry === "string"); + } catch { + // Legacy report labels were comma separated. + } + return value + .split(",") + .map((entry) => entry.trim()) + .filter(Boolean); +} + +function knownValue(value: string | undefined): string | undefined { + return value && value !== "none" && value !== "unknown" ? value : undefined; +} + +function priorityValue(value: string | undefined): DecisionPacketPriority { + return value === "P0" || value === "P1" || value === "P2" || value === "P3" ? value : "none"; +} + +function stateFromReport(frontmatter: Record): DecisionPacketSubjectState { + if ( + frontmatter.current_state === "open" || + frontmatter.current_state === "closed" || + frontmatter.current_state === "merged" + ) { + return frontmatter.current_state; + } + return frontmatter.action_taken === "closed" || + frontmatter.action_taken === "skipped_already_closed" + ? "closed" + : "open"; +} + +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 repoRelativePath(repoRoot: string, path: string): string { + return relative(repoRoot, path).replace(/\\/g, "/"); +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} 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)) { diff --git a/test/clawsweeper.test.ts b/test/clawsweeper.test.ts index 19b1d0794b..75b9450ddb 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,71 @@ 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({ + decision: "keep_open", + 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, /Should this product contract be accepted\?/); + assert.match(comment, /Accept the contract \(recommended\)/); + assert.match(comment, /Likely owner: @owner/); +}); + +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), + }), + "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.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", () => { const root = mkdtempSync(tmpPrefix); try { @@ -119,6 +185,290 @@ 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 { + 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", + maintainer_decision: JSON.stringify(maintainerDecision), + 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 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 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 new file mode 100644 index 0000000000..cde0397b91 --- /dev/null +++ b/test/decision-packets.test.ts @@ -0,0 +1,208 @@ +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, + emptyMaintainerDecision, + maintainerDecisionBlocksClose, + maintainerDecisionFromReport, + parseMaintainerDecision, + syncDecisionPacketRecord, +} from "../dist/decision-packets.js"; +import { tmpPrefix } from "./helpers.ts"; + +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: JSON.stringify("config.patch redacted array write"), + url: "https://github.com/openclaw/openclaw/pull/81234", + 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", + maintainer_decision: JSON.stringify(productDecision), + }); + + 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_direction"); + assert.equal(packet.priority, "P1"); + 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.equal(packet.updatedAt, "2026-06-23T01:00:00Z"); +}); + +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.equal(buildDecisionPacketFromReport(report), null); +}); + +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/, + ); +}); + +test("present malformed maintainer decisions fail closed", () => { + const malformed = decisionReport({ maintainer_decision: "{" }); + assert.throws(() => maintainerDecisionFromReport(malformed), /must contain valid JSON/); + assert.throws( + () => + maintainerDecisionFromReport( + decisionReport({ maintainer_decision: JSON.stringify({ required: true }) }), + ), + /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", () => { + const packet = buildDecisionPacketFromReport( + decisionReport({ + action_taken: "kept_open", + current_state: "closed", + maintainer_decision: JSON.stringify(productDecision), + }), + { generatedAt: "2026-06-23T12:00:00.000Z" }, + ); + + assert.ok(packet); + assert.equal(packet.subject.state, "closed"); +}); + +test("decision packet sync writes pointers and removes stale generated state", () => { + const root = mkdtempSync(tmpPrefix); + try { + 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, + generatedAt: "2026-06-23T12:00:00.000Z", + subjectState: "open", + }); + + assert.ok(first.packetPath); + assert.ok(existsSync(first.packetPath)); + assert.match( + first.markdown, + /^decision_packet_path: records\/openclaw-clawsweeper\/decision-packets\/321\.json$/m, + ); + 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 decisionReport(overrides: Record = {}): string { + const frontmatter = { + number: 321, + repository: "openclaw/clawsweeper", + type: "issue", + 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", + ...overrides, + }; + return `--- +${Object.entries(frontmatter) + .map(([key, value]) => `${key}: ${String(value)}`) + .join("\n")} +--- + +# #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: [], 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({