mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 17:53:39 +00:00
fix(pr): admit reviewed corrections without rewriting incoming verdicts (#149068)
* fix(pr): admit reviewed corrections without rewriting incoming verdicts * fix: register correction review tooling and closeout fixture * test(pr): load review helpers in changelog gate fixtures * fix(pr): keep correction review helper internal * test: join Talk fixture database workers before teardown
This commit is contained in:
Jason (Json)
GitHub
parent
c8a045b2c1
commit
78d5bedb34
21 changed files with 1309 additions and 54 deletions
|
|
@ -177,6 +177,8 @@ const repositoryScriptEntries = [
|
|||
"scripts/pr-lib/ci-dispatch.mjs!",
|
||||
// merge.sh invokes this native review-authority parser by path.
|
||||
"scripts/pr-lib/clawsweeper-review-gate.mjs!",
|
||||
// review.sh invokes the corrected-candidate review validator by path.
|
||||
"scripts/pr-lib/correction-review.mjs!",
|
||||
"scripts/pr-lib/gh-api-preflight.mjs!",
|
||||
"scripts/pr-lib/materialize-dependencies.mjs!",
|
||||
"scripts/pr-lib/merge-body.mjs!",
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ This directory owns local tooling, script wrappers, and generated-artifact helpe
|
|||
## PR Prepare Gates
|
||||
|
||||
- The default agent handoff uses `OPENCLAW_PR_GATES_REMOTE=github` and `merge-run --auto-merge`. Preparation records `github_pending` bound to the published head without successful-proof stamps. Merge requires completed review, the exact prepared head, and the enforced `openclaw/ci-gate`; it rejects known failed required checks, accepts pending checks, and skips separate hosted workflow verification and its synchronous CI watcher. GitHub enforces required CI/security checks and reviews; the agent retains responsibility for follow-through. This mode replaces separate scheduled Testbox evidence with the PR's enforced gate; existing completed-evidence modes remain available. Accepted requests return pending, not completion. Follow the maintainer skill's polling cadence, investigate failures and conflicts, and reconcile through native recovery until merge and cleanup are verified. Preserve accepted or uncertain outcome records; never blindly re-arm a request. No admin or REST fallback applies to pending-gate admission.
|
||||
|
||||
- Normal `prepare-init` requires incoming-head READY. To resolve a validated incoming NEEDS WORK review with BLOCKER/IMPORTANT findings, explicitly use `scripts/pr prepare-correction-init <PR>`. This initializes correction preparation only, preserving the incoming review and contributor ancestry. Commit the fixes, then use `prepare-correction-review-init` to create a separate exact-candidate JSON review template. JSON alone is authoritative; validation renders its summary. Independently review the full corrected candidate and explain resolution of every required incoming finding. Gates, push, sync and merge require that candidate's READY review; changing the candidate or incoming review invalidates it. Discussion/rejection verdicts cannot use this route. This does not change canonical-wrapper trust or permit use of an unlanded wrapper on another PR. Correction publication does not accept `github_pending`; use a completed exact-candidate gate mode or the separately authorized protected Crabbox pending route.
|
||||
- PR source acquisition fetches the full head SHA authenticated by live PR metadata from the canonical origin, verifies the fetched commit, and checks that head SHA, branch, and repository identity stayed unchanged across the fetch. GitHub's asynchronous `refs/pull/<PR>/head` projection is not source authority. All review, prepare, publication, and merge fetches use this owner without changing the private main checkpoint or shared tracking refs.
|
||||
- Supervised PR operations disable automatic Git maintenance through inherited process configuration, preserving repository settings. Explicit maintenance must still join before completion. PR source fetches also disable automatic maintenance: fetching one PR does not authorize repository-wide pruning of unrelated worktree metadata.
|
||||
- Main freshness belongs to one `scripts/pr` operation: nested entry/review guards share a captured main SHA; gate selection, publication after gates, and merge verification after CI each refresh it. Capture the canonical origin's main from the PR worktree's private `FETCH_HEAD`, never a later shared-ref read. Main refreshes never write shared `origin/main`. Cold provisioning fetches into its existing per-PR `temp/pr-<PR>` branch without writing canonical `FETCH_HEAD`, then fully initializes from that seed before the private checkpoint; the seed is not checkpoint authority. Checkout budgets scale with the measured tree allocation (at least 4 KiB per file), with a four-hour ceiling; the same measurement grants Git at least 30 seconds and at most 30 minutes for signal cleanup before force-kill. Provisioning advertises its measured cleanup allowance over the existing lock-notification pipe so the outer supervisor also joins cleanup after an interrupt; the second-signal force escape is unchanged. Allocation leases, template records, and configuration observation use canonical-repo `.local/pr-state`, independently of the operator's state-directory environment. Failed native provisioning may remove only its exclusively reserved, unchanged directory after the Git runner settles and the shared cleanup owner proves no admin entry belongs to that path; ordinary rejection removes only an empty reservation, while interruption permits partial checkout removal; registered hook state, preexisting damage, uncertain process cleanup, and changed authority remain preserved. Directory cleanup never releases the failed operation lock. Every standalone command and newly provisioned worktree starts fresh; coalescing must not skip containment, transition recovery, exact-head checks, or the separate Crabbox authority windows.
|
||||
|
|
|
|||
17
scripts/pr
17
scripts/pr
|
|
@ -70,7 +70,7 @@ pr_subcommand_classification() {
|
|||
ls | ci-dispatch)
|
||||
printf 'advisory\n'
|
||||
;;
|
||||
gc | lock-recover | review-init | review-checkout-main | review-checkout-pr | review-claim | review-guard | review-artifacts-init | review-validate-artifacts | review-tests | prepare-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | merge-verify | merge-run | merge-recover | merge-complete)
|
||||
gc | lock-recover | review-init | review-checkout-main | review-checkout-pr | review-claim | review-guard | review-artifacts-init | review-validate-artifacts | review-tests | prepare-init | prepare-correction-init | prepare-correction-review-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | merge-verify | merge-run | merge-recover | merge-complete)
|
||||
printf 'landing\n'
|
||||
;;
|
||||
*) return 1 ;;
|
||||
|
|
@ -461,7 +461,7 @@ readonly canonical_repo_root
|
|||
|
||||
is_main_only_pr_command() {
|
||||
case "$1" in
|
||||
prepare-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | merge-verify | merge-run | merge-recover) return 0 ;;
|
||||
prepare-init | prepare-correction-init | prepare-correction-review-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | merge-verify | merge-run | merge-recover) return 0 ;;
|
||||
*) return 1 ;;
|
||||
esac
|
||||
}
|
||||
|
|
@ -520,6 +520,8 @@ Usage:
|
|||
scripts/pr review-validate-artifacts <PR>
|
||||
scripts/pr review-tests <PR> <test-file> [<test-file> ...]
|
||||
scripts/pr prepare-init <PR>
|
||||
scripts/pr prepare-correction-init <PR>
|
||||
scripts/pr prepare-correction-review-init <PR>
|
||||
scripts/pr prepare-validate-commit <PR>
|
||||
scripts/pr prepare-gates <PR>
|
||||
scripts/pr prepare-push <PR>
|
||||
|
|
@ -694,7 +696,7 @@ main() {
|
|||
[ "$cancel_auto" = false ] || [ -z "$replacement_head$body_path$legacy_directory$refusal_directory" ] || { usage; exit 2; }
|
||||
set -- "$merge_pr"
|
||||
;;
|
||||
review-init | review-checkout-main | review-checkout-pr | review-claim | review-guard | review-artifacts-init | review-validate-artifacts | prepare-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | ci-dispatch | merge-verify)
|
||||
review-init | review-checkout-main | review-checkout-pr | review-claim | review-guard | review-artifacts-init | review-validate-artifacts | prepare-init | prepare-correction-init | prepare-correction-review-init | prepare-validate-commit | prepare-gates | prepare-push | prepare-sync-head | prepare-run | ci-dispatch | merge-verify)
|
||||
[ "$#" -ge 1 ] || { usage; exit 2; }
|
||||
;;
|
||||
*)
|
||||
|
|
@ -708,6 +710,7 @@ main() {
|
|||
case "$cmd" in
|
||||
review-validate-artifacts) review_artifact_preflight "${1-}" || return 1 ;;
|
||||
prepare-init | prepare-run) review_artifact_preflight "${1-}" true || return 1 ;;
|
||||
prepare-correction-init) review_artifact_preflight "${1-}" correction || return 1 ;;
|
||||
esac
|
||||
|
||||
# merge-run reconciles retained outcomes before reading disposable artifacts or base guards.
|
||||
|
|
@ -779,6 +782,14 @@ main() {
|
|||
shift || true
|
||||
review_tests "$pr" "$@"
|
||||
;;
|
||||
prepare-correction-init)
|
||||
[ "$#" -eq 1 ] || { usage; exit 2; }
|
||||
prepare_init "$1" "${PR_OBSERVATION:-}" correction
|
||||
;;
|
||||
prepare-correction-review-init)
|
||||
[ "$#" -eq 1 ] || { usage; exit 2; }
|
||||
prepare_correction_review_init "$1"
|
||||
;;
|
||||
prepare-init)
|
||||
local pr="${1-}"
|
||||
[ -n "$pr" ] || { usage; exit 2; }
|
||||
|
|
|
|||
160
scripts/pr-lib/correction-review.mjs
Normal file
160
scripts/pr-lib/correction-review.mjs
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
#!/usr/bin/env node
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { copyFileSync, lstatSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { isDirectRunUrl } from "../lib/direct-run.mjs";
|
||||
import {
|
||||
createReviewArtifactTemplate,
|
||||
renderReviewMarkdown,
|
||||
validateReviewArtifacts,
|
||||
} from "./review-artifacts.mjs";
|
||||
|
||||
const gitExecutable = process.env.OPENCLAW_PR_GIT || process.env.GIT_EXEC || "git";
|
||||
const git = (...args) => execFileSync(gitExecutable, args, { encoding: "utf8" }).trim();
|
||||
|
||||
function readRegular(path) {
|
||||
if (!lstatSync(path).isFile()) {
|
||||
throw new Error(`Expected a regular review file: ${path}`);
|
||||
}
|
||||
return readFileSync(path, "utf8");
|
||||
}
|
||||
|
||||
function incomingReview(pr, incoming, jsonOid) {
|
||||
const reviewPath = ".local/review.json";
|
||||
const review = JSON.parse(readRegular(reviewPath));
|
||||
const prMeta = JSON.parse(readRegular(".local/pr-meta.json"));
|
||||
const violations = validateReviewArtifacts({ review, prMeta });
|
||||
if (violations.length) {
|
||||
throw new Error(violations.join("\n"));
|
||||
}
|
||||
if (prMeta.number !== pr || prMeta.headRefOid !== incoming) {
|
||||
throw new Error("Correction preparation does not match the incoming PR review.");
|
||||
}
|
||||
if (git("hash-object", "--no-filters", reviewPath) !== jsonOid) {
|
||||
throw new Error(
|
||||
"Incoming review changed after correction admission. Retain evidence and re-review.",
|
||||
);
|
||||
}
|
||||
const findings = review.findings.filter((item) =>
|
||||
["BLOCKER", "IMPORTANT"].includes(item.severity),
|
||||
);
|
||||
if (review.recommendation !== "NEEDS WORK" || findings.length === 0) {
|
||||
throw new Error("Correction admission requires NEEDS WORK with actionable findings.");
|
||||
}
|
||||
if (new Set(findings.map((item) => item.id)).size !== findings.length) {
|
||||
throw new Error("Correction admission requires distinct finding IDs.");
|
||||
}
|
||||
return { prMeta, findings };
|
||||
}
|
||||
|
||||
function candidateMetadata(prMeta, incoming, head) {
|
||||
if (!/^[0-9a-f]{40}$/u.test(head) || head === incoming || git("rev-parse", "HEAD") !== head) {
|
||||
throw new Error("Review a committed correction, not the unchanged incoming head.");
|
||||
}
|
||||
execFileSync(gitExecutable, ["merge-base", "--is-ancestor", incoming, head]);
|
||||
execFileSync(gitExecutable, ["diff", "--quiet", "HEAD", "--"]);
|
||||
if (git("ls-files", "--others", "--exclude-standard")) {
|
||||
throw new Error("Commit or preserve untracked source before reviewing the correction.");
|
||||
}
|
||||
// Include both the incoming scope and every fixup path. Candidate metadata is
|
||||
// derived in memory; the live incoming-head metadata is never rewritten.
|
||||
const changed = execFileSync(gitExecutable, ["diff", "--name-only", "-z", incoming, head], {
|
||||
encoding: "utf8",
|
||||
})
|
||||
.split("\0")
|
||||
.filter(Boolean);
|
||||
const paths = new Set([...prMeta.files.map((file) => file.path), ...changed]);
|
||||
return { ...prMeta, headRefOid: head, files: [...paths].map((path) => ({ path })) };
|
||||
}
|
||||
|
||||
function runCorrectionReview(command, pr, incoming, head, jsonOid) {
|
||||
if (
|
||||
!["init", "validate"].includes(command) ||
|
||||
!Number.isSafeInteger(pr) ||
|
||||
pr < 1 ||
|
||||
!/^[0-9a-f]{40}$/u.test(incoming)
|
||||
) {
|
||||
throw new Error("Invalid correction-review command or PR number.");
|
||||
}
|
||||
const { prMeta, findings } = incomingReview(pr, incoming, jsonOid);
|
||||
const candidate = candidateMetadata(prMeta, incoming, head);
|
||||
const jsonPath = ".local/correction-review.json";
|
||||
const incomingJsonPath = ".local/correction-incoming-review.json";
|
||||
if (command === "init") {
|
||||
const existing = [jsonPath, incomingJsonPath].filter((path) =>
|
||||
lstatSync(path, { throwIfNoEntry: false }),
|
||||
);
|
||||
for (const path of existing) {
|
||||
readRegular(path);
|
||||
}
|
||||
if (existing.length) {
|
||||
const archive = mkdtempSync(".local/correction-review-retained.");
|
||||
for (const path of existing) {
|
||||
copyFileSync(path, join(archive, path.split("/").at(-1)));
|
||||
}
|
||||
}
|
||||
const review = createReviewArtifactTemplate({ number: pr, headSha: head });
|
||||
review.correction = {
|
||||
incomingHeadSha: incoming,
|
||||
incomingReviewJsonOid: jsonOid,
|
||||
resolvedFindings: findings.map(({ id }) => ({ id, resolution: "" })),
|
||||
};
|
||||
copyFileSync(".local/review.json", incomingJsonPath);
|
||||
writeFileSync(jsonPath, `${JSON.stringify(review, null, 2)}\n`);
|
||||
return;
|
||||
}
|
||||
const review = JSON.parse(readRegular(jsonPath));
|
||||
const violations = validateReviewArtifacts({
|
||||
review,
|
||||
prMeta: candidate,
|
||||
});
|
||||
if (violations.length) {
|
||||
throw new Error(violations.join("\n"));
|
||||
}
|
||||
readRegular(incomingJsonPath);
|
||||
if (
|
||||
review.correction?.incomingReviewJsonOid !== jsonOid ||
|
||||
git("hash-object", "--no-filters", incomingJsonPath) !== jsonOid
|
||||
) {
|
||||
throw new Error("Correction approval does not bind these exact incoming review bytes.");
|
||||
}
|
||||
if (review.recommendation !== "READY FOR /prepare-pr") {
|
||||
throw new Error(
|
||||
"The exact correction needs an independent READY review before gates or publication.",
|
||||
);
|
||||
}
|
||||
const resolved = review.correction?.resolvedFindings;
|
||||
if (
|
||||
review.correction?.incomingHeadSha !== incoming ||
|
||||
!Array.isArray(resolved) ||
|
||||
resolved.length !== findings.length ||
|
||||
new Set(resolved.map((item) => item?.id)).size !== findings.length ||
|
||||
!findings.every(({ id }) =>
|
||||
resolved.some(
|
||||
(item) =>
|
||||
item?.id === id &&
|
||||
typeof item.resolution === "string" &&
|
||||
item.resolution.trim().length > 0,
|
||||
),
|
||||
)
|
||||
) {
|
||||
throw new Error("The correction review must resolve every incoming BLOCKER/IMPORTANT finding.");
|
||||
}
|
||||
process.stdout.write(renderReviewMarkdown(review));
|
||||
}
|
||||
|
||||
if (isDirectRunUrl(process.argv[1], import.meta.url)) {
|
||||
try {
|
||||
const [command, pr, incoming, head, jsonOid, ...extra] = process.argv.slice(2);
|
||||
if (extra.length || !jsonOid) {
|
||||
throw new Error(
|
||||
"Expected command, PR, incoming/candidate heads and incoming review JSON object ID.",
|
||||
);
|
||||
}
|
||||
runCorrectionReview(command, Number(pr), incoming, head, jsonOid);
|
||||
} catch (error) {
|
||||
console.error(error instanceof Error ? error.message : String(error));
|
||||
process.exitCode = 1;
|
||||
}
|
||||
}
|
||||
|
|
@ -353,6 +353,58 @@ write_gates_env_stamp() {
|
|||
} > .local/gates.env
|
||||
}
|
||||
|
||||
# Correction publication requires the native gate owner's exact candidate
|
||||
# stamp. An explicit pending Crabbox stamp is admission to its protected-main
|
||||
# publisher, not proof; retain that separately authorized route.
|
||||
require_correction_publication_gates() (
|
||||
local pr="$1" head="$2" allow_pending="${3:-false}"
|
||||
local PR_NUMBER="" LAST_VERIFIED_HEAD_SHA="" FULL_GATES_HEAD_SHA=""
|
||||
local GATES_MODE="" HOSTED_GATES_TARGET_HEAD_SHA="" DOCS_ONLY=""
|
||||
local REMOTE_GATES_PROVIDER="" REMOTE_GATES_RUN_ID="" REMOTE_GATES_LEASE_ID="" REMOTE_GATES_RUN_URL=""
|
||||
require_artifact .local/gates.env || return 1
|
||||
source .local/gates.env || return 1
|
||||
local qualified_head="$LAST_VERIFIED_HEAD_SHA"
|
||||
[ "$PR_NUMBER" = "$pr" ] || return 1
|
||||
if [ "$qualified_head" != "$head" ]; then
|
||||
# GraphQL can assign a hosted OID for the identical reviewed local tree.
|
||||
# Only a verified publication receipt can bind that pair.
|
||||
local PREP_HEAD_SHA="" LOCAL_PREP_HEAD_SHA=""
|
||||
PR_NUMBER=""
|
||||
[ -s .local/prep.env ] && source .local/prep.env || return 1
|
||||
[ "$PR_NUMBER" = "$pr" ] && [ "$LOCAL_PREP_HEAD_SHA" = "$head" ] &&
|
||||
[ "$PREP_HEAD_SHA" = "$qualified_head" ] &&
|
||||
[ "$(pr_git rev-parse "$head^{tree}")" = "$(pr_git rev-parse "$qualified_head^{tree}")" ] || {
|
||||
echo "Correction publication requires gates for the exact reviewed candidate." >&2
|
||||
return 1
|
||||
}
|
||||
fi
|
||||
case "$GATES_MODE" in
|
||||
full) [ "$FULL_GATES_HEAD_SHA" = "$qualified_head" ] || return 1 ;;
|
||||
docs_only|reused_docs_only) [ "$DOCS_ONLY" = true ] || return 1 ;;
|
||||
hosted_exact_or_recent_parent) [ "$HOSTED_GATES_TARGET_HEAD_SHA" = "$qualified_head" ] || return 1 ;;
|
||||
remote_testbox)
|
||||
[ "$FULL_GATES_HEAD_SHA" = "$qualified_head" ] &&
|
||||
[ "$REMOTE_GATES_PROVIDER" = blacksmith-testbox ] &&
|
||||
[[ "$REMOTE_GATES_LEASE_ID" == tbx_* ]] || return 1
|
||||
;;
|
||||
remote_crabbox_aws)
|
||||
[ "$FULL_GATES_HEAD_SHA" = "$qualified_head" ] &&
|
||||
[ "$REMOTE_GATES_PROVIDER" = aws ] &&
|
||||
[[ "$REMOTE_GATES_RUN_ID" == run_* ]] && [[ "$REMOTE_GATES_LEASE_ID" == cbx_* ]] &&
|
||||
[[ "$REMOTE_GATES_RUN_URL" == https://github.com/openclaw/openclaw/actions/runs/* ]] || return 1
|
||||
;;
|
||||
remote_crabbox_aws_pending)
|
||||
[ "$allow_pending" = true ] && [ "$REMOTE_GATES_PROVIDER" = aws ] || return 1
|
||||
require_active_org_admin_for_crabbox_gate >/dev/null || return 1
|
||||
# The candidate is not hosted yet. Bind eligibility to the publication
|
||||
# lease, then let the existing publisher verify the newly hosted head.
|
||||
[ -n "${PREP_PUBLICATION_LEASE_SHA:-}" ] || return 1
|
||||
read_crabbox_gate_pr_binding "$pr" "$PREP_PUBLICATION_LEASE_SHA" >/dev/null || return 1
|
||||
;;
|
||||
*) echo "Unrecognized correction gate mode: $GATES_MODE" >&2; return 1 ;;
|
||||
esac
|
||||
)
|
||||
|
||||
derive_prepare_gate_change_plan() {
|
||||
PREPARE_GATE_CHANGED_FILES=$(pr_git diff --name-only "$PR_MAIN_SHA...${1:-HEAD}") || return 1
|
||||
PREPARE_GATE_DOCS_ONLY=false
|
||||
|
|
@ -395,6 +447,7 @@ prepare_gates() {
|
|||
# shellcheck disable=SC1091
|
||||
source .local/pr-meta.env
|
||||
|
||||
require_prepared_review "$pr" || return 1
|
||||
derive_prepare_gate_change_plan
|
||||
local changed_files="$PREPARE_GATE_CHANGED_FILES"
|
||||
local docs_only="$PREPARE_GATE_DOCS_ONLY"
|
||||
|
|
@ -552,6 +605,11 @@ prepare_gates() {
|
|||
fi
|
||||
fi
|
||||
|
||||
require_prepared_review "$pr" || return 1
|
||||
[ "$(pr_git rev-parse HEAD)" = "$current_head" ] || {
|
||||
echo "Candidate changed while gates ran; no gate stamp written." >&2
|
||||
return 1
|
||||
}
|
||||
write_gates_env_stamp \
|
||||
"$pr" \
|
||||
"$docs_only" \
|
||||
|
|
|
|||
|
|
@ -217,6 +217,13 @@ merge_verify() {
|
|||
|
||||
require_artifact .local/prep.env || return 1
|
||||
require_artifact .local/gates.env || return 1
|
||||
require_prepared_review "$pr" || return 1
|
||||
local correction_authority correction_gate_oid=""
|
||||
correction_authority=$(correction_review_snapshot "$pr") || return 1
|
||||
if [ -n "$correction_authority" ]; then
|
||||
require_correction_publication_gates "$pr" "$(pr_git rev-parse HEAD)" || return 1
|
||||
correction_gate_oid=$(pr_git hash-object --no-filters .local/gates.env) || return 1
|
||||
fi
|
||||
# shellcheck disable=SC1091
|
||||
source .local/gates.env || return 1
|
||||
# shellcheck disable=SC1091
|
||||
|
|
@ -393,6 +400,10 @@ merge_verify() {
|
|||
fi
|
||||
fi
|
||||
|
||||
verify_correction_review_snapshot "$pr" "$correction_authority" || return 1
|
||||
if [ -n "$correction_authority" ]; then
|
||||
[ "$correction_gate_oid" = "$(pr_git hash-object --no-filters .local/gates.env)" ] || return 1
|
||||
fi
|
||||
echo "merge-verify passed for PR #$pr"
|
||||
}
|
||||
|
||||
|
|
@ -482,6 +493,20 @@ prepare_squash_merge_body() {
|
|||
verify_merge_replacement_artifacts() (
|
||||
local pr="$1" head="$2" qualified_refusal="${3:-false}"
|
||||
local HOSTED_GATES_TARGET_HEAD_SHA=""
|
||||
local PREP_REVIEW_MODE=""
|
||||
source .local/prep-context.env || return 1
|
||||
if [ "$PREP_REVIEW_MODE" = correction ]; then
|
||||
# Incoming H remains the truthful NEEDS WORK identity. The correction
|
||||
# owner verifies H -> local C; the publication receipt binds C -> hosted P.
|
||||
require_prepared_review "$pr" || return 1
|
||||
local PR_NUMBER="" PREP_HEAD_SHA="" LOCAL_PREP_HEAD_SHA=""
|
||||
source .local/prep.env || return 1
|
||||
[ "$PR_NUMBER" = "$pr" ] && [ "$PREP_HEAD_SHA" = "$head" ] &&
|
||||
[ "$LOCAL_PREP_HEAD_SHA" = "$(pr_git rev-parse HEAD)" ] &&
|
||||
[ "$(pr_git rev-parse "$LOCAL_PREP_HEAD_SHA^{tree}")" = "$(pr_git rev-parse "$head^{tree}")" ] || return 1
|
||||
require_correction_publication_gates "$pr" "$LOCAL_PREP_HEAD_SHA" || return 1
|
||||
return 0
|
||||
fi
|
||||
local PR_NUMBER="" PR_HEAD_SHA="" PR_HEAD_SHA_BEFORE=""
|
||||
local PREP_HEAD_SHA="" LOCAL_PREP_HEAD_SHA="" LAST_VERIFIED_HEAD_SHA="" GATES_MODE=""
|
||||
source .local/pr-meta.env || return 1
|
||||
|
|
@ -558,7 +583,7 @@ merge_run() {
|
|||
if [ "$auto_merge_requested" = true ] || [ "${OPENCLAW_PR_MERGE_METHOD:-squash}" != squash ]; then
|
||||
MERGE_TRANSPORT=graphql
|
||||
fi
|
||||
review_artifact_preflight "$pr" true || return 1
|
||||
review_artifact_preflight "$pr" prepared || return 1
|
||||
# Capture before gates or cwd changes; retained outcomes above reconcile even
|
||||
# when the original operator file no longer exists.
|
||||
if [ -n "$body_path" ]; then
|
||||
|
|
@ -591,6 +616,14 @@ merge_run() {
|
|||
.local/prep.env
|
||||
)
|
||||
[ -z "$replacement_head" ] || required_artifacts+=(.local/prep-context.env .local/gates.env)
|
||||
local correction_authority="" correction_gates_oid=""
|
||||
correction_authority=$(correction_review_snapshot "$pr") || return 1
|
||||
if [ -n "$correction_authority" ]; then
|
||||
required_artifacts+=(.local/prep-context.env .local/gates.env
|
||||
.local/correction-review.json
|
||||
.local/correction-incoming-review.json)
|
||||
correction_gates_oid=$(pr_git hash-object --no-filters .local/gates.env) || return 1
|
||||
fi
|
||||
for required in "${required_artifacts[@]}"; do
|
||||
require_artifact "$required" || return 1
|
||||
done
|
||||
|
|
@ -618,7 +651,7 @@ merge_run() {
|
|||
done
|
||||
fi
|
||||
validate_review_artifact_data || return 1
|
||||
require_ready_review_recommendation || return 1
|
||||
require_prepared_review "$pr" || return 1
|
||||
local verify_options
|
||||
verify_options=$(jq -cn --arg replacementHead "$replacement_head" \
|
||||
--argjson autoMergeRequested "$auto_merge_requested" --argjson observation "$MERGE_ENTRY_OBSERVATION" \
|
||||
|
|
@ -896,6 +929,11 @@ merge_run() {
|
|||
if [ "$MERGE_TRANSPORT" = graphql ] && [ "$merge_method" = squash ] && [ "$route" != queue ]; then
|
||||
merge_args+=(--subject "$MERGE_SUBJECT")
|
||||
fi
|
||||
verify_correction_review_snapshot "$pr" "$correction_authority" || return 1
|
||||
if [ -n "$correction_authority" ]; then
|
||||
[ "$correction_gates_oid" = "$(pr_git hash-object --no-filters .local/gates.env)" ] || return 1
|
||||
require_correction_publication_gates "$pr" "$(pr_git rev-parse HEAD)" || return 1
|
||||
fi
|
||||
local intent attempt
|
||||
attempt=$(node -e 'process.stdout.write(require("node:crypto").randomUUID())') || return 1
|
||||
intent=$(printf '%s\n' "$MERGE_OBSERVATION" | jq -c --argjson repo "$MERGE_REPO" \
|
||||
|
|
|
|||
|
|
@ -39,6 +39,10 @@ retire_prep_evidence() {
|
|||
.local/prepare-push-result.env \
|
||||
.local/prepare-sync-result.env \
|
||||
.local/prep.md \
|
||||
.local/correction-review.json \
|
||||
.local/correction-review.md \
|
||||
.local/correction-incoming-review.json \
|
||||
.local/correction-incoming-review.md \
|
||||
.local/gates-*.log; do
|
||||
if [ ! -e "$artifact" ] && [ ! -L "$artifact" ]; then
|
||||
continue
|
||||
|
|
@ -59,6 +63,10 @@ retire_prep_evidence() {
|
|||
rm -f \
|
||||
.local/gates.env \
|
||||
.local/prep.env \
|
||||
.local/correction-review.json \
|
||||
.local/correction-review.md \
|
||||
.local/correction-incoming-review.json \
|
||||
.local/correction-incoming-review.md \
|
||||
.local/prepare-push-result.env \
|
||||
.local/prepare-sync-result.env || return 1
|
||||
printf '%s\n' "- Prior preparation evidence retained at $archive." >> .local/prep.md || return 1
|
||||
|
|
@ -172,11 +180,18 @@ verify_prep_branch_matches_prepared_head() {
|
|||
}
|
||||
|
||||
prepare_init() {
|
||||
local pr="$1"
|
||||
local observation="${2:-}"
|
||||
local pr="$1" observation="${2:-}" review_mode="${3:-ready}"
|
||||
local incoming_json_oid=""
|
||||
# Validate the exact reviewed head before taking the lock past its reversible phase.
|
||||
review_validate_artifacts "$pr" true || return 1
|
||||
require_ready_review_recommendation || return 1
|
||||
case "$review_mode" in
|
||||
ready) review_validate_artifacts "$pr" true || return 1 ;;
|
||||
correction)
|
||||
review_validate_artifacts "$pr" correction || return 1
|
||||
require_correction_review_recommendation || return 1
|
||||
incoming_json_oid=$(pr_git hash-object --no-filters .local/review.json) || return 1
|
||||
;;
|
||||
*) echo "Unknown preparation review mode: $review_mode" >&2; return 1 ;;
|
||||
esac
|
||||
mark_pr_operation_side_effects_started
|
||||
enter_worktree "$pr" false || return 1
|
||||
|
||||
|
|
@ -239,6 +254,8 @@ prepare_init() {
|
|||
PR_HEAD "$reviewed_head" \
|
||||
PR_HEAD_SHA_BEFORE "$reviewed_head_sha" \
|
||||
PREP_BRANCH "pr-$pr-prep" \
|
||||
PREP_REVIEW_MODE "$review_mode" \
|
||||
PREP_INCOMING_JSON_OID "$incoming_json_oid" \
|
||||
PR_AUTHOR_ACCESS_AT_PREP "$author_access_at_prep" \
|
||||
PREP_STARTED_AT "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \
|
||||
> .local/prep-context.env
|
||||
|
|
@ -262,6 +279,16 @@ EOF_PREP
|
|||
echo "wrote=.local/prep-context.env .local/prep.md"
|
||||
}
|
||||
|
||||
prepare_correction_review_init() {
|
||||
local pr="$1"
|
||||
enter_worktree "$pr" false || return 1
|
||||
mark_pr_operation_side_effects_started
|
||||
checkout_prep_branch "$pr" || return 1
|
||||
run_prepared_correction_review "$pr" init || return 1
|
||||
echo "Complete independent review of this exact correction in .local/correction-review.json (the validated summary is rendered from JSON)."
|
||||
echo "The incoming review is unchanged; gates and publication require the corrected-candidate READY review."
|
||||
}
|
||||
|
||||
prepare_validate_commit() {
|
||||
local pr="$1"
|
||||
enter_worktree "$pr" false || return 1
|
||||
|
|
@ -330,9 +357,17 @@ resolve_prep_publication_target() {
|
|||
fi
|
||||
}
|
||||
|
||||
verify_correction_publication_authority() {
|
||||
[ -n "${PREP_PUBLICATION_REVIEW_SNAPSHOT:-}" ] || return 0
|
||||
require_correction_publication_gates "$PREP_PUBLICATION_PR" "$(pr_git rev-parse HEAD)" \
|
||||
"$PREP_PUBLICATION_ALLOW_PENDING" || return 1
|
||||
verify_correction_review_snapshot "$PREP_PUBLICATION_PR" "$PREP_PUBLICATION_REVIEW_SNAPSHOT"
|
||||
}
|
||||
|
||||
prepare_push() {
|
||||
local pr="$1"
|
||||
local observation="${2:-}"
|
||||
local PREP_PUBLICATION_REVIEW_SNAPSHOT="" PREP_PUBLICATION_PR="$pr" PREP_PUBLICATION_ALLOW_PENDING=true
|
||||
PR_MAIN_SHA=""
|
||||
enter_worktree "$pr" false || return 1
|
||||
|
||||
|
|
@ -371,6 +406,9 @@ prepare_push() {
|
|||
return 1
|
||||
fi
|
||||
|
||||
require_prepared_review "$pr" || return 1
|
||||
PREP_PUBLICATION_REVIEW_SNAPSHOT=$(correction_review_snapshot "$pr") || return 1
|
||||
verify_correction_publication_authority || return 1
|
||||
push_prep_head_to_pr_branch "$pr" "$PR_HEAD" "$prep_head_sha" "$lease_sha" "$push_result_env" "$observation" || return $?
|
||||
# shellcheck disable=SC1090
|
||||
source "$push_result_env"
|
||||
|
|
@ -447,6 +485,7 @@ EOF_PREP
|
|||
|
||||
prepare_sync_head() {
|
||||
local pr="$1"
|
||||
local PREP_PUBLICATION_REVIEW_SNAPSHOT="" PREP_PUBLICATION_PR="$pr" PREP_PUBLICATION_ALLOW_PENDING=false
|
||||
enter_worktree "$pr" false || return 1
|
||||
|
||||
require_artifact .local/pr-meta.env
|
||||
|
|
@ -471,6 +510,9 @@ prepare_sync_head() {
|
|||
prep_head_sha="$PREP_PUBLICATION_HEAD_SHA"
|
||||
local push_result_env=".local/prepare-sync-result.env"
|
||||
|
||||
require_prepared_review "$pr" || return 1
|
||||
PREP_PUBLICATION_REVIEW_SNAPSHOT=$(correction_review_snapshot "$pr") || return 1
|
||||
verify_correction_publication_authority || return 1
|
||||
push_prep_head_to_pr_branch "$pr" "$PR_HEAD" "$prep_head_sha" "$lease_sha" "$push_result_env" || return $?
|
||||
# shellcheck disable=SC1090
|
||||
source "$push_result_env"
|
||||
|
|
|
|||
|
|
@ -186,6 +186,9 @@ GRAPHQL
|
|||
return 1
|
||||
fi
|
||||
local result
|
||||
if [ -n "${PREP_PUBLICATION_REVIEW_SNAPSHOT:-}" ]; then
|
||||
verify_correction_publication_authority || { rm -f "$payload_file"; return 1; }
|
||||
fi
|
||||
result=$(pr_gh_plain api graphql --input "$payload_file" 2>&1) || {
|
||||
rm -f "$payload_file"
|
||||
echo "GraphQL push failed: $result" >&2
|
||||
|
|
@ -297,6 +300,9 @@ push_prep_head_once() {
|
|||
|
||||
revalidate_pr_publication "$pr" "$observation" "$pr_head" "$lease_sha" "$prep_head_sha" || return 1
|
||||
local push_output push_status
|
||||
if [ -n "${PREP_PUBLICATION_REVIEW_SNAPSHOT:-}" ]; then
|
||||
verify_correction_publication_authority || return 1
|
||||
fi
|
||||
if push_output=$(pr_git push "--force-with-lease=refs/heads/$pr_head:$lease_sha" "$PRHEAD_REMOTE_URL" "$prep_head_sha:refs/heads/$pr_head" 2>&1); then
|
||||
printf '%s\n' "$push_output" >&2
|
||||
else
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ function reviewIdentityLine({ number, headSha }) {
|
|||
return `Review artifact for PR #${number} at ${headSha}`;
|
||||
}
|
||||
|
||||
function renderReviewMarkdown(review) {
|
||||
export function renderReviewMarkdown(review) {
|
||||
const lines = [reviewIdentityLine(review.pr), "", review.recommendation, ""];
|
||||
for (const finding of review.findings) {
|
||||
lines.push(`- ${finding.severity}: ${finding.title} (${finding.area})`, ` ${finding.fix}`);
|
||||
|
|
@ -53,7 +53,7 @@ function renderReviewMarkdown(review) {
|
|||
return `${lines.join("\n")}\n`;
|
||||
}
|
||||
|
||||
function createReviewArtifactTemplate({ number, headSha }) {
|
||||
export function createReviewArtifactTemplate({ number, headSha }) {
|
||||
return {
|
||||
// Identity stamp, not reviewer input: validation refuses artifacts whose pr
|
||||
// disagrees with .local/pr-meta.json, so a review written for another PR (or
|
||||
|
|
@ -92,7 +92,7 @@ function jsonValue(value) {
|
|||
return JSON.stringify(value === undefined ? null : value);
|
||||
}
|
||||
|
||||
function validateReviewArtifacts({ review, prMeta }) {
|
||||
export function validateReviewArtifacts({ review, prMeta }) {
|
||||
const violations = [];
|
||||
const add = (message) => {
|
||||
if (!violations.includes(message)) {
|
||||
|
|
|
|||
|
|
@ -239,6 +239,91 @@ require_ready_review_recommendation() {
|
|||
fi
|
||||
}
|
||||
|
||||
require_correction_review_recommendation() {
|
||||
if ! jq -e '.recommendation == "NEEDS WORK" and
|
||||
any(.findings[]; .severity == "BLOCKER" or .severity == "IMPORTANT")' \
|
||||
.local/review.json >/dev/null; then
|
||||
echo "Correction preparation requires a validated NEEDS WORK review with actionable findings."
|
||||
return 1
|
||||
fi
|
||||
}
|
||||
|
||||
run_prepared_correction_review() (
|
||||
local pr="$1" command="$2"
|
||||
require_artifact .local/prep-context.env || return 1
|
||||
local PREP_REVIEW_MODE="" PREP_INCOMING_JSON_OID=""
|
||||
local PR_NUMBER="" PR_HEAD_SHA_BEFORE="" PREP_BRANCH=""
|
||||
# shellcheck disable=SC1091
|
||||
source .local/prep-context.env || return 1
|
||||
if [ "$PREP_REVIEW_MODE" != correction ] || [ "$PR_NUMBER" != "$pr" ] ||
|
||||
[ -z "$PREP_INCOMING_JSON_OID" ] ||
|
||||
[ -z "$PREP_BRANCH" ]; then
|
||||
echo "Missing exact incoming review binding; run prepare-correction-init first." >&2
|
||||
return 1
|
||||
fi
|
||||
local head branch_head
|
||||
head=$(pr_git rev-parse HEAD) || return 1
|
||||
branch_head=$(pr_git rev-parse "refs/heads/$PREP_BRANCH") || return 1
|
||||
if [ "$head" != "$branch_head" ]; then
|
||||
echo "Correction review requires the current preparation branch." >&2
|
||||
return 1
|
||||
fi
|
||||
validate_review_artifact_data || return 1
|
||||
node "$(dirname "$(review_artifacts_helper_path)")/correction-review.mjs" \
|
||||
"$command" "$pr" "$PR_HEAD_SHA_BEFORE" "$head" \
|
||||
"$PREP_INCOMING_JSON_OID"
|
||||
)
|
||||
|
||||
require_prepared_review() {
|
||||
local pr="$1" mode=ready
|
||||
validate_review_artifact_data || return 1
|
||||
if [ -s .local/prep-context.env ]; then
|
||||
mode=$(
|
||||
unset PREP_REVIEW_MODE
|
||||
# shellcheck disable=SC1091
|
||||
source .local/prep-context.env || exit 1
|
||||
printf '%s\n' "${PREP_REVIEW_MODE:-ready}"
|
||||
) || return 1
|
||||
fi
|
||||
case "$mode" in
|
||||
ready) require_ready_review_recommendation ;;
|
||||
correction) run_prepared_correction_review "$pr" validate ;;
|
||||
*) echo "Unknown preparation review mode: $mode" >&2; return 1 ;;
|
||||
esac
|
||||
}
|
||||
|
||||
# A correction's review authority must survive every awaited admission read.
|
||||
# Normal READY preparation keeps its existing contract; the nonempty snapshot
|
||||
# also detects loss of correction mode during an operation.
|
||||
correction_review_snapshot() (
|
||||
local pr="$1" PREP_REVIEW_MODE=""
|
||||
[ -s .local/prep-context.env ] || return 0
|
||||
source .local/prep-context.env || return 1
|
||||
[ "$PREP_REVIEW_MODE" = correction ] || return 0
|
||||
require_prepared_review "$pr" >/dev/null || return 1
|
||||
local publication_receipt=()
|
||||
if [ -e .local/prep.env ] || [ -L .local/prep.env ]; then
|
||||
[ -f .local/prep.env ] && [ ! -L .local/prep.env ] || return 1
|
||||
publication_receipt+=(.local/prep.env)
|
||||
fi
|
||||
pr_git hash-object --no-filters -- \
|
||||
.local/prep-context.env .local/pr-meta.json .local/pr-meta.env \
|
||||
.local/review.json \
|
||||
.local/correction-review.json \
|
||||
.local/correction-incoming-review.json \
|
||||
${publication_receipt[@]+"${publication_receipt[@]}"}
|
||||
)
|
||||
|
||||
verify_correction_review_snapshot() {
|
||||
local pr="$1" expected="$2" current
|
||||
[ -n "$expected" ] || return 0
|
||||
current=$(correction_review_snapshot "$pr") || return 1
|
||||
if [ "$expected" != "$current" ]; then
|
||||
echo "Correction review authority changed during admission; no publication or merge is authorized." >&2
|
||||
return 1
|
||||
fi
|
||||
}
|
||||
|
||||
# Pure local admission: malformed or unfinished input must not start a fetch or
|
||||
# leave an operation lock behind. This does not establish remote freshness.
|
||||
review_artifact_preflight() (
|
||||
|
|
@ -258,7 +343,17 @@ review_artifact_preflight() (
|
|||
echo "Review artifact identity mismatch: expected PR #$pr. Re-run scripts/pr review-init $pr"
|
||||
return 1
|
||||
fi
|
||||
if [ "$ready" = true ]; then require_ready_review_recommendation || return 1; fi
|
||||
case "$ready" in
|
||||
true) require_ready_review_recommendation || return 1 ;;
|
||||
correction) require_correction_review_recommendation || return 1 ;;
|
||||
prepared)
|
||||
# Ordinary READY remains sufficient. A truthful incoming NEEDS WORK may
|
||||
# reach merge only through the exact correction owner's local validation.
|
||||
if ! jq -e '.recommendation == "READY FOR /prepare-pr"' .local/review.json >/dev/null; then
|
||||
run_prepared_correction_review "$pr" validate >/dev/null || return 1
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
)
|
||||
|
||||
review_validate_artifacts() {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
set -euo pipefail
|
||||
|
||||
if [ "$#" -ne 2 ]; then
|
||||
echo "Usage: scripts/pr-prepare <init|validate-commit|gates|push|run> <PR>"
|
||||
echo "Usage: scripts/pr-prepare <init|correction-init|correction-review-init|validate-commit|gates|push|run> <PR>"
|
||||
exit 2
|
||||
fi
|
||||
|
||||
|
|
@ -15,6 +15,12 @@ case "$mode" in
|
|||
init)
|
||||
exec "$base" prepare-init "$pr"
|
||||
;;
|
||||
correction-init)
|
||||
exec "$base" prepare-correction-init "$pr"
|
||||
;;
|
||||
correction-review-init)
|
||||
exec "$base" prepare-correction-review-init "$pr"
|
||||
;;
|
||||
validate-commit)
|
||||
exec "$base" prepare-validate-commit "$pr"
|
||||
;;
|
||||
|
|
@ -28,7 +34,7 @@ case "$mode" in
|
|||
exec "$base" prepare-run "$pr"
|
||||
;;
|
||||
*)
|
||||
echo "Usage: scripts/pr-prepare <init|validate-commit|gates|push|run> <PR>"
|
||||
echo "Usage: scripts/pr-prepare <init|correction-init|correction-review-init|validate-commit|gates|push|run> <PR>"
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import {
|
|||
const changelogScriptPath = path.join(process.cwd(), "scripts", "pr-lib", "changelog.sh");
|
||||
const commonScriptPath = path.join(process.cwd(), "scripts", "pr-lib", "common.sh");
|
||||
const gatesScriptPath = path.join(process.cwd(), "scripts", "pr-lib", "gates.sh");
|
||||
const reviewScriptPath = path.join(process.cwd(), "scripts", "pr-lib", "review.sh");
|
||||
|
||||
function run(cwd: string, command: string, args: string[], env?: NodeJS.ProcessEnv): string {
|
||||
return execFileSync(command, args, {
|
||||
|
|
@ -288,11 +289,14 @@ set -euo pipefail
|
|||
source "$OPENCLAW_PR_COMMON_SH"
|
||||
source "$OPENCLAW_PR_CHANGELOG_SH"
|
||||
source "$OPENCLAW_PR_GATES_SH"
|
||||
source "$OPENCLAW_PR_REVIEW_SH"
|
||||
|
||||
pr_gh() { printf '{"headRefName":"feature"}\\n'; }
|
||||
enter_worktree() { PR_MAIN_SHA=$(git rev-parse --verify refs/remotes/origin/main); }
|
||||
checkout_prep_branch() { :; }
|
||||
refresh_prep_branch_for_reviewed_head() { :; }
|
||||
# Preparation tests cover review admission; these fixtures isolate changelog policy.
|
||||
require_prepared_review() { :; }
|
||||
bootstrap_deps_if_needed() { :; }
|
||||
require_artifact() { [ -s "$1" ]; }
|
||||
validate_changelog_attribution_policy() { printf 'policy\\n' >>"$OPENCLAW_TEST_CALLS"; }
|
||||
|
|
@ -307,6 +311,7 @@ prepare_gates 123
|
|||
OPENCLAW_PR_COMMON_SH: commonScriptPath,
|
||||
OPENCLAW_PR_CHANGELOG_SH: changelogScriptPath,
|
||||
OPENCLAW_PR_GATES_SH: gatesScriptPath,
|
||||
OPENCLAW_PR_REVIEW_SH: reviewScriptPath,
|
||||
OPENCLAW_TEST_CALLS: callsPath,
|
||||
OPENCLAW_TESTBOX: "0",
|
||||
},
|
||||
|
|
@ -339,11 +344,14 @@ set -euo pipefail
|
|||
source "$OPENCLAW_PR_COMMON_SH"
|
||||
source "$OPENCLAW_PR_CHANGELOG_SH"
|
||||
source "$OPENCLAW_PR_GATES_SH"
|
||||
source "$OPENCLAW_PR_REVIEW_SH"
|
||||
|
||||
pr_gh() { printf '{"headRefName":"feature"}\\n'; }
|
||||
enter_worktree() { PR_MAIN_SHA=$(git rev-parse --verify refs/remotes/origin/main); }
|
||||
checkout_prep_branch() { :; }
|
||||
refresh_prep_branch_for_reviewed_head() { :; }
|
||||
# Preparation tests cover review admission; these fixtures isolate changelog policy.
|
||||
require_prepared_review() { :; }
|
||||
bootstrap_deps_if_needed() { :; }
|
||||
require_artifact() { [ -s "$1" ]; }
|
||||
validate_changelog_attribution_policy() { printf 'policy\\n' >>"$OPENCLAW_TEST_CALLS"; }
|
||||
|
|
@ -359,6 +367,7 @@ prepare_gates 123
|
|||
OPENCLAW_PR_COMMON_SH: commonScriptPath,
|
||||
OPENCLAW_PR_CHANGELOG_SH: changelogScriptPath,
|
||||
OPENCLAW_PR_GATES_SH: gatesScriptPath,
|
||||
OPENCLAW_PR_REVIEW_SH: reviewScriptPath,
|
||||
OPENCLAW_TEST_CALLS: callsPath,
|
||||
OPENCLAW_TESTBOX: "0",
|
||||
},
|
||||
|
|
|
|||
|
|
@ -141,9 +141,12 @@ set -euo pipefail
|
|||
source "$SCRIPTS/pr-lib/common.sh"
|
||||
source "$SCRIPTS/pr-lib/changelog.sh"
|
||||
source "$SCRIPTS/pr-lib/gates.sh"
|
||||
source "$SCRIPTS/pr-lib/review.sh"
|
||||
enter_worktree() { PR_MAIN_SHA="$MAIN_SHA"; }
|
||||
refresh_prep_branch_for_reviewed_head() { :; }
|
||||
checkout_prep_branch() { :; }
|
||||
# Review authority is covered by the preparation fixtures; this isolates release classification.
|
||||
require_prepared_review() { :; }
|
||||
run_quiet_logged() {
|
||||
if [ "$1" = 'hosted CI/Testbox gates' ]; then
|
||||
jq -se --slurpfile expected metadata.json '
|
||||
|
|
|
|||
550
test/scripts/pr-correction-preparation.test.ts
Normal file
550
test/scripts/pr-correction-preparation.test.ts
Normal file
|
|
@ -0,0 +1,550 @@
|
|||
import { spawnSync } from "node:child_process";
|
||||
import {
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
readdirSync,
|
||||
rmSync,
|
||||
symlinkSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
|
||||
import { validReview, writeReviewArtifacts } from "./pr-review-artifact-fixture.js";
|
||||
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
const scripts = join(process.cwd(), "scripts");
|
||||
const describePosix = process.platform === "win32" ? describe.skip : describe;
|
||||
|
||||
function fixture() {
|
||||
const root = tempDirs.make("openclaw-pr-correction-");
|
||||
const env = {
|
||||
...process.env,
|
||||
GIT_CONFIG_GLOBAL: "/dev/null",
|
||||
GIT_CONFIG_NOSYSTEM: "1",
|
||||
GIT_AUTHOR_NAME: "Fixture",
|
||||
GIT_AUTHOR_EMAIL: "fixture@example.invalid",
|
||||
GIT_COMMITTER_NAME: "Fixture",
|
||||
GIT_COMMITTER_EMAIL: "fixture@example.invalid",
|
||||
};
|
||||
const git = (...args: string[]) => {
|
||||
const result = spawnSync("git", args, { cwd: root, env, encoding: "utf8" });
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
return result.stdout.trim();
|
||||
};
|
||||
git("init", "-q", "-b", "topic");
|
||||
git("config", "commit.gpgSign", "false");
|
||||
git("config", "core.hooksPath", "/dev/null");
|
||||
writeFileSync(join(root, ".gitignore"), ".local/\n");
|
||||
mkdirSync(join(root, "docs"));
|
||||
writeFileSync(join(root, "docs/fix.md"), "broken\n");
|
||||
git("add", ".");
|
||||
git("commit", "-qm", "incoming");
|
||||
const incoming = git("rev-parse", "HEAD");
|
||||
const review = validReview(incoming);
|
||||
review.issueValidation.status = "valid";
|
||||
review.findings.push({
|
||||
id: "I1",
|
||||
severity: "IMPORTANT",
|
||||
title: "Incorrect behavior",
|
||||
area: "docs/fix.md",
|
||||
fix: "Correct the behavior",
|
||||
});
|
||||
writeReviewArtifacts(root, review, { headSha: incoming, files: ["docs/fix.md"] });
|
||||
const metadata = {
|
||||
number: 42,
|
||||
headRefOid: incoming,
|
||||
headRefName: "topic",
|
||||
url: "https://github.com/fixture/repo/pull/42",
|
||||
baseRepository: {
|
||||
id: "fixture-repo",
|
||||
databaseId: 1,
|
||||
nameWithOwner: "fixture/repo",
|
||||
url: "https://github.com/fixture/repo",
|
||||
},
|
||||
files: [{ path: "docs/fix.md" }],
|
||||
};
|
||||
writeFileSync(join(root, ".local/pr-meta.json"), JSON.stringify(metadata));
|
||||
writeFileSync(
|
||||
join(root, ".local/pr-meta.env"),
|
||||
`PR_NUMBER=42\nPR_HEAD=topic\nPR_HEAD_SHA=${incoming}\n`,
|
||||
);
|
||||
const run = (invocation: string, envOverrides: NodeJS.ProcessEnv = {}) =>
|
||||
spawnSync(
|
||||
"bash",
|
||||
[
|
||||
"-c",
|
||||
[
|
||||
"set -euo pipefail",
|
||||
'script_parent_dir="$1"',
|
||||
'source "$1/pr-lib/common.sh"',
|
||||
'source "$1/pr-lib/review.sh"',
|
||||
'source "$1/pr-lib/prepare-core.sh"',
|
||||
'source "$1/pr-lib/gates.sh"',
|
||||
'require_artifact() { [ -s "$1" ]; }',
|
||||
"enter_worktree() { :; }",
|
||||
'pr_git() { "${OPENCLAW_PR_GIT:-${GIT_EXEC:-git}}" "$@"; }',
|
||||
'pr_gh() { gh "$@"; }',
|
||||
"common_repo_root() { pwd; }",
|
||||
"pr_worktree_state() { jq -n --arg path \"$PWD\" '{present:true,path:$path}'; }",
|
||||
"read_pr_view_json() { cat .local/pr-meta.json; }",
|
||||
'review_guard() { REVIEW_MODE=pr; source .local/pr-meta.env; [ "$(git rev-parse HEAD)" = "$PR_HEAD_SHA" ]; }',
|
||||
"print_review_stdout_summary() { :; }",
|
||||
"mark_pr_operation_side_effects_started() { touch .local/side-effects; }",
|
||||
'checkout_pr_worktree_target() { git checkout -q --detach "$2"; }',
|
||||
"pr_meta_json() { cat .local/pr-meta.json; }",
|
||||
"resolve_pr_author_access_at_prepare() { echo external; }",
|
||||
'fetch_pr_head() { PR_HEAD_OBSERVATION="$4"; git update-ref "$3" "$2"; }',
|
||||
invocation,
|
||||
].join("\n"),
|
||||
"correction-fixture",
|
||||
scripts,
|
||||
],
|
||||
{ cwd: root, env: { ...env, ...envOverrides }, encoding: "utf8" },
|
||||
);
|
||||
const commitFix = () => {
|
||||
writeFileSync(join(root, "docs/fix.md"), "corrected\n");
|
||||
git("add", "docs/fix.md");
|
||||
git("commit", "-qm", "fix behavior");
|
||||
};
|
||||
const approve = () => {
|
||||
const path = join(root, ".local/correction-review.json");
|
||||
const correction = JSON.parse(readFileSync(path, "utf8"));
|
||||
Object.assign(correction, validReview(git("rev-parse", "HEAD")));
|
||||
correction.recommendation = "READY FOR /prepare-pr";
|
||||
correction.issueValidation.status = "valid";
|
||||
correction.correction.resolvedFindings[0].resolution = "The corrected behavior is verified.";
|
||||
writeFileSync(path, JSON.stringify(correction));
|
||||
};
|
||||
return { root, run, git, incoming, review, commitFix, approve };
|
||||
}
|
||||
|
||||
describePosix("native correction preparation", () => {
|
||||
it("keeps normal READY preparation available", () => {
|
||||
const f = fixture();
|
||||
f.review.recommendation = "READY FOR /prepare-pr";
|
||||
f.review.findings = [];
|
||||
writeFileSync(join(f.root, ".local/review.json"), JSON.stringify(f.review));
|
||||
expect(f.run("prepare_init 42").status).toBe(0);
|
||||
expect(f.run("require_prepared_review 42").status).toBe(0);
|
||||
});
|
||||
|
||||
it.each(["OPENCLAW_PR_GIT", "GIT_EXEC"])(
|
||||
"uses configured Git for correction review initialization and validation via %s",
|
||||
(selector) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
const resolved = spawnSync("bash", ["-c", "command -v git"], { encoding: "utf8" });
|
||||
expect(resolved.status, resolved.stderr).toBe(0);
|
||||
const bin = join(f.root, ".local", "git-bin");
|
||||
mkdirSync(bin);
|
||||
writeFileSync(join(bin, "git"), "#!/bin/sh\necho 'unexpected PATH Git' >&2\nexit 97\n", {
|
||||
mode: 0o755,
|
||||
});
|
||||
const selected = join(f.root, ".local", "selected git");
|
||||
symlinkSync(resolved.stdout.trim(), selected);
|
||||
const env = {
|
||||
PATH: `${bin}:${process.env.PATH}`,
|
||||
OPENCLAW_PR_GIT: selector === "OPENCLAW_PR_GIT" ? selected : "",
|
||||
GIT_EXEC: selector === "GIT_EXEC" ? selected : "",
|
||||
};
|
||||
const initialized = f.run("prepare_correction_review_init 42", env);
|
||||
expect(initialized.status, initialized.stdout + initialized.stderr).toBe(0);
|
||||
f.approve();
|
||||
const validated = f.run("require_prepared_review 42", env);
|
||||
expect(validated.status, validated.stdout + validated.stderr).toBe(0);
|
||||
expect(validated.stdout).toContain("READY FOR /prepare-pr");
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves the observed PR through ordinary prepare-run without treating it as correction mode", () => {
|
||||
const f = fixture();
|
||||
f.review.recommendation = "READY FOR /prepare-pr";
|
||||
f.review.findings = [];
|
||||
writeFileSync(join(f.root, ".local/review.json"), JSON.stringify(f.review));
|
||||
const result = f.run(
|
||||
[
|
||||
'pr_observe() { echo "unexpected replacement observation" >&2; return 99; }',
|
||||
'prepare_gates() { [ "$1" = 42 ] && [ "$2" = "$(cat .local/pr-meta.json)" ] && touch .local/gates-reached; }',
|
||||
'prepare_push() { [ "$1" = 42 ] && [ "$2" = "$(cat .local/pr-meta.json)" ] && touch .local/push-reached; }',
|
||||
'prepare_run 42 "$(cat .local/pr-meta.json)"',
|
||||
].join("\n"),
|
||||
);
|
||||
expect(result.status, result.stdout + result.stderr).toBe(0);
|
||||
expect(readFileSync(join(f.root, ".local/prep-context.env"), "utf8")).toContain(
|
||||
"PREP_REVIEW_MODE=ready",
|
||||
);
|
||||
expect(existsSync(join(f.root, ".local/gates-reached"))).toBe(true);
|
||||
expect(existsSync(join(f.root, ".local/push-reached"))).toBe(true);
|
||||
expect(f.git("rev-parse", "HEAD")).toBe(f.incoming);
|
||||
});
|
||||
|
||||
it("preserves default NEEDS WORK refusal and explicitly admits only correction preparation", () => {
|
||||
const f = fixture();
|
||||
const original = readFileSync(join(f.root, ".local/review.json"), "utf8");
|
||||
const denied = f.run("prepare_init 42");
|
||||
expect(denied.status).toBe(1);
|
||||
expect(denied.stdout).toContain("requires a validated READY");
|
||||
expect(existsSync(join(f.root, ".local/side-effects"))).toBe(false);
|
||||
const admitted = f.run("prepare_init 42 '' correction");
|
||||
expect(admitted.status, admitted.stderr).toBe(0);
|
||||
expect(f.git("rev-parse", "HEAD")).toBe(f.incoming);
|
||||
expect(readFileSync(join(f.root, ".local/review.json"), "utf8")).toBe(original);
|
||||
expect(f.run("require_prepared_review 42").status).toBe(1);
|
||||
expect(existsSync(join(f.root, ".local/gates.env"))).toBe(false);
|
||||
});
|
||||
|
||||
it.each(["NEEDS DISCUSSION", "NOT USEFUL (CLOSE)"])(
|
||||
"does not admit %s for correction",
|
||||
(recommendation) => {
|
||||
const f = fixture();
|
||||
f.review.recommendation = recommendation;
|
||||
writeFileSync(join(f.root, ".local/review.json"), JSON.stringify(f.review));
|
||||
const result = f.run("prepare_init 42 '' correction");
|
||||
expect(result.status).toBe(1);
|
||||
expect(existsSync(join(f.root, ".local/side-effects"))).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it("requires complete exact-candidate review, then rejects subsequent source drift", () => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
expect(f.run("require_prepared_review 42").status).toBe(1);
|
||||
f.approve();
|
||||
const accepted = f.run("require_prepared_review 42");
|
||||
expect(accepted.status, accepted.stderr).toBe(0);
|
||||
f.git("commit", "-q", "--allow-empty", "-m", "candidate moved");
|
||||
expect(f.run("require_prepared_review 42").status).toBe(1);
|
||||
});
|
||||
|
||||
it.each(["gates", "push", "sync"])(
|
||||
"refuses %s before its execution/publication owner without candidate approval",
|
||||
(operation) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
writeFileSync(join(f.root, ".local/gates.env"), "GATES_MODE=full\n");
|
||||
const result = f.run(
|
||||
[
|
||||
'source "$script_parent_dir/pr-lib/gates.sh"',
|
||||
"resolve_pr_gates_remote_mode() { echo local; }",
|
||||
"mark_pr_operation_side_effects_if_available() { :; }",
|
||||
"derive_prepare_gate_change_plan() { touch .local/execution-reached; return 1; }",
|
||||
"push_prep_head_to_pr_branch() { touch .local/execution-reached; return 1; }",
|
||||
operation === "gates"
|
||||
? "prepare_gates 42"
|
||||
: operation === "push"
|
||||
? "prepare_push 42"
|
||||
: "prepare_sync_head 42",
|
||||
].join("\n"),
|
||||
);
|
||||
expect(result.status).toBe(1);
|
||||
expect(result.stderr).toContain("correction-review.json");
|
||||
expect(existsSync(join(f.root, ".local/execution-reached"))).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects candidate review when the corrected history drops the incoming commit", () => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
f.git("checkout", "--orphan", "replacement");
|
||||
f.git("add", ".");
|
||||
f.git("commit", "-qm", "rewritten source");
|
||||
f.git("branch", "-f", "pr-42-prep", "HEAD");
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(1);
|
||||
});
|
||||
|
||||
it.each(["incoming review", "finding resolution", "foreign candidate", "lost correction mode"])(
|
||||
"refuses changed %s",
|
||||
(kind) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
if (kind === "incoming review") {
|
||||
writeFileSync(join(f.root, ".local/review.json"), `${JSON.stringify(f.review)}\n\n`);
|
||||
} else if (kind === "lost correction mode") {
|
||||
const path = join(f.root, ".local/prep-context.env");
|
||||
writeFileSync(
|
||||
path,
|
||||
readFileSync(path, "utf8").replace(
|
||||
"PREP_REVIEW_MODE=correction",
|
||||
"PREP_REVIEW_MODE=ready",
|
||||
),
|
||||
);
|
||||
} else {
|
||||
const path = join(f.root, ".local/correction-review.json");
|
||||
const review = JSON.parse(readFileSync(path, "utf8"));
|
||||
if (kind === "finding resolution") {
|
||||
review.correction.resolvedFindings = [];
|
||||
} else {
|
||||
review.pr.headSha = "a".repeat(40);
|
||||
}
|
||||
writeFileSync(path, JSON.stringify(review));
|
||||
}
|
||||
expect(f.run("require_prepared_review 42").status).toBe(1);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([false, true])(
|
||||
"binds recovered approval to incoming review bytes, changed=%s",
|
||||
(changed) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
const candidate = f.git("rev-parse", "HEAD");
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const names = ["correction-review.json", "correction-incoming-review.json"];
|
||||
const retained = names.map((name) => ({
|
||||
name,
|
||||
bytes: readFileSync(join(f.root, ".local", name)),
|
||||
}));
|
||||
f.git("checkout", "--detach", f.incoming);
|
||||
if (changed) {
|
||||
const finding = f.review.findings[0];
|
||||
if (!finding) {
|
||||
throw new Error("Missing incoming fixture finding");
|
||||
}
|
||||
finding.fix = "A different obligation with the same I1 identifier";
|
||||
writeFileSync(join(f.root, ".local/review.json"), JSON.stringify(f.review));
|
||||
}
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.git("reset", "--hard", candidate);
|
||||
retained.forEach(({ name, bytes }) => writeFileSync(join(f.root, ".local", name), bytes));
|
||||
const result = f.run("require_prepared_review 42");
|
||||
expect(result.status, result.stderr).toBe(changed ? 1 : 0);
|
||||
if (changed) {
|
||||
expect(result.stderr).toContain("exact incoming review bytes");
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["push", "sync"])("requires exact gates before correction %s", (operation) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
const qualified = f.git("rev-parse", "HEAD");
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const publish = () =>
|
||||
f.run(
|
||||
[
|
||||
"push_prep_head_to_pr_branch() { touch .local/execution-reached; return 73; }",
|
||||
operation === "push" ? "prepare_push 42" : "prepare_sync_head 42",
|
||||
].join("\n"),
|
||||
);
|
||||
expect(publish().status).toBe(1);
|
||||
expect(existsSync(join(f.root, ".local/execution-reached"))).toBe(false);
|
||||
writeFileSync(
|
||||
join(f.root, ".local/gates.env"),
|
||||
`PR_NUMBER=42\nGATES_MODE=full\nLAST_VERIFIED_HEAD_SHA=${qualified}\nFULL_GATES_HEAD_SHA=${qualified}\n`,
|
||||
);
|
||||
f.git("commit", "-q", "--allow-empty", "-m", "new candidate same tree");
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
expect(publish().status).toBe(1);
|
||||
expect(existsSync(join(f.root, ".local/execution-reached"))).toBe(false);
|
||||
const current = f.git("rev-parse", "HEAD");
|
||||
writeFileSync(
|
||||
join(f.root, ".local/gates.env"),
|
||||
`PR_NUMBER=42\nGATES_MODE=full\nLAST_VERIFIED_HEAD_SHA=${current}\nFULL_GATES_HEAD_SHA=${current}\n`,
|
||||
);
|
||||
expect(publish().status).toBe(73);
|
||||
expect(existsSync(join(f.root, ".local/execution-reached"))).toBe(true);
|
||||
});
|
||||
|
||||
it("does not qualify a correction with deferred GitHub gates", () => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const result = f.run(
|
||||
[
|
||||
"resolve_pr_gates_remote_mode() { echo github; }",
|
||||
"mark_pr_operation_side_effects_if_available() { :; }",
|
||||
"derive_prepare_gate_change_plan() { PREPARE_GATE_CHANGED_FILES=docs/fix.md; PREPARE_GATE_DOCS_ONLY=false; PREPARE_GATE_CHANGELOG_ONLY=false; PREPARE_GATE_CHANGELOG_REQUIRED=false; PREPARE_GATE_CHANGELOG_UPDATE=false; }",
|
||||
"push_prep_head_to_pr_branch() { touch .local/execution-reached; return 73; }",
|
||||
"prepare_gates 42",
|
||||
"prepare_push 42",
|
||||
].join("\n"),
|
||||
);
|
||||
expect(result.status, result.stdout + result.stderr).toBe(1);
|
||||
const gates = readFileSync(join(f.root, ".local/gates.env"), "utf8");
|
||||
expect(gates).toContain("GATES_MODE=github_pending");
|
||||
expect(gates).not.toContain("GATES_PASSED_AT");
|
||||
expect(gates).not.toContain("LAST_VERIFIED_HEAD_SHA");
|
||||
expect(existsSync(join(f.root, ".local/execution-reached"))).toBe(false);
|
||||
expect(f.run("prepare_sync_head 42").status).toBe(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ fork: true, authorization: "granted" },
|
||||
{ fork: false, authorization: "granted" },
|
||||
{ fork: false, authorization: "denied" },
|
||||
{ fork: false, authorization: "revoked" },
|
||||
])(
|
||||
"checks native pending-route eligibility before correction publication, fork=$fork, authorization=$authorization",
|
||||
({ fork, authorization }) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const target = JSON.stringify({
|
||||
state: "OPEN",
|
||||
isCrossRepository: fork,
|
||||
baseRefName: "main",
|
||||
baseRefOid: f.incoming,
|
||||
headRefOid: f.incoming,
|
||||
});
|
||||
const result = f.run(
|
||||
[
|
||||
"resolve_pr_gates_remote_mode() { echo crabbox-aws; }",
|
||||
"mark_pr_operation_side_effects_if_available() { :; }",
|
||||
`require_active_org_admin_for_crabbox_gate() {
|
||||
local phase=qualification
|
||||
[ ! -e .local/publication-started ] || phase=publication
|
||||
printf '%s\\n' "$phase" >> .local/admin-checks
|
||||
[ '${authorization}' != denied ] || return 1
|
||||
if [ '${authorization}' = revoked ] && [ "$phase" = publication ]; then return 1; fi
|
||||
echo fixture-admin
|
||||
}`,
|
||||
"derive_prepare_gate_change_plan() { PREPARE_GATE_CHANGED_FILES=docs/fix.md; PREPARE_GATE_DOCS_ONLY=false; PREPARE_GATE_CHANGELOG_ONLY=false; PREPARE_GATE_CHANGELOG_REQUIRED=false; PREPARE_GATE_CHANGELOG_UPDATE=false; }",
|
||||
`gh() { printf '%s\\n' '${target}'; }`,
|
||||
"push_prep_head_to_pr_branch() { touch .local/execution-reached; return 73; }",
|
||||
"prepare_gates 42",
|
||||
"touch .local/publication-started",
|
||||
"prepare_push 42",
|
||||
].join("\n"),
|
||||
);
|
||||
const allowed = !fork && authorization === "granted";
|
||||
expect(result.status, result.stdout + result.stderr).toBe(allowed ? 73 : 1);
|
||||
expect(existsSync(join(f.root, ".local/execution-reached"))).toBe(allowed);
|
||||
const checks = readFileSync(join(f.root, ".local/admin-checks"), "utf8").trim().split("\n");
|
||||
expect(checks).toContain("qualification");
|
||||
if (authorization === "denied") {
|
||||
expect(checks).not.toContain("publication");
|
||||
expect(existsSync(join(f.root, ".local/gates.env"))).toBe(false);
|
||||
} else {
|
||||
expect(checks).toContain("publication");
|
||||
}
|
||||
const sync = f.run("push_prep_head_to_pr_branch() { return 73; }; prepare_sync_head 42");
|
||||
expect(sync.status, sync.stderr).toBe(1);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["missing", "stale"])(
|
||||
"ignores %s Markdown presentation during correction admission",
|
||||
(kind) => {
|
||||
const f = fixture();
|
||||
const markdown = join(f.root, ".local/review.md");
|
||||
if (kind === "missing") {
|
||||
rmSync(markdown);
|
||||
} else {
|
||||
writeFileSync(markdown, "Obsolete presentation, not review authority\n");
|
||||
}
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
writeFileSync(join(f.root, ".local/correction-review.md"), "NEEDS WORK\n");
|
||||
const result = f.run("require_prepared_review 42");
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
expect(result.stdout).toContain("READY FOR /prepare-pr");
|
||||
},
|
||||
);
|
||||
|
||||
it("includes runtime fixup paths in candidate review even when incoming scope is docs", () => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
mkdirSync(join(f.root, "src"));
|
||||
writeFileSync(join(f.root, "src/fix.ts"), "export const fixed = true;\n");
|
||||
f.git("add", "src/fix.ts");
|
||||
f.git("commit", "-qm", "fix runtime");
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const result = f.run("require_prepared_review 42");
|
||||
expect(result.status).toBe(1);
|
||||
expect(result.stderr).toContain("runtime file changes require");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["git", "unchanged"],
|
||||
["git", "JSON"],
|
||||
["git", "Markdown"],
|
||||
["graphql", "unchanged"],
|
||||
["graphql", "JSON"],
|
||||
["graphql", "Markdown"],
|
||||
])(
|
||||
"revalidates JSON authority immediately before %s publication after %s change",
|
||||
(route, change) => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const head = f.git("rev-parse", "HEAD");
|
||||
writeFileSync(
|
||||
join(f.root, ".local/gates.env"),
|
||||
`PR_NUMBER=42\nGATES_MODE=full\nLAST_VERIFIED_HEAD_SHA=${head}\nFULL_GATES_HEAD_SHA=${head}\n`,
|
||||
);
|
||||
const mutation =
|
||||
change === "JSON"
|
||||
? "printf '\\n' >> .local/correction-review.json"
|
||||
: change === "Markdown"
|
||||
? "printf 'obsolete presentation\\n' > .local/correction-review.md"
|
||||
: ":";
|
||||
const result = f.run(
|
||||
[
|
||||
'source "$script_parent_dir/pr-lib/push.sh"',
|
||||
"PREP_PUBLICATION_PR=42; PREP_PUBLICATION_ALLOW_PENDING=false",
|
||||
"PREP_PUBLICATION_REVIEW_SNAPSHOT=$(correction_review_snapshot 42)",
|
||||
'pr_git() { if [ "$1" = push ]; then touch .local/publication; return 0; fi; git "$@"; }',
|
||||
`pr_gh_plain() { touch .local/publication; printf '%s\\n' '{"data":{"createCommitOnBranch":{"commit":{"oid":"${head}"}}}}'; }`,
|
||||
`verify_prep_first_parent_range_signed() { ${mutation}; return 0; }`,
|
||||
`verify_prep_head_extends_hosted_head() { git merge-base --is-ancestor "$1" HEAD || return 1; ${mutation}; }`,
|
||||
"PRHEAD_REMOTE_URL=https://example.invalid/repo.git; OPENCLAW_PR_PUSH_MODE=git",
|
||||
'revalidate_pr_publication() { [ "$1" = 42 ] && [ "$2" = fixture-observation ] && [ "$3" = topic ] && [ "$5" = "$(git rev-parse HEAD)" ]; }',
|
||||
route === "git"
|
||||
? `push_prep_head_once topic ${f.incoming} ${head} 42 fixture-observation`
|
||||
: `graphql_push_to_fork fixture/repo topic ${f.incoming} 42 fixture-observation ${head}`,
|
||||
].join("\n"),
|
||||
);
|
||||
expect(result.status, result.stdout + result.stderr).toBe(change === "JSON" ? 1 : 0);
|
||||
expect(existsSync(join(f.root, ".local/publication"))).toBe(change !== "JSON");
|
||||
if (change === "JSON") {
|
||||
expect(result.stderr).toContain("Correction review authority changed");
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("retains prior candidate reviews when initializing another review", () => {
|
||||
const f = fixture();
|
||||
expect(f.run("prepare_init 42 '' correction").status).toBe(0);
|
||||
f.commitFix();
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
f.approve();
|
||||
const original = readFileSync(join(f.root, ".local/correction-review.json"), "utf8");
|
||||
expect(f.run("prepare_correction_review_init 42").status).toBe(0);
|
||||
const retained = readdirSync(join(f.root, ".local")).find((name) =>
|
||||
name.startsWith("correction-review-retained."),
|
||||
);
|
||||
expect(retained).toBeTruthy();
|
||||
if (!retained) {
|
||||
throw new Error("Missing retained review");
|
||||
}
|
||||
expect(readFileSync(join(f.root, ".local", retained, "correction-review.json"), "utf8")).toBe(
|
||||
original,
|
||||
);
|
||||
expect(f.run("require_prepared_review 42").status).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -482,6 +482,7 @@ else if (endpoint === "graphql" && args.some(arg => arg.includes("repository(own
|
|||
'repo_root() { printf "%s\\n" "$PWD"; }',
|
||||
'source "$script_parent_dir/pr-lib/gates.sh"',
|
||||
'source "$script_parent_dir/pr-lib/merge.sh"',
|
||||
'source "$script_parent_dir/pr-lib/review.sh"',
|
||||
command,
|
||||
].join("\n"),
|
||||
],
|
||||
|
|
@ -585,7 +586,7 @@ refresh_main_snapshot() { PR_MAIN_SHA=${mainSha}; }
|
|||
verify_prep_branch_matches_prepared_head() { :; }
|
||||
review_artifact_preflight() { :; }
|
||||
validate_review_artifact_data() { :; }
|
||||
require_ready_review_recommendation() { :; }
|
||||
require_prepared_review() { :; }
|
||||
mark_pr_operation_side_effects_started() { :; }
|
||||
is_canonical_pr_number() { [[ "$1" =~ ^[1-9][0-9]*$ ]]; }
|
||||
merge_outcome_load_local() { MERGE_OUTCOME_OID=""; MERGE_OUTCOME_RECORD=""; }
|
||||
|
|
|
|||
210
test/scripts/pr-merge-correction.test.ts
Normal file
210
test/scripts/pr-merge-correction.test.ts
Normal file
|
|
@ -0,0 +1,210 @@
|
|||
import { execFileSync } from "node:child_process";
|
||||
import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { expect, it } from "vitest";
|
||||
import { createMergeOutcomeFixtureHarness } from "./pr-merge-outcome.test-support.js";
|
||||
import { validReview, writeReviewArtifacts } from "./pr-review-artifact-fixture.js";
|
||||
|
||||
const { fixture, outcomeRef, describePosix, scripts, nodeExecutable, gitEnv } =
|
||||
createMergeOutcomeFixtureHarness();
|
||||
|
||||
function configureCorrection(f: ReturnType<typeof fixture>) {
|
||||
const incoming = f.sourceCommits[0];
|
||||
if (!incoming) {
|
||||
throw new Error("Missing incoming fixture commit");
|
||||
}
|
||||
const candidate = f.git(["rev-parse", "HEAD"], undefined, f.worktree);
|
||||
const review = validReview(incoming);
|
||||
review.pr.number = 123;
|
||||
review.issueValidation.status = "valid";
|
||||
review.findings.push({
|
||||
id: "I1",
|
||||
severity: "IMPORTANT",
|
||||
title: "Wrong behavior",
|
||||
area: "owner.txt",
|
||||
fix: "Correct behavior",
|
||||
});
|
||||
writeReviewArtifacts(f.worktree, review, {
|
||||
headSha: incoming,
|
||||
prNumber: 123,
|
||||
files: ["owner.txt"],
|
||||
});
|
||||
const jsonOid = f.git(
|
||||
["hash-object", "--no-filters", ".local/review.json"],
|
||||
undefined,
|
||||
f.worktree,
|
||||
);
|
||||
writeFileSync(
|
||||
join(f.worktree, ".local/prep-context.env"),
|
||||
`PR_NUMBER=123\nPR_HEAD_SHA_BEFORE=${incoming}\nPREP_BRANCH=pr-123-prep\nPREP_REVIEW_MODE=correction\nPREP_INCOMING_JSON_OID=${jsonOid}\n`,
|
||||
);
|
||||
execFileSync(
|
||||
nodeExecutable,
|
||||
[join(scripts, "pr-lib/correction-review.mjs"), "init", "123", incoming, candidate, jsonOid],
|
||||
{ cwd: f.worktree, env: gitEnv },
|
||||
);
|
||||
const reviewPath = join(f.worktree, ".local/correction-review.json");
|
||||
const correction = JSON.parse(readFileSync(reviewPath, "utf8"));
|
||||
Object.assign(correction, validReview(candidate));
|
||||
correction.pr.number = 123;
|
||||
correction.recommendation = "READY FOR /prepare-pr";
|
||||
correction.issueValidation.status = "valid";
|
||||
correction.correction.resolvedFindings[0].resolution = "Corrected behavior reviewed.";
|
||||
writeFileSync(reviewPath, JSON.stringify(correction));
|
||||
writeFileSync(
|
||||
join(f.worktree, ".local/gates.env"),
|
||||
`PR_NUMBER=123\nGATES_MODE=full\nLAST_VERIFIED_HEAD_SHA=${candidate}\nFULL_GATES_HEAD_SHA=${candidate}\n`,
|
||||
);
|
||||
return f;
|
||||
}
|
||||
|
||||
function correctionFixture() {
|
||||
return configureCorrection(fixture(undefined, [["incoming bug\n"], ["corrected\n"]]));
|
||||
}
|
||||
|
||||
describePosix("correction authority through native merge admission", () => {
|
||||
it("merges an exactly reviewed and qualified correction while retaining original NEEDS WORK", () => {
|
||||
const f = correctionFixture();
|
||||
const result = f.run();
|
||||
expect(result.status, result.output).toBe(0);
|
||||
expect(f.state().mutations).toBe(1);
|
||||
});
|
||||
it.each(["correction-review.json", "prep-context.env", "gates.env", "prep.env", "pr-meta.env"])(
|
||||
"refuses changed %s after CI checks and before intent",
|
||||
(artifact) => {
|
||||
const f = correctionFixture();
|
||||
f.save({ ...f.state(), duringChecks: { artifact } });
|
||||
const result = f.run();
|
||||
expect(result.status, result.output).toBe(1);
|
||||
expect(f.state().mutations).toBe(0);
|
||||
expect(() => f.record()).toThrow();
|
||||
},
|
||||
);
|
||||
it("refuses correction approval changed during final remote review admission", () => {
|
||||
const f = correctionFixture();
|
||||
f.save({ ...f.state(), tamperCorrectionAtFinalReview: true });
|
||||
const result = f.run();
|
||||
expect(result.status, result.output).toBe(1);
|
||||
expect(result.output).toContain("Correction review authority changed");
|
||||
expect(f.state().mutations).toBe(0);
|
||||
expect(() => f.record()).toThrow();
|
||||
});
|
||||
|
||||
it("accepts the verified GraphQL local/hosted correction pair", () => {
|
||||
const f = correctionFixture();
|
||||
const incoming = f.sourceCommits[0];
|
||||
if (!incoming) {
|
||||
throw new Error("Missing incoming fixture commit");
|
||||
}
|
||||
const hosted = f.commit(f.tree("corrected\n"), [incoming], "Hosted correction\n");
|
||||
f.git([
|
||||
"push",
|
||||
"-q",
|
||||
"--force",
|
||||
"origin",
|
||||
`${hosted}:refs/pull/123/head`,
|
||||
`${hosted}:refs/heads/topic`,
|
||||
]);
|
||||
f.prepare(hosted, f.base, f.head);
|
||||
configureCorrection(f);
|
||||
const state = f.state();
|
||||
state.pr.headRefOid = hosted;
|
||||
state.issueComments[0]!.body = state.issueComments[0]!.body.replace(f.head, hosted);
|
||||
f.save(state);
|
||||
const result = f.run();
|
||||
expect(result.status, result.output).toBe(0);
|
||||
expect(f.record().localHead).toBe(f.head);
|
||||
expect(f.record().head).toBe(hosted);
|
||||
});
|
||||
|
||||
it.each(["none", "incoming digest", "foreign review", "stale gates", "wrong replacement"])(
|
||||
"handles explicit correction replacement with %s",
|
||||
(fault) => {
|
||||
const f = correctionFixture();
|
||||
f.save({ ...f.state(), mode: "unapplied" });
|
||||
expect(f.run().status).toBe(1);
|
||||
f.recover();
|
||||
const previous = f.git(["rev-parse", outcomeRef]);
|
||||
const replacement = f.commit(
|
||||
f.tree("replacement correction\n"),
|
||||
[f.head],
|
||||
"Reviewed replacement\n",
|
||||
);
|
||||
f.git(["-C", f.worktree, "checkout", "-B", "pr-123-prep", replacement]);
|
||||
f.git([
|
||||
"push",
|
||||
"-q",
|
||||
"--force",
|
||||
"origin",
|
||||
`${replacement}:refs/pull/123/head`,
|
||||
`${replacement}:refs/heads/topic`,
|
||||
]);
|
||||
f.prepare(replacement);
|
||||
configureCorrection(f);
|
||||
const state = f.state();
|
||||
state.mode = "success";
|
||||
state.pr.headRefOid = replacement;
|
||||
state.issueComments[0]!.body = state.issueComments[0]!.body.replace(f.head, replacement);
|
||||
f.save(state);
|
||||
if (fault === "incoming digest") {
|
||||
const path = join(f.worktree, ".local/review.json");
|
||||
writeFileSync(path, readFileSync(path, "utf8") + "\n");
|
||||
} else if (fault === "foreign review") {
|
||||
const path = join(f.worktree, ".local/correction-review.json");
|
||||
const review = JSON.parse(readFileSync(path, "utf8"));
|
||||
review.pr.headSha = f.head;
|
||||
writeFileSync(path, JSON.stringify(review));
|
||||
} else if (fault === "stale gates") {
|
||||
const path = join(f.worktree, ".local/gates.env");
|
||||
writeFileSync(path, readFileSync(path, "utf8").replaceAll(replacement, f.head));
|
||||
}
|
||||
const result = f.run(
|
||||
false,
|
||||
f.repo,
|
||||
"squash",
|
||||
previous,
|
||||
fault === "wrong replacement" ? f.head : replacement,
|
||||
);
|
||||
expect(result.status, result.output).toBe(fault === "none" ? 0 : 1);
|
||||
expect(f.state().mutations).toBe(fault === "none" ? 2 : 1);
|
||||
if (fault !== "none") {
|
||||
expect(f.git(["rev-parse", outcomeRef])).toBe(previous);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["LOCAL_PREP_HEAD_SHA", "PREP_HEAD_SHA"] as const)(
|
||||
"direct verify rejects changed receipt %s after checks",
|
||||
(receiptField) => {
|
||||
const f = correctionFixture();
|
||||
f.save({ ...f.state(), duringChecks: { receiptField } });
|
||||
const result = f.verify();
|
||||
expect(result.status, result.output).toBe(1);
|
||||
expect(result.output).toContain("Correction review authority changed");
|
||||
expect(f.state().mutations).toBe(0);
|
||||
},
|
||||
);
|
||||
|
||||
it("direct merge-verify refuses missing correction approval", () => {
|
||||
const f = correctionFixture();
|
||||
rmSync(join(f.worktree, ".local/correction-review.json"));
|
||||
const result = f.verify();
|
||||
expect(result.status, result.output).toBe(1);
|
||||
expect(f.state().mutations).toBe(0);
|
||||
});
|
||||
it("reconciles an accepted correction merge without disposable review files", () => {
|
||||
const f = correctionFixture();
|
||||
f.save({ ...f.state(), mode: "applied-merged" });
|
||||
const result = f.run();
|
||||
expect(result.status, result.output).toBe(0);
|
||||
for (const name of ["correction-review.json", "review.json", "review.md"]) {
|
||||
const path = join(f.worktree, ".local", name);
|
||||
if (existsSync(path)) {
|
||||
rmSync(path);
|
||||
}
|
||||
}
|
||||
const again = f.run();
|
||||
expect(again.status, again.output).toBe(0);
|
||||
expect(f.state().mutations).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -268,6 +268,7 @@ export function createMergeOutcomeFixtureHarness() {
|
|||
user: { id: number; login: string; type: string };
|
||||
}>,
|
||||
issueCommentReads: 0,
|
||||
tamperCorrectionAtFinalReview: false,
|
||||
issueCommentsErrorAt: 0,
|
||||
comments: [] as { body: string; html_url: string }[],
|
||||
posts: 0,
|
||||
|
|
@ -283,7 +284,12 @@ export function createMergeOutcomeFixtureHarness() {
|
|||
requiredCheckName: "CI",
|
||||
refusalCapture: "error: string rewrite protection blocked unsafe input\n",
|
||||
ciExit: 0,
|
||||
duringChecks: null as null | { head?: string; artifact?: string; bodyPath?: string },
|
||||
duringChecks: null as null | {
|
||||
head?: string;
|
||||
artifact?: string;
|
||||
bodyPath?: string;
|
||||
receiptField?: "LOCAL_PREP_HEAD_SHA" | "PREP_HEAD_SHA";
|
||||
},
|
||||
review: true,
|
||||
ready: true,
|
||||
cleanup: "",
|
||||
|
|
@ -520,6 +526,7 @@ else if(args[0]==="pr"&&args[1]==="checks") {
|
|||
if(s.duringChecks?.bodyPath) fs.writeFileSync(s.duringChecks.bodyPath,"Changed later");
|
||||
if(s.duringChecks?.head) s.pr.headRefOid=s.duringChecks.head;
|
||||
if(s.duringChecks?.artifact) fs.appendFileSync(process.env.FIXTURE_REPO+"/.worktrees/pr-123/.local/"+s.duringChecks.artifact,"\\n# changed during checks\\n");
|
||||
if(s.duringChecks?.receiptField) { const receipt=process.env.FIXTURE_REPO+"/.worktrees/pr-123/.local/prep.env"; fs.writeFileSync(receipt,fs.readFileSync(receipt,"utf8").replace(new RegExp("^"+s.duringChecks.receiptField+"=.*$","m"),s.duringChecks.receiptField+"="+main())); }
|
||||
out([{name:s.requiredCheckName,bucket:s.gates,state:s.gates==="pass"?"SUCCESS":"FAILURE"}]);}
|
||||
else if(args[0]==="pr"&&args[1]==="view") {
|
||||
const fields=args[args.indexOf("--json")+1].split(",");
|
||||
|
|
@ -634,6 +641,7 @@ else if(args[0]==="pr"&&args[1]==="view") {
|
|||
} else {
|
||||
if(!args.includes("Cache-Control: max-age=0")) fail("missing live comment header");
|
||||
s.issueCommentReads++;
|
||||
if(s.issueCommentReads>1&&s.tamperCorrectionAtFinalReview) fs.appendFileSync(process.env.FIXTURE_REPO+"/.worktrees/pr-123/.local/correction-review.json","\\n");
|
||||
if(s.tamperMergeBody) {
|
||||
const local=process.env.FIXTURE_REPO+"/.worktrees/pr-123/.local/";
|
||||
for(const name of fs.readdirSync(local).filter(name=>name.startsWith("merge-body."))) fs.writeFileSync(local+name,"Tampered");
|
||||
|
|
@ -666,6 +674,7 @@ source "$script_parent_dir/pr-lib/operation-lock.sh"
|
|||
source "$script_parent_dir/pr-lib/common.sh"
|
||||
source "$script_parent_dir/pr-lib/merge.sh"
|
||||
source "$script_parent_dir/pr-lib/review.sh"
|
||||
source "$script_parent_dir/pr-lib/gates.sh"
|
||||
repo_root() { printf '%s\\n' "$FIXTURE_REPO"; }
|
||||
ensure_gh_api_auth() { :; }
|
||||
verify_prep_branch_matches_prepared_head() { [ "$(command git rev-parse HEAD)" = "$2" ]; }
|
||||
|
|
@ -714,7 +723,9 @@ pr_git() {
|
|||
export FIXTURE_LEADER="$$"
|
||||
acquire_pr_operation_lock 123
|
||||
begin_pr_operation_validation_phase
|
||||
if [ -n "\${5:-}" ]; then
|
||||
if [ "\${9:-}" = verify ]; then
|
||||
merge_verify 123 '{"replacementHead":"","autoMergeRequested":false,"qualifiedRefusal":false,"observation":null}'
|
||||
elif [ -n "\${5:-}" ]; then
|
||||
merge_complete 123 "$5"
|
||||
else
|
||||
merge_run 123 "\${1:-false}" "\${2:-}" "\${3:-}" "\${4:-}" "\${6:-}" "\${7:-false}" "\${8:-}"
|
||||
|
|
@ -764,6 +775,7 @@ fi
|
|||
legacyDirectory = "",
|
||||
cancelAuto = false,
|
||||
refusalDirectory = "",
|
||||
verifyOnly = false,
|
||||
) => {
|
||||
const result = spawnSync(
|
||||
nodeExecutable,
|
||||
|
|
@ -780,6 +792,7 @@ fi
|
|||
legacyDirectory,
|
||||
String(cancelAuto),
|
||||
refusalDirectory,
|
||||
verifyOnly ? "verify" : "",
|
||||
],
|
||||
{
|
||||
cwd,
|
||||
|
|
@ -889,6 +902,7 @@ fi
|
|||
save,
|
||||
run,
|
||||
complete: (oid: string) => run(false, repo, "squash", "", "", "", oid),
|
||||
verify: () => run(false, repo, "squash", "", "", "", "", "", false, "", true),
|
||||
cancel: (oid: string) => run(false, repo, "squash", oid, "", "", "", "", true),
|
||||
recover,
|
||||
advance,
|
||||
|
|
|
|||
|
|
@ -34,7 +34,9 @@ function runGatesBash(
|
|||
`script_parent_dir='${repoRoot}/scripts'`,
|
||||
`source '${repoRoot}/scripts/pr-lib/common.sh'`,
|
||||
`source '${repoRoot}/scripts/pr-lib/gates.sh'`,
|
||||
`source '${repoRoot}/scripts/pr-lib/review.sh'`,
|
||||
"mark_pr_operation_side_effects_started() { :; }",
|
||||
"require_prepared_review() { :; }",
|
||||
...(options.sourcePush
|
||||
? [
|
||||
`source '${repoRoot}/scripts/pr-lib/worktree.sh'`,
|
||||
|
|
|
|||
|
|
@ -135,6 +135,9 @@ function runMergeVerification(
|
|||
const localDir = join(fixtureRoot, ".local");
|
||||
const head = "a".repeat(40);
|
||||
mkdirSync(localDir);
|
||||
const review = validReadyReview();
|
||||
review.pr = { number: 42, headSha: head };
|
||||
writeReviewArtifacts(fixtureRoot, review, { headSha: head, prNumber: 42 });
|
||||
writeFileSync(join(localDir, "prep.env"), `PREP_HEAD_SHA=${head}\n`);
|
||||
writeFileSync(join(localDir, "gates.env"), "GATES_MODE=full\n");
|
||||
|
||||
|
|
@ -177,6 +180,7 @@ function runMergeVerification(
|
|||
'script_parent_dir=$(cd "$(dirname "$1")/.." && pwd)',
|
||||
'fixture_root="$2"',
|
||||
'source "$script_parent_dir/pr-lib/common.sh"',
|
||||
'source "$script_parent_dir/pr-lib/review.sh"',
|
||||
'source "$script_parent_dir/pr-lib/worktree.sh"',
|
||||
'source "$script_parent_dir/pr-lib/merge-outcome.sh"',
|
||||
'repo_root() { printf "%s\\n" "$fixture_root"; }',
|
||||
|
|
|
|||
|
|
@ -286,6 +286,7 @@ ${invocation}
|
|||
for (const [invocation, contents] of [
|
||||
["review_validate_artifacts 42", "invalid JSON"],
|
||||
["prepare_init 42", JSON.stringify(f.review)],
|
||||
["prepare_init 42 '' correction", JSON.stringify(f.review)],
|
||||
]) {
|
||||
writeFileSync(join(local, "review.json"), contents);
|
||||
writeFileSync(
|
||||
|
|
|
|||
|
|
@ -376,7 +376,10 @@ function resolveCommand(command: string): string {
|
|||
throw new Error(`command not found in test PATH: ${command}`);
|
||||
}
|
||||
|
||||
function seedReadyReview(fixture: ReturnType<typeof makeMismatchedWrapperRepo>) {
|
||||
function seedReadyReview(
|
||||
fixture: ReturnType<typeof makeMismatchedWrapperRepo>,
|
||||
correction = false,
|
||||
) {
|
||||
const reviewRoot = join(fixture.canonical, ".worktrees", "pr-123");
|
||||
fixture.git(fixture.canonical, [
|
||||
"worktree",
|
||||
|
|
@ -387,8 +390,17 @@ function seedReadyReview(fixture: ReturnType<typeof makeMismatchedWrapperRepo>)
|
|||
]);
|
||||
const review = validReview(fixture.localRevision);
|
||||
review.pr.number = 123;
|
||||
review.recommendation = "READY FOR /prepare-pr";
|
||||
review.recommendation = correction ? "NEEDS WORK" : "READY FOR /prepare-pr";
|
||||
review.issueValidation.status = "valid";
|
||||
if (correction) {
|
||||
review.findings.push({
|
||||
id: "I1",
|
||||
severity: "IMPORTANT",
|
||||
title: "Wrong behavior",
|
||||
area: "docs/fix.md",
|
||||
fix: "Correct behavior",
|
||||
});
|
||||
}
|
||||
writeReviewArtifacts(reviewRoot, review, { prNumber: 123, headSha: fixture.localRevision });
|
||||
}
|
||||
|
||||
|
|
@ -659,6 +671,29 @@ describe("scripts/pr wrappers", () => {
|
|||
}
|
||||
});
|
||||
|
||||
itPosix("dispatches public correction commands to the explicit native owners", () => {
|
||||
const fixture = makeMismatchedWrapperRepo();
|
||||
seedReadyReview(fixture, true);
|
||||
const ghPath = join(fixture.bin, "gh");
|
||||
writeFileSync(ghPath, readFileSync(ghPath, "utf8").replace("not-main", "main"));
|
||||
writeFileSync(
|
||||
join(fixture.canonical, "scripts/pr-lib/prepare-core.sh"),
|
||||
`prepare_init() { printf '%s\\n' "$2" | jq -e '.number == 123 and .baseRefName == "main"' >/dev/null || return 1; printf 'init <%s> <%s>\\n' "$1" "$3"; }\nprepare_correction_review_init() { printf 'review <%s>\\n' "$1"; }\n`,
|
||||
);
|
||||
for (const [command, expected] of [
|
||||
["prepare-correction-init", "init <123> <correction>"],
|
||||
["prepare-correction-review-init", "review <123>"],
|
||||
] as const) {
|
||||
const result = spawnSync(join(fixture.canonical, "scripts/pr"), [command, "123"], {
|
||||
cwd: fixture.canonical,
|
||||
env: fixture.env,
|
||||
encoding: "utf8",
|
||||
});
|
||||
expect(result.status, result.stdout + result.stderr).toBe(0);
|
||||
expect(result.stdout.trim()).toBe(expected);
|
||||
}
|
||||
});
|
||||
|
||||
itPosix("resolves an explicit merge body from the caller before supervisor cwd changes", () => {
|
||||
const fixture = makeMismatchedWrapperRepo();
|
||||
const caller = join(fixture.canonical, "nested");
|
||||
|
|
@ -851,36 +886,38 @@ describe("scripts/pr wrappers", () => {
|
|||
expect(result.stderr).not.toContain("Refusing to silently substitute");
|
||||
});
|
||||
|
||||
it.each(["prepare-run", "merge-recover"])(
|
||||
"routes mismatched %s to the canonical wrapper despite opt-in",
|
||||
(command) => {
|
||||
const fixture = makeMismatchedWrapperRepo();
|
||||
if (command === "prepare-run") {
|
||||
seedReadyReview(fixture);
|
||||
}
|
||||
const result = spawnSync(
|
||||
join(fixture.linked, "scripts", "pr"),
|
||||
[
|
||||
"--dev-wrapper",
|
||||
command,
|
||||
"123",
|
||||
...(command === "merge-recover" ? ["a".repeat(40), "--confirmed-operator-recovery"] : []),
|
||||
],
|
||||
{ cwd: fixture.linked, encoding: "utf8", env: fixture.env },
|
||||
);
|
||||
expect(result.status).toBe(1);
|
||||
expect(result.stderr).toContain(
|
||||
`subcommand '${command}' is classified landing; dev-wrapper opt-in is unavailable.`,
|
||||
);
|
||||
expect(result.stderr).toContain(anchorSubstitutionNotice(fixture.canonical));
|
||||
// The stubbed gh reports a non-main base: reaching this gate proves the
|
||||
// canonical wrapper ran instead of the mismatched local one.
|
||||
expect(result.stderr).toContain(
|
||||
"scripts/pr prepare and merge commands only support PRs targeting main; PR #123 targets not-main.",
|
||||
);
|
||||
expect(result.stdout).not.toContain("local wrapper executed");
|
||||
},
|
||||
);
|
||||
it.each([
|
||||
"prepare-run",
|
||||
"prepare-correction-init",
|
||||
"prepare-correction-review-init",
|
||||
"merge-recover",
|
||||
])("routes mismatched %s to the canonical wrapper despite opt-in", (command) => {
|
||||
const fixture = makeMismatchedWrapperRepo();
|
||||
if (command === "prepare-run" || command === "prepare-correction-init") {
|
||||
seedReadyReview(fixture, command === "prepare-correction-init");
|
||||
}
|
||||
const result = spawnSync(
|
||||
join(fixture.linked, "scripts", "pr"),
|
||||
[
|
||||
"--dev-wrapper",
|
||||
command,
|
||||
"123",
|
||||
...(command === "merge-recover" ? ["a".repeat(40), "--confirmed-operator-recovery"] : []),
|
||||
],
|
||||
{ cwd: fixture.linked, encoding: "utf8", env: fixture.env },
|
||||
);
|
||||
expect(result.status).toBe(1);
|
||||
expect(result.stderr).toContain(
|
||||
`subcommand '${command}' is classified landing; dev-wrapper opt-in is unavailable.`,
|
||||
);
|
||||
expect(result.stderr).toContain(anchorSubstitutionNotice(fixture.canonical));
|
||||
// The stubbed gh reports a non-main base: reaching this gate proves the
|
||||
// canonical wrapper ran instead of the mismatched local one.
|
||||
expect(result.stderr).toContain(
|
||||
"scripts/pr prepare and merge commands only support PRs targeting main; PR #123 targets not-main.",
|
||||
);
|
||||
expect(result.stdout).not.toContain("local wrapper executed");
|
||||
});
|
||||
|
||||
it("substitutes the canonical wrapper for a stale-base worktree once main moves the wrapper", () => {
|
||||
const fixture = makeMismatchedWrapperRepo();
|
||||
|
|
@ -2029,9 +2066,15 @@ exit 99
|
|||
}
|
||||
|
||||
itPosix.each([
|
||||
...["init", "validate-commit", "gates", "push", "run"].map(
|
||||
(mode) => ["pr-prepare", [mode, "123"], [`prepare-${mode}`, "123"]] as const,
|
||||
),
|
||||
...[
|
||||
"init",
|
||||
"correction-init",
|
||||
"correction-review-init",
|
||||
"validate-commit",
|
||||
"gates",
|
||||
"push",
|
||||
"run",
|
||||
].map((mode) => ["pr-prepare", [mode, "123"], [`prepare-${mode}`, "123"]] as const),
|
||||
[
|
||||
"pr-review",
|
||||
["123", "argument with spaces", ""],
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue