fix(engine): require executable calibrated pr repairs

This commit is contained in:
Vincent Koc 2026-04-27 01:45:06 -07:00
parent 4edd9b2141
commit 8ffde172f9
No known key found for this signature in database
11 changed files with 69 additions and 5 deletions

View File

@ -30,6 +30,11 @@ address human and review-bot comments, run Codex `/review`, run
only with concrete proof that the PR is nonsense, crosses a real security or
privilege boundary, or cannot be repaired inside the job window.
If that calibrated canonical PR is not merge-ready in the worker result, emit
`fix_needed` and `build_fix_artifact` with `repair_strategy:
repair_contributor_branch` and the canonical PR URL in `source_prs`. A
`keep_canonical` note saying the executor should repair it is not executable.
Every merge action must include `merge_preflight` proving security clearance,
resolved comments, resolved bot comments, passed Codex `/review`, addressed
review findings, and validation commands. Missing proof blocks merge.

View File

@ -31,3 +31,8 @@ Use `needs_human` only when the unresolved decision is genuinely a maintainer
judgment call after quarantine. False positives are still cheaper than
accidentally routing security work through backlog-cleanup automation, but false
positives should be scoped to the item, not the whole cluster.
If a job explicitly lists `security_override_refs`, those exact refs have a
maintainer-approved false-positive override. Continue to require normal review,
validation, and merge gates for them, but do not route them solely because a
comment, bot review, file path, or label contains security-shaped words.

View File

@ -44,6 +44,8 @@ cluster_refs:
- "#72619"
security_policy: central_security_only
security_sensitive: false
security_override_refs:
- "#72619"
allow_instant_close: true
allow_fix_pr: true
allow_merge: true

View File

@ -48,6 +48,8 @@ cluster_refs:
- "#72614"
security_policy: central_security_only
security_sensitive: false
security_override_refs:
- "#72614"
allow_instant_close: true
allow_fix_pr: true
allow_merge: true

View File

@ -44,6 +44,8 @@ cluster_refs:
- "#72659"
security_policy: central_security_only
security_sensitive: false
security_override_refs:
- "#72659"
allow_instant_close: true
allow_fix_pr: true
allow_merge: true

View File

@ -40,6 +40,8 @@ cluster_refs:
- "#72658"
security_policy: central_security_only
security_sensitive: false
security_override_refs:
- "#72658"
allow_instant_close: true
allow_fix_pr: true
allow_merge: true

View File

@ -38,6 +38,8 @@ cluster_refs:
- "#72662"
security_policy: central_security_only
security_sensitive: false
security_override_refs:
- "#72662"
allow_instant_close: true
allow_fix_pr: true
allow_merge: true

View File

@ -40,6 +40,8 @@ cluster_refs:
- "#72660"
security_policy: central_security_only
security_sensitive: false
security_override_refs:
- "#72660"
allow_instant_close: true
allow_fix_pr: true
allow_merge: true

View File

@ -7,6 +7,7 @@ Scope:
- Start only from refs in the job file and refs linked from those item bodies, comments, review threads, closing refs, commits, or PR descriptions.
- Do not run broad GitHub search unless the job explicitly says so.
- If the job includes `maintainer_calibration`, treat it as an explicit maintainer decision for that cluster. Use it to avoid stale `needs_human` outcomes, but do not bypass the normal merge gates, security boundary, review comments, Codex `/review`, or validation requirements.
- For a maintainer-calibrated open canonical PR that is not merge-ready yet, do not return only `keep_canonical`. Emit `fix_needed` plus `build_fix_artifact` with `status: "planned"`, `repair_strategy: "repair_contributor_branch"`, and `source_prs` containing that PR URL so the executor can rebase, fix, review, and push the existing PR branch.
- If any hydrated item is security-sensitive, quarantine that item with `route_security` and route it to central OpenClaw security triage. Do not mutate that item. Continue classifying unrelated non-security items, duplicate pairs, provider gaps, and ordinary bugs.
- Use the provided cluster preflight artifact and fix artifact as your starting inventory. It should include hydrated issue comments, PR review summaries, inline PR review comments, check state, merge state, touched files, and linked refs.
- Treat closed context refs as evidence, not targets. Do not emit close actions for them.
@ -71,6 +72,7 @@ Merge and post-merge close:
- Recommend `merge_canonical` only when security-sensitive concerns are cleared, all actionable PR comments and review threads are resolved, review state, conflicts, changelog, and changed-surface validation are clean, and the job permits merge. Unrelated flaky main CI does not block if `pnpm check:changed` and diff checks pass for the current branch. Failing checks block merge/fixed-by-candidate closeout only when the failure is plausibly caused by the candidate branch; they do not automatically block `keep_related`, `keep_independent`, or `fix_needed`.
- If the job is calibrated for finalization, treat stale branches, unknown/unstable merge state, failing relevant checks, and missing review proof as repair work for the executor: rebase/refactor narrowly, rerun review, address bot/human comments, and only then merge or block with concrete proof.
- A calibrated canonical PR that needs repair must produce an executable fix artifact. Use `repair_contributor_branch` when `maintainer_can_modify=true`; use `replace_uneditable_branch` or `new_fix_pr` only when the existing branch cannot be safely updated.
- Before recommending a merge, review actionable PR comments, address required changes or state why they are blocked, prefer a narrower refactor over broad churn, and rebase against current `main` when the branch is stale.
- Bot review comments count as required review comments. Greptile, Codex, Asile, CodeRabbit, Copilot, and similar automated reviewer findings must be addressed, proven non-actionable, or escalated.
- Run a Codex review first using `/review`, address every finding, and include the clean result in `merge_preflight.codex_review`. Do not recommend merge from a stale or missing Codex review.

View File

@ -90,7 +90,7 @@ while (pending.length > 0) {
}
const itemList = [...items.values()].sort((left, right) => left.number - right.number);
const securitySensitiveItems = itemList.filter((item) => itemSecuritySensitive(item));
const securitySensitiveItems = itemList.filter((item) => itemSecuritySensitive(item, job));
const plan = {
repo: job.frontmatter.repo,
cluster_id: job.frontmatter.cluster_id,
@ -290,7 +290,7 @@ function summarizeItem(item, job) {
closed_at: item.closed_at,
hydration_error: item.hydration_error ?? null,
body_excerpt: item.body_excerpt,
security_sensitive: itemSecuritySensitive(item),
security_sensitive: itemSecuritySensitive(item, job),
comments_count: item.comments_count ?? item.comments.length,
comments_hydrated: item.comments.length,
comments_truncated: Math.max(0, item.comments.length - MAX_COMMENTS_PER_ITEM),
@ -433,7 +433,7 @@ function canonicalCandidates(items, job) {
}
function classificationHint(item, job) {
if (itemSecuritySensitive(item)) return "security_sensitive_route_only";
if (itemSecuritySensitive(item, job)) return "security_sensitive_route_only";
const canonicalNumbers = new Set((job.frontmatter.canonical ?? []).map((ref) => normalizeRef(job.frontmatter.repo, ref).number));
if (canonicalNumbers.has(item.number)) return "canonical_hint";
if (item.state !== "open") return "already_closed";
@ -445,7 +445,8 @@ function classificationHint(item, job) {
return "open_issue_candidate";
}
function itemSecuritySensitive(item) {
function itemSecuritySensitive(item, job) {
if (securityOverrideRefs(job).has(`#${item.number}`)) return false;
return hasSecuritySignalText(
item.title,
item.body,
@ -455,6 +456,10 @@ function itemSecuritySensitive(item) {
);
}
function securityOverrideRefs(job) {
return new Set((job.frontmatter.security_override_refs ?? []).map((ref) => `#${String(ref).replace(/^#/, "")}`));
}
function extractLinkedRefs(defaultRepo, item) {
const texts = [
item.title,

View File

@ -1,7 +1,7 @@
#!/usr/bin/env node
import fs from "node:fs";
import path from "node:path";
import { parseArgs, repoRoot } from "./lib.mjs";
import { parseArgs, parseJob, repoRoot } from "./lib.mjs";
const CLOSE_ACTIONS = new Set([
"close",
@ -219,6 +219,14 @@ function reviewResult(resultPath) {
if (plannedMergeActions.length > 0) {
validateMergePreflight(result.merge_preflight, plannedMergeActions, failures);
}
validateCalibratedPrFinalization({
job: readSourceJob(plan),
result,
itemByRef,
fixActions,
mergeActions,
failures,
});
if (result.canonical) {
const canonicalRef = normalizeRef(result.canonical);
@ -255,6 +263,24 @@ function reviewResult(resultPath) {
};
}
function validateCalibratedPrFinalization({ job, result, itemByRef, fixActions, mergeActions, failures }) {
if (!Array.isArray(job?.frontmatter?.maintainer_calibration) || job.frontmatter.maintainer_calibration.length === 0) {
return;
}
const canonicalPrRef = normalizeRef(result.canonical_pr ?? result.canonical);
if (!canonicalPrRef) return;
const canonicalItem = itemByRef.get(canonicalPrRef);
if (!canonicalItem || canonicalItem.kind !== "pull_request" || canonicalItem.state !== "open") return;
const hasPlannedMerge = mergeActions.some(
(action) => normalizeRef(action.target) === canonicalPrRef && action.status === "planned",
);
const hasPlannedFix = fixActions.some((action) => ["planned"].includes(String(action.status ?? "")));
if (hasPlannedMerge || hasPlannedFix) return;
failures.push(
`${canonicalPrRef} calibrated canonical PR requires either a planned merge action with merge_preflight or a planned fix action with repair_contributor_branch/new_fix_pr fix_artifact`,
);
}
function isClusterScopedAction(action, result) {
const name = String(action.action ?? "");
const target = String(action.target ?? "");
@ -423,6 +449,15 @@ function readSiblingJson(runDir, filename) {
return null;
}
function readSourceJob(plan) {
if (!plan?.source_job) return null;
try {
return parseJob(path.join(repoRoot(), plan.source_job));
} catch {
return null;
}
}
function buildItemMap(plan, repo) {
const items = plan?.item_matrix ?? plan?.items ?? [];
const out = new Map();