chore(pr): landing-UX — aggregated validation, lock classes, ci-dispatch, committer guard (#111287)

* chore(pr): landing-UX — aggregate artifact validation, lock classes, ci-dispatch, committer worktree guard

* fix(pr): CI lane cleanup for landing-ux — lint causes, hermetic tests, private helpers

* chore(deadcode): register pr-lib CLI entries in knip config
This commit is contained in:
Peter Steinberger
2026-07-19 02:09:55 -07:00
committed by GitHub
parent ba06fe1541
commit 84e54ab264
18 changed files with 1245 additions and 327 deletions
+2
View File
@@ -69,6 +69,8 @@ const repositoryScriptEntries = [
"scripts/oxlint-boundary-guards.mjs!",
"scripts/plugin-prerelease-liveish-matrix.mjs!",
"scripts/pr-gates-lock.mjs!",
"scripts/pr-lib/ci-dispatch.mjs!",
"scripts/pr-lib/review-artifacts.mjs!",
"scripts/pr-lib/process-group-runner.mjs!",
"scripts/pre-commit/filter-staged-files.mjs!",
"scripts/qa-coverage-report.ts!",
+1 -1
View File
@@ -20,7 +20,7 @@ This directory owns local tooling, script wrappers, and generated-artifact helpe
## PR Prepare Gates
- `scripts/pr` serializes review, prepare, and merge operations per PR across linked worktrees; `scripts/pr gc` skips active or indeterminate locks. A successful command return is the trusted synchronous-completion contract: every PR-state-mutating child must be joined before returning, and such work must never daemonize or explicitly escape both the operation group and lock-notification FD. Failed, interrupted, or controller-lost operations stay locked because detached children cannot be disproved; after verifying no child tools remain, use the reported exact-OID `scripts/pr lock-recover` command. Never bypass or delete these refs manually.
- `scripts/pr` serializes review, prepare, and merge operations per PR across linked worktrees; `scripts/pr gc` skips active or indeterminate locks. A successful command return is the trusted synchronous-completion contract: every PR-state-mutating child must be joined before returning, and such work must never daemonize or explicitly escape both the operation group and lock-notification FD. A failed command auto-releases only while its explicit pre-side-effect validation marker remains active; failures after mutation/tool launch, interruptions, and controller loss stay locked because detached children cannot be disproved. After verifying no child tools remain, use the reported exact-OID `scripts/pr lock-recover` command. Never bypass or delete these refs manually.
- `scripts/pr prepare-gates` holds the heavy-check lock for its whole local gate block (`scripts/pr-gates-lock.mjs`), so concurrent gate runs across `.worktrees` queue as units instead of dying on child lock timeouts or vitest no-output watchdog kills.
- `OPENCLAW_PR_GATES_REMOTE=testbox` runs the full-suite `pnpm test` gate on a Blacksmith Testbox through `scripts/crabbox-wrapper.mjs` (same delegation as `check:changed`); `pnpm build`/`pnpm check` stay local. The `tbx_` lease id and Actions run URL land in `.local/gates.env` (`REMOTE_GATES_*`) and `.local/prep.md`. Use it for reviewed trusted code when a loaded host makes the local 88-shard run stall-kill; contributor/fork code stays on secretless CI or sanitized AWS unless a maintainer explicitly approves credentialed execution.
+57 -2
View File
@@ -6,9 +6,9 @@ set -f
usage() {
local exit_code=${1:-2}
if [ "$exit_code" -eq 0 ]; then
printf 'Usage: %s [--force] [--fast] "commit message" "file" ["file" ...]\n' "$(basename "$0")"
printf 'Usage: %s [--force] [--fast] [--no-verify-formatted] "commit message" "file" ["file" ...]\n' "$(basename "$0")"
else
printf 'Usage: %s [--force] [--fast] "commit message" "file" ["file" ...]\n' "$(basename "$0")" >&2
printf 'Usage: %s [--force] [--fast] [--no-verify-formatted] "commit message" "file" ["file" ...]\n' "$(basename "$0")" >&2
fi
exit "$exit_code"
}
@@ -23,6 +23,7 @@ fi
force_delete_lock=false
fast_commit=false
no_verify_formatted=false
while [[ "${1:-}" == --* ]]; do
case "${1:-}" in
--force)
@@ -33,6 +34,10 @@ while [[ "${1:-}" == --* ]]; do
fast_commit=true
shift
;;
--no-verify-formatted)
no_verify_formatted=true
shift
;;
--help|-h)
usage 0
;;
@@ -207,6 +212,51 @@ for file in "${files[@]}"; do
fi
done
committer_requires_repo_formatter() {
local root="$1"
local filter="$root/scripts/pre-commit/filter-staged-files.mjs"
[ -f "$filter" ] || return 1
local filter_output selected
filter_output=$(mktemp "${TMPDIR:-/tmp}/openclaw-committer-filter.XXXXXX") || {
echo "Unable to create temporary output for formatter applicability check." >&2
return 2
}
if ! node "$filter" format -- "${files[@]}" >"$filter_output"; then
rm -f "$filter_output"
echo "Unable to determine formatter applicability: filter-staged-files.mjs failed." >&2
return 2
fi
if IFS= read -r -d '' selected <"$filter_output"; then
rm -f "$filter_output"
return 0
fi
rm -f "$filter_output"
return 1
}
repo_root=$(git rev-parse --show-toplevel)
if [ "$fast_commit" = false ] && [ "$no_verify_formatted" = false ] && \
[ -f "$repo_root/pnpm-lock.yaml" ] && \
command -v pnpm >/dev/null 2>&1 && \
[ ! -e "$repo_root/node_modules" ]
then
if committer_requires_repo_formatter "$repo_root"; then
formatter_requirement=0
else
formatter_requirement=$?
fi
if [ "$formatter_requirement" = 2 ]; then
exit 1
fi
if [ "$formatter_requirement" = 0 ]; then
echo "Missing repo dependencies: cannot run oxfmt without node_modules." >&2
echo "Run pnpm install in a normal checkout, or bypass the hook only after separate formatting proof." >&2
echo "After that proof, pass --no-verify-formatted to commit explicitly with --no-verify." >&2
exit 1
fi
fi
run_git_with_lock_retry "unstaging files" git restore --staged :/
run_git_with_lock_retry "staging files" git add --all --force -- "${files[@]}"
@@ -221,6 +271,11 @@ if [ "$fast_commit" = true ]; then
if run_git_with_lock_retry "commit" env "${commit_env[@]}" git commit --no-verify -m "$commit_message"; then
committed=true
fi
elif [ "$no_verify_formatted" = true ]; then
echo "Notice: --no-verify-formatted asserts separate formatting proof; committing with --no-verify."
if run_git_with_lock_retry "commit" git commit --no-verify -m "$commit_message"; then
committed=true
fi
else
if run_git_with_lock_retry "commit" git commit -m "$commit_message"; then
committed=true
+9 -2
View File
@@ -70,7 +70,7 @@ fi
is_locked_pr_command() {
case "$1" in
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) return 0 ;;
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 | ci-dispatch | merge-verify | merge-run) return 0 ;;
*) return 1 ;;
esac
}
@@ -124,6 +124,7 @@ Usage:
scripts/pr prepare-push <PR>
scripts/pr prepare-sync-head <PR>
scripts/pr prepare-run <PR>
scripts/pr ci-dispatch <PR>
scripts/pr merge-verify <PR>
scripts/pr merge-run <PR>
OPENCLAW_PR_MERGE_METHOD=merge|rebase preserves the PR commit series.
@@ -213,7 +214,7 @@ main() {
review-tests)
[ "$#" -ge 2 ] || { usage; exit 2; }
;;
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 | merge-verify | merge-run)
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 | merge-run)
[ "$#" -ge 1 ] || { usage; exit 2; }
;;
*)
@@ -231,6 +232,7 @@ main() {
if is_locked_pr_command "$cmd"; then
local locked_pr="${1-}"
acquire_pr_operation_lock "$locked_pr"
begin_pr_operation_validation_phase
trap 'exit 129' HUP
trap 'exit 130' INT
trap 'exit 131' QUIT
@@ -319,6 +321,11 @@ main() {
[ -n "$pr" ] || { usage; exit 2; }
prepare_run "$pr"
;;
ci-dispatch)
local pr="${1-}"
[ -n "$pr" ] || { usage; exit 2; }
ci_dispatch "$pr"
;;
merge-verify)
local pr="${1-}"
[ -n "$pr" ] || { usage; exit 2; }
+165
View File
@@ -0,0 +1,165 @@
#!/usr/bin/env node
import { isDirectRunUrl } from "../lib/direct-run.mjs";
import { execPlainGh } from "../lib/plain-gh.mjs";
const SHA_PATTERN = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/u;
function requirePrRecord({ pr, headRefName, headRefOid, isCrossRepository }) {
if (!Number.isSafeInteger(pr) || pr <= 0) {
throw new Error("Expected a positive PR number.");
}
if (typeof headRefName !== "string" || headRefName.length === 0 || headRefName.startsWith("-")) {
throw new Error("Expected a non-empty PR headRefName.");
}
if (!SHA_PATTERN.test(headRefOid)) {
throw new Error("Expected a full PR headRefOid.");
}
if (isCrossRepository === true) {
throw new Error(
`PR #${pr} comes from a fork; release-gate workflow dispatch requires a branch in the base repository at ${headRefOid}.`,
);
}
}
function buildCiDispatchArgs(record) {
requirePrRecord(record);
return [
"workflow",
"run",
"ci.yml",
"--ref",
record.headRefName,
"-f",
`target_ref=${record.headRefOid}`,
"-f",
"release_gate=true",
"-f",
`pull_request_number=${record.pr}`,
];
}
function listCiRuns(headRefOid) {
return JSON.parse(
execPlainGh(
[
"run",
"list",
"--commit",
headRefOid,
"--workflow",
"ci.yml",
"--event",
"workflow_dispatch",
"--limit",
"20",
"--json",
"databaseId,url,headSha,createdAt,status",
],
{ encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] },
),
);
}
function readCurrentPrHeadOid(pr) {
return execPlainGh(["pr", "view", String(pr), "--json", "headRefOid", "--jq", ".headRefOid"], {
encoding: "utf8",
stdio: ["ignore", "pipe", "pipe"],
}).trim();
}
function delay(milliseconds) {
return new Promise((resolve) => {
setTimeout(resolve, milliseconds);
});
}
async function dispatchCiForPr(
record,
{
pollAttempts = 10,
pollIntervalMs = 1500,
listRuns = listCiRuns,
runDispatch = (args) =>
execPlainGh(args, { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }),
readHeadOid = readCurrentPrHeadOid,
wait = delay,
} = {},
) {
requirePrRecord(record);
const priorRunIds = new Set(listRuns(record.headRefOid).map((run) => run.databaseId));
const headBeforeDispatch = readHeadOid(record.pr);
if (headBeforeDispatch !== record.headRefOid) {
throw new Error(
`PR #${record.pr} head changed before CI dispatch (expected ${record.headRefOid}, got ${headBeforeDispatch}).`,
);
}
runDispatch(buildCiDispatchArgs(record));
for (let attempt = 1; attempt <= pollAttempts; attempt += 1) {
const run = listRuns(record.headRefOid).find(
(candidate) =>
candidate.headSha === record.headRefOid &&
!priorRunIds.has(candidate.databaseId) &&
typeof candidate.url === "string" &&
candidate.url.length > 0,
);
if (run) {
const headAtObservation = readHeadOid(record.pr);
if (headAtObservation !== record.headRefOid) {
throw new Error(
`PR #${record.pr} head changed before an exact-SHA CI run became visible (expected ${record.headRefOid}, got ${headAtObservation}); verify the run before retrying.`,
);
}
return run;
}
if (attempt < pollAttempts) {
await wait(pollIntervalMs);
}
}
const headAfterDispatch = readHeadOid(record.pr);
if (headAfterDispatch !== record.headRefOid) {
throw new Error(
`PR #${record.pr} head changed while CI dispatch was being indexed (expected ${record.headRefOid}, got ${headAfterDispatch}); verify the run before retrying.`,
);
}
return undefined;
}
async function main(argv = process.argv.slice(2)) {
if (argv.length !== 4 || !["true", "false"].includes(argv[3])) {
console.error("Usage: ci-dispatch.mjs <PR> <headRefName> <headRefOid> <isCrossRepository>");
process.exitCode = 2;
return;
}
const record = {
pr: Number(argv[0]),
headRefName: argv[1],
headRefOid: argv[2],
isCrossRepository: argv[3] === "true",
};
const run = await dispatchCiForPr(record);
if (run) {
console.log(
`GitHub accepted CI dispatch for PR #${record.pr} at unchanged remote head ${record.headRefOid} (${record.headRefName}).`,
);
console.log(
"Observed a new exact-SHA manual run after dispatch; GitHub does not expose a dispatch correlation ID, so concurrent requests cannot be distinguished.",
);
console.log(`observed_run_url=${run.url}`);
} else {
console.log(
`Requested CI for PR #${record.pr} at unchanged remote head ${record.headRefOid} (${record.headRefName}).`,
);
console.log(
"run_url=pending (GitHub accepted the dispatch, but Actions has not indexed it yet)",
);
console.log(
`inspect_with=gh run list --commit ${record.headRefOid} --workflow ci.yml --event workflow_dispatch`,
);
}
}
if (isDirectRunUrl(process.argv[1], import.meta.url)) {
await main();
}
+60 -3
View File
@@ -3,8 +3,11 @@ run_hosted_prepare_gates() {
local current_head="$2"
local changelog_only="$3"
local recent_sha=""
local remote_head
remote_head=$(gh pr view "$pr" --json headRefOid --jq .headRefOid)
local remote_record remote_head remote_head_ref remote_is_cross_repository
remote_record=$(gh pr view "$pr" --json headRefName,headRefOid,isCrossRepository)
remote_head=$(printf '%s\n' "$remote_record" | jq -r .headRefOid)
remote_head_ref=$(printf '%s\n' "$remote_record" | jq -r .headRefName)
remote_is_cross_repository=$(printf '%s\n' "$remote_record" | jq -r .isCrossRepository)
if [ "$remote_head" != "$current_head" ]; then
echo "PR head changed before hosted gate verification (expected $current_head, got $remote_head). Re-run prepare-init."
return 1
@@ -40,7 +43,60 @@ run_hosted_prepare_gates() {
if [ "$changelog_only" = "true" ]; then
args+=(--changelog-only)
fi
run_quiet_logged "hosted CI/Testbox gates" ".local/gates-hosted-checks.log" node "${args[@]}"
if run_quiet_logged "hosted CI/Testbox gates" ".local/gates-hosted-checks.log" node "${args[@]}"; then
return 0
fi
if rg -F -q "Missing successful recent CI workflow for $current_head. Observed: none" \
.local/gates-hosted-checks.log
then
if [ "$remote_is_cross_repository" = "true" ]; then
cat <<EOF_RECOVERY
Missing hosted CI recovery:
scripts/pr ci-dispatch $pr
unavailable: PR #$pr comes from a fork, and release-gate dispatch requires the exact target SHA on a base-repository branch.
EOF_RECOVERY
return 1
fi
cat <<EOF_RECOVERY
Missing hosted CI recovery:
scripts/pr ci-dispatch $pr
Underlying command:
EOF_RECOVERY
printf ' gh workflow run ci.yml --ref %q -f %q -f release_gate=true -f %q\n' \
"$remote_head_ref" \
"target_ref=$remote_head" \
"pull_request_number=$pr"
fi
return 1
}
ci_dispatch() {
local pr="$1"
local record head_ref head_sha is_cross_repository
record=$(gh pr view "$pr" --json headRefName,headRefOid,isCrossRepository)
head_ref=$(printf '%s\n' "$record" | jq -r .headRefName)
head_sha=$(printf '%s\n' "$record" | jq -r .headRefOid)
is_cross_repository=$(printf '%s\n' "$record" | jq -r .isCrossRepository)
if [ -z "$head_ref" ] || [ "$head_ref" = "null" ] || [ -z "$head_sha" ] || [ "$head_sha" = "null" ]; then
echo "PR #$pr is missing remote headRefName/headRefOid metadata." >&2
return 1
fi
if [ "$is_cross_repository" = "true" ]; then
echo "PR #$pr comes from a fork; release-gate workflow dispatch requires a base-repository branch at $head_sha." >&2
return 1
fi
mark_pr_operation_side_effects_if_available
node "$script_parent_dir/pr-lib/ci-dispatch.mjs" "$pr" "$head_ref" "$head_sha" false
}
mark_pr_operation_side_effects_if_available() {
# scripts/pr sources operation-lock.sh first. Policy tests may source this
# library alone, where advancing a lock phase is neither possible nor needed.
if declare -F mark_pr_operation_side_effects_started >/dev/null; then
mark_pr_operation_side_effects_started
fi
}
pin_worktree_bundled_plugins_dir() {
@@ -313,6 +369,7 @@ prepare_gates() {
enter_worktree "$pr" false
mark_pr_operation_side_effects_if_available
checkout_prep_branch "$pr"
require_artifact .local/pr-meta.env
# shellcheck disable=SC1091
+2
View File
@@ -115,6 +115,7 @@ merge_verify() {
echo "Re-run prepare to refresh prep artifacts and gates: scripts/pr-prepare run $pr"
echo "Note: docs/changelog-only follow-ups reuse prior gate results automatically."
mark_pr_operation_side_effects_started
git fetch origin "pull/$pr/head" >/dev/null 2>&1 || true
if git cat-file -e "${PREP_HEAD_SHA}^{commit}" 2>/dev/null && git cat-file -e "${pr_head_sha}^{commit}" 2>/dev/null; then
echo "HEAD delta (expected...current):"
@@ -125,6 +126,7 @@ merge_verify() {
exit 1
fi
mark_pr_operation_side_effects_started
gh pr checks "$pr" --required --watch --fail-fast >.local/merge-checks-watch.log 2>&1 || true
local checks_json
local checks_err_file
+29
View File
@@ -5,6 +5,9 @@ PR_OPERATION_LOCK_CANDIDATE_PR=""
PR_OPERATION_LOCK_CANDIDATE_OID=""
PR_OPERATION_LOCK_BLOCKED_OID=""
PR_OPERATION_LOCK_BLOCKED_REASON=""
# This is monotonic for one supervised command. Once side effects begin, a
# descendant must not be able to reopen the auto-release validation window.
PR_OPERATION_VALIDATION_PHASE_STATE=unannounced
is_canonical_pr_number() {
local pr="$1"
@@ -121,6 +124,32 @@ clear_pr_operation_lock_state() {
PR_OPERATION_LOCK_CANDIDATE_OID=""
PR_OPERATION_LOCK_BLOCKED_OID=""
PR_OPERATION_LOCK_BLOCKED_REASON=""
PR_OPERATION_VALIDATION_PHASE_STATE=unannounced
}
notify_pr_operation_phase() {
local phase="$1"
if [ -z "${OPENCLAW_PR_LOCK_NOTIFY_FD:-}" ]; then
return 0
fi
case "$OPENCLAW_PR_LOCK_NOTIFY_FD" in ''|*[!0-9]*) return 1 ;; esac
printf 'phase\t%s\n' "$phase" >&"$OPENCLAW_PR_LOCK_NOTIFY_FD"
}
begin_pr_operation_validation_phase() {
if [ "$PR_OPERATION_VALIDATION_PHASE_STATE" != "unannounced" ]; then
return 0
fi
notify_pr_operation_phase validation-started || return 1
PR_OPERATION_VALIDATION_PHASE_STATE=validation
}
mark_pr_operation_side_effects_started() {
if [ "$PR_OPERATION_VALIDATION_PHASE_STATE" != "validation" ]; then
return 0
fi
notify_pr_operation_phase side-effects-started || return 1
PR_OPERATION_VALIDATION_PHASE_STATE=side_effects
}
pr_operation_lock_owner_is_current() {
+4
View File
@@ -49,6 +49,7 @@ verify_prep_branch_matches_prepared_head() {
prepare_init() {
local pr="$1"
mark_pr_operation_side_effects_started
enter_worktree "$pr" true
require_artifact .local/pr-meta.env
@@ -105,6 +106,7 @@ prepare_validate_commit() {
enter_worktree "$pr" false
require_artifact .local/pr-meta.env
mark_pr_operation_side_effects_started
checkout_prep_branch "$pr"
# shellcheck disable=SC1091
@@ -135,6 +137,7 @@ prepare_push() {
require_artifact .local/prep-context.env
require_artifact .local/gates.env
mark_pr_operation_side_effects_started
checkout_prep_branch "$pr"
# shellcheck disable=SC1091
@@ -222,6 +225,7 @@ prepare_sync_head() {
require_artifact .local/pr-meta.env
require_artifact .local/prep-context.env
mark_pr_operation_side_effects_started
checkout_prep_branch "$pr"
# shellcheck disable=SC1091
+28 -3
View File
@@ -53,6 +53,7 @@ let lingeringGroupProcesses = [];
let drainFailure;
let drainFailureGroupStatus;
let drainFailureNotificationOpen = false;
let validationPhaseState = "unannounced";
function delay(ms) {
return new Promise((resolveDelay) => {
@@ -172,6 +173,18 @@ if (killDeadline) {
}
function consumeNotificationLine(line) {
if (line === "phase\tvalidation-started") {
// The FD is inherited by descendants, so phase messages are monotonic:
// no later writer may reopen validation after side effects have started.
if (validationPhaseState === "unannounced") {
validationPhaseState = "validation";
}
return;
}
if (line === "phase\tside-effects-started") {
validationPhaseState = "side-effects";
return;
}
const [lockRef, ownerOid, extra] = line.split("\t");
if (
extra !== undefined ||
@@ -445,17 +458,29 @@ for (const [signal, handler] of signalHandlers) {
}
// PR commands must join all state-mutating children before returning. A clean
// exit is that trusted completion signal; abnormal exits retain the lock because
// an escaped child can outlive both the recorded group and notification pipe.
// exit is the normal completion signal. A nonzero exit may also release while
// the child explicitly remains in its pre-side-effect validation phase; every
// other abnormal exit retains because an escaped child can outlive the group.
const completedCleanly =
childResult.code === 0 &&
!receivedSignal &&
!childResult.signal &&
!notificationFailure &&
!hadLingeringGroup;
const failedDuringValidation =
validationPhaseState === "validation" &&
childResult.code !== null &&
childResult.code > 0 &&
// Shells encode signal termination as 128+signal. Retain conservatively for
// every such status, including signals scripts/pr does not trap itself.
childResult.code < 128 &&
!receivedSignal &&
!childResult.signal &&
!notificationFailure &&
!hadLingeringGroup;
const retainedLocks = [];
const releaseFailures = new Set();
if (drained && completedCleanly) {
if (drained && (completedCleanly || failedDuringValidation)) {
for (const lock of locks.values()) {
try {
releaseLock(lock);
+474
View File
@@ -0,0 +1,474 @@
#!/usr/bin/env node
import { readFileSync } from "node:fs";
import { isDirectRunUrl } from "../lib/direct-run.mjs";
const REVIEW_ARTIFACT_ENUMS = Object.freeze({
recommendation: Object.freeze([
"READY FOR /prepare-pr",
"NEEDS WORK",
"NEEDS DISCUSSION",
"NOT USEFUL (CLOSE)",
]),
findingSeverity: Object.freeze(["BLOCKER", "IMPORTANT", "NIT"]),
nitSweepStatus: Object.freeze(["none", "has_nits"]),
issueValidationSource: Object.freeze(["linked_issue", "pr_body", "both"]),
issueValidationStatus: Object.freeze(["valid", "unclear", "invalid", "already_fixed_on_main"]),
behavioralSweepStatus: Object.freeze(["pass", "needs_work", "not_applicable"]),
behavioralSweepRisk: Object.freeze(["none", "present", "unknown"]),
testsResult: Object.freeze(["pass", "fail", "not_run"]),
docs: Object.freeze(["up_to_date", "missing", "not_applicable"]),
changelog: Object.freeze(["required", "not_required"]),
});
function reviewArtifactEnumHint(enumName, initialValue) {
const allowed = REVIEW_ARTIFACT_ENUMS[enumName];
if (!allowed?.includes(initialValue)) {
throw new Error(`Invalid initial value ${initialValue} for review enum ${enumName}.`);
}
return `${initialValue} (allowed: ${allowed.join("|")})`;
}
function createReviewArtifactTemplate() {
return {
recommendation: reviewArtifactEnumHint("recommendation", "NEEDS WORK"),
findings: [],
nitSweep: {
performed: true,
status: reviewArtifactEnumHint("nitSweepStatus", "none"),
summary: "No optional nits identified.",
},
behavioralSweep: {
performed: true,
status: reviewArtifactEnumHint("behavioralSweepStatus", "not_applicable"),
summary: "No runtime branch-level behavior changes require sweep evidence.",
silentDropRisk: reviewArtifactEnumHint("behavioralSweepRisk", "none"),
branches: [],
},
issueValidation: {
performed: true,
source: reviewArtifactEnumHint("issueValidationSource", "pr_body"),
status: reviewArtifactEnumHint("issueValidationStatus", "unclear"),
summary: "Review not completed yet.",
},
tests: {
ran: [],
gaps: [],
result: reviewArtifactEnumHint("testsResult", "pass"),
},
docs: reviewArtifactEnumHint("docs", "not_applicable"),
changelog: reviewArtifactEnumHint("changelog", "not_required"),
};
}
function isObject(value) {
return value !== null && typeof value === "object" && !Array.isArray(value);
}
function isNonEmptyString(value) {
return typeof value === "string" && value.trim().length > 0;
}
function jsonValue(value) {
return JSON.stringify(value === undefined ? null : value);
}
function validateReviewArtifacts({ review, reviewMarkdown, prMeta }) {
const violations = [];
const add = (message) => {
if (!violations.includes(message)) {
violations.push(message);
}
};
const requireType = (valid, message) => {
if (!valid) {
add(message);
}
return valid;
};
const requireEnum = (value, enumName, messagePrefix) => {
const allowed = REVIEW_ARTIFACT_ENUMS[enumName];
if (!allowed.includes(value)) {
add(`${messagePrefix}: ${jsonValue(value)} (allowed: ${allowed.join("|")})`);
return false;
}
return true;
};
const reviewIsObject = requireType(
isObject(review),
"Invalid .local/review.json: top-level value must be an object",
);
const value = reviewIsObject ? review : {};
const recommendationIsString = requireType(
typeof value.recommendation === "string",
"Invalid recommendation in .local/review.json: recommendation must be a string",
);
const findingsAreArray = requireType(
Array.isArray(value.findings),
"Invalid findings in .local/review.json: findings must be an array",
);
const findings = findingsAreArray ? value.findings : [];
requireType(
findings.every(isObject),
"Invalid finding entry in .local/review.json: each finding must be an object",
);
const nitSweepIsObject = requireType(
isObject(value.nitSweep),
"Invalid nit sweep in .local/review.json: nitSweep must be an object",
);
const issueValidationIsObject = requireType(
isObject(value.issueValidation),
"Invalid issue validation in .local/review.json: issueValidation must be an object",
);
const behavioralSweepIsObject = requireType(
isObject(value.behavioralSweep),
"Invalid behavioral sweep in .local/review.json: behavioralSweep must be an object",
);
const testsIsObject = requireType(
isObject(value.tests),
"Invalid tests in .local/review.json: tests must be an object",
);
for (const section of ["A)", "B)", "C)", "D)", "E)", "F)", "G)", "H)", "I)", "J)"]) {
if (!reviewMarkdown.split("\n").some((line) => line.startsWith(section))) {
add(`Missing section header in .local/review.md: ${section}`);
}
}
if (recommendationIsString) {
requireEnum(
value.recommendation,
"recommendation",
"Invalid recommendation in .local/review.json",
);
}
const invalidSeverity = findings.find(
(finding) =>
isObject(finding) && !REVIEW_ARTIFACT_ENUMS.findingSeverity.includes(finding.severity),
);
if (invalidSeverity) {
add(
`Invalid finding severity in .local/review.json: ${jsonValue(invalidSeverity.severity)} (allowed: ${REVIEW_ARTIFACT_ENUMS.findingSeverity.join("|")})`,
);
}
if (
findings.some(
(finding) =>
!isObject(finding) ||
typeof finding.id !== "string" ||
typeof finding.title !== "string" ||
typeof finding.area !== "string" ||
typeof finding.fix !== "string",
)
) {
add("Invalid finding shape in .local/review.json (id/title/area/fix must be strings)");
}
const nitFindingsCount = findings.filter(
(finding) => isObject(finding) && finding.severity === "NIT",
).length;
const nitSweep = nitSweepIsObject ? value.nitSweep : {};
const nitSweepPerformedIsBoolean = requireType(
typeof nitSweep.performed === "boolean",
"Invalid nit sweep in .local/review.json: nitSweep.performed must be a boolean",
);
if (nitSweepPerformedIsBoolean && nitSweep.performed !== true) {
add("Invalid nit sweep in .local/review.json: nitSweep.performed must be true");
}
const nitSweepStatusIsString = requireType(
typeof nitSweep.status === "string",
"Invalid nit sweep status in .local/review.json: nitSweep.status must be a string",
);
if (nitSweepStatusIsString) {
const validStatus = requireEnum(
nitSweep.status,
"nitSweepStatus",
"Invalid nit sweep status in .local/review.json",
);
if (validStatus && nitSweep.status === "none" && nitFindingsCount > 0) {
add(
"Invalid nit sweep in .local/review.json: nitSweep.status is none but NIT findings exist",
);
}
if (validStatus && nitSweep.status === "has_nits" && nitFindingsCount < 1) {
add(
"Invalid nit sweep in .local/review.json: nitSweep.status is has_nits but no NIT findings exist",
);
}
}
requireType(
typeof nitSweep.summary === "string",
"Invalid nit sweep summary in .local/review.json: nitSweep.summary must be a string",
);
if (typeof nitSweep.summary === "string" && !isNonEmptyString(nitSweep.summary)) {
add(
"Invalid nit sweep summary in .local/review.json: nitSweep.summary must be a non-empty string",
);
}
const issueValidation = issueValidationIsObject ? value.issueValidation : {};
const issuePerformedIsBoolean = requireType(
typeof issueValidation.performed === "boolean",
"Invalid issue validation in .local/review.json: issueValidation.performed must be a boolean",
);
if (issuePerformedIsBoolean && issueValidation.performed !== true) {
add("Invalid issue validation in .local/review.json: issueValidation.performed must be true");
}
const issueSourceIsString = requireType(
typeof issueValidation.source === "string",
"Invalid issue validation source in .local/review.json: issueValidation.source must be a string",
);
if (issueSourceIsString) {
requireEnum(
issueValidation.source,
"issueValidationSource",
"Invalid issue validation source in .local/review.json",
);
}
const issueStatusIsString = requireType(
typeof issueValidation.status === "string",
"Invalid issue validation status in .local/review.json: issueValidation.status must be a string",
);
if (issueStatusIsString) {
requireEnum(
issueValidation.status,
"issueValidationStatus",
"Invalid issue validation status in .local/review.json",
);
}
requireType(
typeof issueValidation.summary === "string",
"Invalid issue validation summary in .local/review.json: issueValidation.summary must be a string",
);
if (typeof issueValidation.summary === "string" && !isNonEmptyString(issueValidation.summary)) {
add(
"Invalid issue validation summary in .local/review.json: issueValidation.summary must be a non-empty string",
);
}
const prMetaIsValid =
isObject(prMeta) &&
Array.isArray(prMeta.files) &&
prMeta.files.every((file) => isObject(file) && typeof file.path === "string");
if (!prMetaIsValid) {
add("Invalid .local/pr-meta.json: files must be an array of objects with string path");
}
const runtimeFileCount = prMetaIsValid
? prMeta.files.filter(
({ path }) =>
/^(src|extensions|apps)\//u.test(path) &&
!/(^|\/)__tests__\/|\.test\.|\.spec\./u.test(path) &&
!/\.(md|mdx)$/u.test(path),
).length
: 0;
const runtimeReviewRequired = runtimeFileCount > 0;
const behavioralSweep = behavioralSweepIsObject ? value.behavioralSweep : {};
const behavioralPerformedIsBoolean = requireType(
typeof behavioralSweep.performed === "boolean",
"Invalid behavioral sweep in .local/review.json: behavioralSweep.performed must be a boolean",
);
if (behavioralPerformedIsBoolean && behavioralSweep.performed !== true) {
add("Invalid behavioral sweep in .local/review.json: behavioralSweep.performed must be true");
}
const behavioralStatusIsString = requireType(
typeof behavioralSweep.status === "string",
"Invalid behavioral sweep status in .local/review.json: behavioralSweep.status must be a string",
);
const behavioralStatusIsValid =
behavioralStatusIsString &&
requireEnum(
behavioralSweep.status,
"behavioralSweepStatus",
"Invalid behavioral sweep status in .local/review.json",
);
const behavioralRiskIsString = requireType(
typeof behavioralSweep.silentDropRisk === "string",
"Invalid behavioral sweep risk in .local/review.json: behavioralSweep.silentDropRisk must be a string",
);
const behavioralRiskIsValid =
behavioralRiskIsString &&
requireEnum(
behavioralSweep.silentDropRisk,
"behavioralSweepRisk",
"Invalid behavioral sweep risk in .local/review.json",
);
requireType(
typeof behavioralSweep.summary === "string",
"Invalid behavioral sweep summary in .local/review.json: behavioralSweep.summary must be a string",
);
if (typeof behavioralSweep.summary === "string" && !isNonEmptyString(behavioralSweep.summary)) {
add(
"Invalid behavioral sweep summary in .local/review.json: behavioralSweep.summary must be a non-empty string",
);
}
const branchesAreArray = Array.isArray(behavioralSweep.branches);
if (!branchesAreArray) {
add(
"Invalid behavioral sweep in .local/review.json: behavioralSweep.branches must be an array",
);
}
const branches = branchesAreArray ? behavioralSweep.branches : [];
if (
branches.some(
(branch) =>
!isObject(branch) ||
typeof branch.path !== "string" ||
typeof branch.decision !== "string" ||
typeof branch.outcome !== "string",
)
) {
add(
"Invalid behavioral sweep branch entry in .local/review.json: each entry must be an object with string path/decision/outcome",
);
}
if (
behavioralStatusIsValid &&
runtimeReviewRequired &&
behavioralSweep.status === "not_applicable"
) {
add(
"Invalid behavioral sweep in .local/review.json: runtime file changes require behavioralSweep.status=pass|needs_work",
);
}
if (runtimeReviewRequired && branches.length < 1) {
add(
"Invalid behavioral sweep in .local/review.json: runtime file changes require at least one branch entry",
);
}
if (
behavioralStatusIsValid &&
behavioralSweep.status === "not_applicable" &&
branches.length > 0
) {
add(
"Invalid behavioral sweep in .local/review.json: not_applicable cannot include branch entries",
);
}
if (
behavioralStatusIsValid &&
behavioralRiskIsValid &&
behavioralSweep.status === "pass" &&
behavioralSweep.silentDropRisk !== "none"
) {
add("Invalid behavioral sweep in .local/review.json: status=pass requires silentDropRisk=none");
}
if (value.recommendation === "READY FOR /prepare-pr" && issueValidation.status !== "valid") {
add(
"Invalid recommendation in .local/review.json: READY FOR /prepare-pr requires issueValidation.status=valid",
);
}
if (value.recommendation === "READY FOR /prepare-pr" && behavioralSweep.status === "needs_work") {
add(
"Invalid recommendation in .local/review.json: READY FOR /prepare-pr requires behavioralSweep.status!=needs_work",
);
}
if (
value.recommendation === "READY FOR /prepare-pr" &&
runtimeReviewRequired &&
behavioralSweep.status !== "pass"
) {
add(
"Invalid recommendation in .local/review.json: READY FOR /prepare-pr on runtime changes requires behavioralSweep.status=pass",
);
}
if (
value.recommendation === "READY FOR /prepare-pr" &&
behavioralSweep.silentDropRisk === "present"
) {
add(
"Invalid recommendation in .local/review.json: READY FOR /prepare-pr is not allowed when behavioralSweep.silentDropRisk=present",
);
}
const tests = testsIsObject ? value.tests : {};
const testsRanAreArray = requireType(
Array.isArray(tests.ran),
"Invalid tests in .local/review.json: tests.ran must be an array of strings",
);
if (testsRanAreArray && !tests.ran.every((entry) => typeof entry === "string")) {
add("Invalid tests in .local/review.json: tests.ran must be an array of strings");
}
const testsGapsAreArray = requireType(
Array.isArray(tests.gaps),
"Invalid tests in .local/review.json: tests.gaps must be an array of strings",
);
if (testsGapsAreArray && !tests.gaps.every((entry) => typeof entry === "string")) {
add("Invalid tests in .local/review.json: tests.gaps must be an array of strings");
}
const testsResultIsString = requireType(
typeof tests.result === "string",
"Invalid tests result in .local/review.json: tests.result must be a string",
);
if (testsResultIsString) {
requireEnum(tests.result, "testsResult", "Invalid tests result in .local/review.json");
}
const docsIsString = requireType(
typeof value.docs === "string",
"Invalid docs status in .local/review.json: docs must be a string",
);
if (docsIsString) {
requireEnum(value.docs, "docs", "Invalid docs status in .local/review.json");
}
const changelogIsString = requireType(
typeof value.changelog === "string",
"Invalid changelog status in .local/review.json: changelog must be a string",
);
if (changelogIsString) {
requireEnum(value.changelog, "changelog", "Invalid changelog status in .local/review.json");
}
return violations;
}
function readJson(filePath) {
try {
return JSON.parse(readFileSync(filePath, "utf8"));
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
throw new Error(`Invalid JSON in ${filePath}: ${message}`, { cause: error });
}
}
function main(argv = process.argv.slice(2)) {
const [command, ...args] = argv;
if (command === "template" && args.length === 0) {
process.stdout.write(`${JSON.stringify(createReviewArtifactTemplate(), null, 2)}\n`);
return;
}
if (command === "validate" && args.length === 3) {
const [reviewPath, reviewMarkdownPath, prMetaPath] = args;
const violations = validateReviewArtifacts({
review: readJson(reviewPath),
reviewMarkdown: readFileSync(reviewMarkdownPath, "utf8"),
prMeta: readJson(prMetaPath),
});
if (violations.length > 0) {
for (const violation of violations) {
console.log(violation);
}
console.log(`${violations.length} artifact violations`);
process.exitCode = 1;
}
return;
}
console.error(
"Usage: review-artifacts.mjs template | validate <review.json> <review.md> <pr-meta.json>",
);
process.exitCode = 2;
}
if (isDirectRunUrl(process.argv[1], import.meta.url)) {
try {
main();
} catch (error) {
console.log(error instanceof Error ? error.message : String(error));
console.log("1 artifact violations");
process.exitCode = 1;
}
}
+23 -313
View File
@@ -7,8 +7,17 @@ set_review_mode() {
> .local/review-mode.env
}
review_artifacts_helper_path() {
local scripts_dir="${script_parent_dir:-}"
if [ -z "$scripts_dir" ]; then
scripts_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
fi
printf '%s/pr-lib/review-artifacts.mjs\n' "$scripts_dir"
}
review_claim() {
local pr="$1"
mark_pr_operation_side_effects_started
local root
root=$(repo_root)
cd "$root"
@@ -64,6 +73,7 @@ review_claim() {
review_checkout_main() {
local pr="$1"
enter_worktree "$pr" false
mark_pr_operation_side_effects_started
git fetch origin main
git checkout --detach origin/main
set_review_mode main
@@ -76,6 +86,7 @@ review_checkout_main() {
review_checkout_pr() {
local pr="$1"
enter_worktree "$pr" false
mark_pr_operation_side_effects_started
git fetch origin "pull/$pr/head:pr-$pr" --force
git checkout --detach "pr-$pr"
set_review_mode pr
@@ -90,6 +101,7 @@ review_guard() {
enter_worktree "$pr" false
require_artifact .local/review-mode.env
require_artifact .local/pr-meta.env
# shellcheck disable=SC1091
source .local/review-mode.env
# shellcheck disable=SC1091
@@ -136,6 +148,8 @@ review_artifacts_init() {
enter_worktree "$pr" false
require_artifact .local/pr-meta.env
mark_pr_operation_side_effects_started
if [ ! -f .local/review.md ]; then
cat > .local/review.md <<'EOF_MD'
A) TL;DR recommendation
@@ -161,52 +175,13 @@ EOF_MD
fi
if [ ! -f .local/review.json ]; then
cat > .local/review.json <<'EOF_JSON'
{
"recommendation": "NEEDS WORK",
"findings": [],
"nitSweep": {
"performed": true,
"status": "none",
"summary": "No optional nits identified."
},
"behavioralSweep": {
"performed": true,
"status": "not_applicable",
"summary": "No runtime branch-level behavior changes require sweep evidence.",
"silentDropRisk": "none",
"branches": []
},
"issueValidation": {
"performed": true,
"source": "pr_body",
"status": "unclear",
"summary": "Review not completed yet."
},
"tests": {
"ran": [],
"gaps": [],
"result": "pass"
},
"docs": "not_applicable",
"changelog": "not_required"
}
EOF_JSON
node "$(review_artifacts_helper_path)" template > .local/review.json
fi
echo "review artifact templates are ready"
echo "files=.local/review.md .local/review.json"
}
review_json_require() {
local expression="$1"
local message="$2"
if ! jq -e "$expression" .local/review.json >/dev/null 2>&1; then
echo "$message"
exit 1
fi
}
review_validate_artifacts() {
local pr="$1"
enter_worktree "$pr" false
@@ -217,281 +192,14 @@ review_validate_artifacts() {
review_guard "$pr"
jq . .local/review.json >/dev/null
review_json_require 'type == "object"' "Invalid .local/review.json: top-level value must be an object"
review_json_require '(.recommendation | type) == "string"' "Invalid recommendation in .local/review.json: recommendation must be a string"
review_json_require '(.findings | type) == "array"' "Invalid findings in .local/review.json: findings must be an array"
review_json_require 'all(.findings[]; type == "object")' "Invalid finding entry in .local/review.json: each finding must be an object"
review_json_require '(.nitSweep | type) == "object"' "Invalid nit sweep in .local/review.json: nitSweep must be an object"
review_json_require '(.issueValidation | type) == "object"' "Invalid issue validation in .local/review.json: issueValidation must be an object"
review_json_require '(.behavioralSweep | type) == "object"' "Invalid behavioral sweep in .local/review.json: behavioralSweep must be an object"
review_json_require '(.tests | type) == "object"' "Invalid tests in .local/review.json: tests must be an object"
local section
for section in "A)" "B)" "C)" "D)" "E)" "F)" "G)" "H)" "I)" "J)"; do
awk -v s="$section" 'index($0, s) == 1 { found=1; exit } END { exit(found ? 0 : 1) }' .local/review.md || {
echo "Missing section header in .local/review.md: $section"
exit 1
}
done
local recommendation
recommendation=$(jq -r '.recommendation // ""' .local/review.json)
case "$recommendation" in
"READY FOR /prepare-pr"|"NEEDS WORK"|"NEEDS DISCUSSION"|"NOT USEFUL (CLOSE)")
;;
*)
printf 'Invalid recommendation in .local/review.json: %s (allowed: READY FOR /prepare-pr|NEEDS WORK|NEEDS DISCUSSION|NOT USEFUL (CLOSE))\n' "$(jq -c '.recommendation' .local/review.json)"
exit 1
;;
esac
local invalid_severity_count
invalid_severity_count=$(jq '[.findings[]? | select((.severity // "") != "BLOCKER" and (.severity // "") != "IMPORTANT" and (.severity // "") != "NIT")] | length' .local/review.json)
if [ "$invalid_severity_count" -gt 0 ]; then
printf 'Invalid finding severity in .local/review.json: %s (allowed: BLOCKER|IMPORTANT|NIT)\n' "$(jq -c 'first(.findings[] | select((.severity // "") != "BLOCKER" and (.severity // "") != "IMPORTANT" and (.severity // "") != "NIT")).severity' .local/review.json)"
exit 1
if ! node "$(review_artifacts_helper_path)" validate \
.local/review.json \
.local/review.md \
.local/pr-meta.json
then
return 1
fi
local invalid_findings_count
invalid_findings_count=$(jq '[.findings[]? | select((.id|type)!="string" or (.title|type)!="string" or (.area|type)!="string" or (.fix|type)!="string")] | length' .local/review.json)
if [ "$invalid_findings_count" -gt 0 ]; then
echo "Invalid finding shape in .local/review.json (id/title/area/fix must be strings)"
exit 1
fi
local nit_findings_count
nit_findings_count=$(jq '[.findings[]? | select((.severity // "") == "NIT")] | length' .local/review.json)
local nit_sweep_performed
review_json_require '(.nitSweep.performed | type) == "boolean"' "Invalid nit sweep in .local/review.json: nitSweep.performed must be a boolean"
nit_sweep_performed=$(jq -r '.nitSweep.performed // empty' .local/review.json)
if [ "$nit_sweep_performed" != "true" ]; then
echo "Invalid nit sweep in .local/review.json: nitSweep.performed must be true"
exit 1
fi
local nit_sweep_status
review_json_require '(.nitSweep.status | type) == "string"' "Invalid nit sweep status in .local/review.json: nitSweep.status must be a string"
nit_sweep_status=$(jq -r '.nitSweep.status // ""' .local/review.json)
case "$nit_sweep_status" in
"none")
if [ "$nit_findings_count" -gt 0 ]; then
echo "Invalid nit sweep in .local/review.json: nitSweep.status is none but NIT findings exist"
exit 1
fi
;;
"has_nits")
if [ "$nit_findings_count" -lt 1 ]; then
echo "Invalid nit sweep in .local/review.json: nitSweep.status is has_nits but no NIT findings exist"
exit 1
fi
;;
*)
printf 'Invalid nit sweep status in .local/review.json: %s (allowed: none|has_nits)\n' "$(jq -c '.nitSweep.status' .local/review.json)"
exit 1
;;
esac
local invalid_nit_summary_count
review_json_require '(.nitSweep.summary | type) == "string"' "Invalid nit sweep summary in .local/review.json: nitSweep.summary must be a string"
invalid_nit_summary_count=$(jq '[.nitSweep.summary | select((type != "string") or (gsub("^\\s+|\\s+$";"") | length == 0))] | length' .local/review.json)
if [ "$invalid_nit_summary_count" -gt 0 ]; then
echo "Invalid nit sweep summary in .local/review.json: nitSweep.summary must be a non-empty string"
exit 1
fi
local issue_validation_performed
review_json_require '(.issueValidation.performed | type) == "boolean"' "Invalid issue validation in .local/review.json: issueValidation.performed must be a boolean"
issue_validation_performed=$(jq -r '.issueValidation.performed // empty' .local/review.json)
if [ "$issue_validation_performed" != "true" ]; then
echo "Invalid issue validation in .local/review.json: issueValidation.performed must be true"
exit 1
fi
local issue_validation_source
review_json_require '(.issueValidation.source | type) == "string"' "Invalid issue validation source in .local/review.json: issueValidation.source must be a string"
issue_validation_source=$(jq -r '.issueValidation.source // ""' .local/review.json)
case "$issue_validation_source" in
"linked_issue"|"pr_body"|"both")
;;
*)
printf 'Invalid issue validation source in .local/review.json: %s (allowed: linked_issue|pr_body|both)\n' "$(jq -c '.issueValidation.source' .local/review.json)"
exit 1
;;
esac
local issue_validation_status
review_json_require '(.issueValidation.status | type) == "string"' "Invalid issue validation status in .local/review.json: issueValidation.status must be a string"
issue_validation_status=$(jq -r '.issueValidation.status // ""' .local/review.json)
case "$issue_validation_status" in
"valid"|"unclear"|"invalid"|"already_fixed_on_main")
;;
*)
printf 'Invalid issue validation status in .local/review.json: %s (allowed: valid|unclear|invalid|already_fixed_on_main)\n' "$(jq -c '.issueValidation.status' .local/review.json)"
exit 1
;;
esac
local invalid_issue_summary_count
review_json_require '(.issueValidation.summary | type) == "string"' "Invalid issue validation summary in .local/review.json: issueValidation.summary must be a string"
invalid_issue_summary_count=$(jq '[.issueValidation.summary | select((type != "string") or (gsub("^\\s+|\\s+$";"") | length == 0))] | length' .local/review.json)
if [ "$invalid_issue_summary_count" -gt 0 ]; then
echo "Invalid issue validation summary in .local/review.json: issueValidation.summary must be a non-empty string"
exit 1
fi
local runtime_file_count
if ! jq -e 'type == "object" and (.files | type) == "array" and all(.files[]; type == "object" and (.path | type) == "string")' .local/pr-meta.json >/dev/null 2>&1; then
echo "Invalid .local/pr-meta.json: files must be an array of objects with string path"
exit 1
fi
runtime_file_count=$(jq '[.files[]? | (.path // "") | select(test("^(src|extensions|apps)/")) | select(test("(^|/)__tests__/|\\.test\\.|\\.spec\\.") | not) | select(test("\\.(md|mdx)$") | not)] | length' .local/pr-meta.json)
local runtime_review_required="false"
if [ "$runtime_file_count" -gt 0 ]; then
runtime_review_required="true"
fi
local behavioral_sweep_performed
review_json_require '(.behavioralSweep.performed | type) == "boolean"' "Invalid behavioral sweep in .local/review.json: behavioralSweep.performed must be a boolean"
behavioral_sweep_performed=$(jq -r '.behavioralSweep.performed // empty' .local/review.json)
if [ "$behavioral_sweep_performed" != "true" ]; then
echo "Invalid behavioral sweep in .local/review.json: behavioralSweep.performed must be true"
exit 1
fi
local behavioral_sweep_status
review_json_require '(.behavioralSweep.status | type) == "string"' "Invalid behavioral sweep status in .local/review.json: behavioralSweep.status must be a string"
behavioral_sweep_status=$(jq -r '.behavioralSweep.status // ""' .local/review.json)
case "$behavioral_sweep_status" in
"pass"|"needs_work"|"not_applicable")
;;
*)
printf 'Invalid behavioral sweep status in .local/review.json: %s (allowed: pass|needs_work|not_applicable)\n' "$(jq -c '.behavioralSweep.status' .local/review.json)"
exit 1
;;
esac
local behavioral_sweep_risk
review_json_require '(.behavioralSweep.silentDropRisk | type) == "string"' "Invalid behavioral sweep risk in .local/review.json: behavioralSweep.silentDropRisk must be a string"
behavioral_sweep_risk=$(jq -r '.behavioralSweep.silentDropRisk // ""' .local/review.json)
case "$behavioral_sweep_risk" in
"none"|"present"|"unknown")
;;
*)
printf 'Invalid behavioral sweep risk in .local/review.json: %s (allowed: none|present|unknown)\n' "$(jq -c '.behavioralSweep.silentDropRisk' .local/review.json)"
exit 1
;;
esac
local invalid_behavioral_summary_count
review_json_require '(.behavioralSweep.summary | type) == "string"' "Invalid behavioral sweep summary in .local/review.json: behavioralSweep.summary must be a string"
invalid_behavioral_summary_count=$(jq '[.behavioralSweep.summary | select((type != "string") or (gsub("^\\s+|\\s+$";"") | length == 0))] | length' .local/review.json)
if [ "$invalid_behavioral_summary_count" -gt 0 ]; then
echo "Invalid behavioral sweep summary in .local/review.json: behavioralSweep.summary must be a non-empty string"
exit 1
fi
local behavioral_branches_is_array
behavioral_branches_is_array=$(jq -r 'if (.behavioralSweep.branches | type) == "array" then "true" else "false" end' .local/review.json)
if [ "$behavioral_branches_is_array" != "true" ]; then
echo "Invalid behavioral sweep in .local/review.json: behavioralSweep.branches must be an array"
exit 1
fi
local invalid_behavioral_branch_count
review_json_require 'all(.behavioralSweep.branches[]; type == "object")' "Invalid behavioral sweep branch entry in .local/review.json: each entry must be an object with string path/decision/outcome"
invalid_behavioral_branch_count=$(jq '[.behavioralSweep.branches[]? | select((.path|type)!="string" or (.decision|type)!="string" or (.outcome|type)!="string")] | length' .local/review.json)
if [ "$invalid_behavioral_branch_count" -gt 0 ]; then
echo "Invalid behavioral sweep branch entry in .local/review.json: each entry must be an object with string path/decision/outcome"
exit 1
fi
local behavioral_branch_count
behavioral_branch_count=$(jq '[.behavioralSweep.branches[]?] | length' .local/review.json)
if [ "$runtime_review_required" = "true" ] && [ "$behavioral_sweep_status" = "not_applicable" ]; then
echo "Invalid behavioral sweep in .local/review.json: runtime file changes require behavioralSweep.status=pass|needs_work"
exit 1
fi
if [ "$runtime_review_required" = "true" ] && [ "$behavioral_branch_count" -lt 1 ]; then
echo "Invalid behavioral sweep in .local/review.json: runtime file changes require at least one branch entry"
exit 1
fi
if [ "$behavioral_sweep_status" = "not_applicable" ] && [ "$behavioral_branch_count" -gt 0 ]; then
echo "Invalid behavioral sweep in .local/review.json: not_applicable cannot include branch entries"
exit 1
fi
if [ "$behavioral_sweep_status" = "pass" ] && [ "$behavioral_sweep_risk" != "none" ]; then
echo "Invalid behavioral sweep in .local/review.json: status=pass requires silentDropRisk=none"
exit 1
fi
if [ "$recommendation" = "READY FOR /prepare-pr" ] && [ "$issue_validation_status" != "valid" ]; then
echo "Invalid recommendation in .local/review.json: READY FOR /prepare-pr requires issueValidation.status=valid"
exit 1
fi
if [ "$recommendation" = "READY FOR /prepare-pr" ] && [ "$behavioral_sweep_status" = "needs_work" ]; then
echo "Invalid recommendation in .local/review.json: READY FOR /prepare-pr requires behavioralSweep.status!=needs_work"
exit 1
fi
if [ "$recommendation" = "READY FOR /prepare-pr" ] && [ "$runtime_review_required" = "true" ] && [ "$behavioral_sweep_status" != "pass" ]; then
echo "Invalid recommendation in .local/review.json: READY FOR /prepare-pr on runtime changes requires behavioralSweep.status=pass"
exit 1
fi
if [ "$recommendation" = "READY FOR /prepare-pr" ] && [ "$behavioral_sweep_risk" = "present" ]; then
echo "Invalid recommendation in .local/review.json: READY FOR /prepare-pr is not allowed when behavioralSweep.silentDropRisk=present"
exit 1
fi
review_json_require '(.tests.ran | type) == "array"' "Invalid tests in .local/review.json: tests.ran must be an array of strings"
review_json_require 'all(.tests.ran[]; type == "string")' "Invalid tests in .local/review.json: tests.ran must be an array of strings"
review_json_require '(.tests.gaps | type) == "array"' "Invalid tests in .local/review.json: tests.gaps must be an array of strings"
review_json_require 'all(.tests.gaps[]; type == "string")' "Invalid tests in .local/review.json: tests.gaps must be an array of strings"
local tests_result
review_json_require '(.tests.result | type) == "string"' "Invalid tests result in .local/review.json: tests.result must be a string"
tests_result=$(jq -r '.tests.result // ""' .local/review.json)
case "$tests_result" in
"pass"|"fail"|"not_run")
;;
*)
printf 'Invalid tests result in .local/review.json: %s (allowed: pass|fail|not_run)\n' "$(jq -c '.tests.result' .local/review.json)"
exit 1
;;
esac
local docs_status
review_json_require '(.docs | type) == "string"' "Invalid docs status in .local/review.json: docs must be a string"
docs_status=$(jq -r '.docs // ""' .local/review.json)
case "$docs_status" in
"up_to_date"|"missing"|"not_applicable")
;;
*)
printf 'Invalid docs status in .local/review.json: %s (allowed: up_to_date|missing|not_applicable)\n' "$(jq -c '.docs' .local/review.json)"
exit 1
;;
esac
local changelog_status
review_json_require '(.changelog | type) == "string"' "Invalid changelog status in .local/review.json: changelog must be a string"
changelog_status=$(jq -r '.changelog // ""' .local/review.json)
case "$changelog_status" in
"required"|"not_required")
;;
*)
printf 'Invalid changelog status in .local/review.json: %s (allowed: required|not_required)\n' "$(jq -c '.changelog' .local/review.json)"
exit 1
;;
esac
echo "review artifacts validated"
print_review_stdout_summary
}
@@ -515,6 +223,7 @@ review_tests() {
fi
done
mark_pr_operation_side_effects_started
bootstrap_deps_if_needed
local run_log=".local/review-tests-run.log"
@@ -546,6 +255,7 @@ review_tests() {
review_init() {
local pr="$1"
mark_pr_operation_side_effects_started
enter_worktree "$pr" true
local json pr_url
+64 -2
View File
@@ -1,5 +1,5 @@
// Committer tests cover committer script behavior.
import { execFileSync } from "node:child_process";
import { execFileSync, spawnSync } from "node:child_process";
import { cpSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
@@ -63,6 +63,10 @@ function commitWithHelperArgs(repo: string, ...args: string[]) {
return run(repo, "bash", [scriptPath, ...args]);
}
function commitWithHelperFailure(repo: string, ...args: string[]) {
return spawnSync("bash", [scriptPath, ...args], { cwd: repo, encoding: "utf8" });
}
function committedPaths(repo: string) {
const output = git(repo, "diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD");
const paths: string[] = [];
@@ -172,6 +176,64 @@ describe("scripts/committer", () => {
expect(committedPaths(repo)).toEqual(["note.txt"]);
});
it("fails before staging when formatting dependencies are missing", () => {
const repo = createRepo();
writeRepoFile(repo, "pnpm-lock.yaml", "lockfileVersion: '9.0'\n");
writeRepoFile(
repo,
"scripts/pre-commit/filter-staged-files.mjs",
"for (const file of process.argv.slice(4)) { if (file.endsWith('.ts')) process.stdout.write(file + '\\0'); }\n",
);
writeRepoFile(repo, "note.ts", "export const note = true;\n");
const result = commitWithHelperFailure(repo, "test: missing formatter", "note.ts");
expect(result.status).toBe(1);
expect(result.stderr).toContain("cannot run oxfmt without node_modules");
expect(result.stderr).toContain("--no-verify-formatted");
expect(git(repo, "diff", "--cached", "--name-only")).toBe("");
expect(git(repo, "log", "-1", "--pretty=%s")).toBe("seed");
});
it("commits dependency-less formatted work only with the explicit assertion", () => {
const repo = createRepo();
writeRepoFile(repo, "pnpm-lock.yaml", "lockfileVersion: '9.0'\n");
writeRepoFile(
repo,
"scripts/pre-commit/filter-staged-files.mjs",
"for (const file of process.argv.slice(4)) { if (file.endsWith('.ts')) process.stdout.write(file + '\\0'); }\n",
);
writeRepoFile(repo, "note.ts", "export const note = true;\n");
const output = commitWithHelperArgs(
repo,
"--no-verify-formatted",
"test: formatted assertion",
"note.ts",
);
expect(output).toContain("asserts separate formatting proof; committing with --no-verify");
expect(committedPaths(repo)).toEqual(["note.ts"]);
});
it("fails before staging when formatter applicability cannot be determined", () => {
const repo = createRepo();
writeRepoFile(repo, "pnpm-lock.yaml", "lockfileVersion: '9.0'\n");
writeRepoFile(
repo,
"scripts/pre-commit/filter-staged-files.mjs",
"process.stderr.write('fixture filter failure\\n'); process.exit(7);\n",
);
writeRepoFile(repo, "note.ts", "export const note = true;\n");
const result = commitWithHelperFailure(repo, "test: failed formatter filter", "note.ts");
expect(result.status).toBe(1);
expect(result.stderr).toContain("Unable to determine formatter applicability");
expect(git(repo, "diff", "--cached", "--name-only")).toBe("");
expect(git(repo, "log", "-1", "--pretty=%s")).toBe("seed");
});
it("bypasses git hooks when using --fast", () => {
const repo = createRepo();
installHook(repo, ".githooks/pre-commit", "#!/usr/bin/env bash\nset -euo pipefail\nexit 91\n");
@@ -211,7 +273,7 @@ describe("scripts/committer", () => {
const output = commitWithHelperArgs(repo, "--help");
expect(output).toContain(
'Usage: committer [--force] [--fast] "commit message" "file" ["file" ...]',
'Usage: committer [--force] [--fast] [--no-verify-formatted] "commit message" "file" ["file" ...]',
);
});
});
+141
View File
@@ -0,0 +1,141 @@
import { spawnSync } from "node:child_process";
import { chmodSync, existsSync, readFileSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
const dispatchScript = join(process.cwd(), "scripts/pr-lib/ci-dispatch.mjs");
const sha = "0123456789abcdef0123456789abcdef01234567";
const changedSha = "fedcba9876543210fedcba9876543210fedcba98";
const describePosix = process.platform === "win32" ? describe.skip : describe;
function createFakeGh() {
const tempDir = tempDirs.make("openclaw-pr-ci-dispatch-");
const fakeGh = join(tempDir, "gh");
const calls = join(tempDir, "calls.log");
const dispatched = join(tempDir, "dispatched");
const seenRunList = join(tempDir, "seen-run-list");
writeFileSync(
fakeGh,
`#!/usr/bin/env bash
set -euo pipefail
printf '%s\\n' "$*" >> "$OPENCLAW_TEST_GH_CALLS"
case "$1 $2" in
"run list")
if [ "\${OPENCLAW_TEST_GH_MODE:-}" = "pending-head-change" ]; then
printf '[]\\n'
elif [ -e "$OPENCLAW_TEST_GH_SEEN_RUN_LIST" ]; then
printf '[{"databaseId":99,"url":"https://github.com/openclaw/openclaw/actions/runs/99","headSha":"%s","createdAt":"2026-01-01T00:00:00Z","status":"queued"}]\\n' "$OPENCLAW_TEST_HEAD_SHA"
else
: > "$OPENCLAW_TEST_GH_SEEN_RUN_LIST"
printf '[]\\n'
fi
;;
"pr view")
if [ -e "$OPENCLAW_TEST_GH_DISPATCHED" ] && [ -n "\${OPENCLAW_TEST_GH_MODE:-}" ]; then
printf '%s\\n' "$OPENCLAW_TEST_CHANGED_HEAD_SHA"
else
printf '%s\\n' "$OPENCLAW_TEST_HEAD_SHA"
fi
;;
"workflow run") : > "$OPENCLAW_TEST_GH_DISPATCHED" ;;
*) echo "unexpected gh invocation: $*" >&2; exit 2 ;;
esac
`,
);
chmodSync(fakeGh, 0o755);
return { calls, dispatched, fakeGh, seenRunList };
}
function runDispatch(
fakeGh: ReturnType<typeof createFakeGh>,
options: {
mode?: "observed-head-change" | "pending-head-change";
immediateTimers?: boolean;
} = {},
) {
let nodeOptions = process.env.NODE_OPTIONS ?? "";
if (options.immediateTimers) {
const preload = join(tempDirs.make("openclaw-pr-ci-dispatch-timers-"), "immediate-timers.cjs");
writeFileSync(preload, "global.setTimeout = (callback) => { callback(); return 0; };\n");
nodeOptions = `${nodeOptions} --require ${preload}`.trim();
}
return spawnSync(
process.execPath,
[dispatchScript, "12345", "contributor/fix-hosted-gates", sha, "false"],
{
encoding: "utf8",
env: {
...process.env,
NODE_OPTIONS: nodeOptions,
OPENCLAW_GH_BIN: fakeGh.fakeGh,
OPENCLAW_TEST_CHANGED_HEAD_SHA: changedSha,
OPENCLAW_TEST_GH_CALLS: fakeGh.calls,
OPENCLAW_TEST_GH_DISPATCHED: fakeGh.dispatched,
OPENCLAW_TEST_GH_MODE: options.mode ?? "",
OPENCLAW_TEST_GH_SEEN_RUN_LIST: fakeGh.seenRunList,
OPENCLAW_TEST_HEAD_SHA: sha,
},
},
);
}
describePosix("scripts/pr ci-dispatch", () => {
it("dispatches the exact CI workflow for the remote PR head", () => {
const fakeGh = createFakeGh();
const result = runDispatch(fakeGh);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
expect(result.stdout).toContain(
"observed_run_url=https://github.com/openclaw/openclaw/actions/runs/99",
);
expect(readFileSync(fakeGh.calls, "utf8")).toContain(
`workflow run ci.yml --ref contributor/fix-hosted-gates -f target_ref=${sha} -f release_gate=true -f pull_request_number=12345`,
);
});
it("refuses a fork-local branch name before invoking GitHub", () => {
const fakeGh = createFakeGh();
const result = spawnSync(
process.execPath,
[dispatchScript, "12345", "fix-hosted-gates", sha, "true"],
{
encoding: "utf8",
env: {
...process.env,
OPENCLAW_GH_BIN: fakeGh.fakeGh,
OPENCLAW_TEST_GH_CALLS: fakeGh.calls,
OPENCLAW_TEST_GH_DISPATCHED: fakeGh.dispatched,
OPENCLAW_TEST_GH_SEEN_RUN_LIST: fakeGh.seenRunList,
OPENCLAW_TEST_HEAD_SHA: sha,
},
},
);
expect(result.status).not.toBe(0);
expect(`${result.stdout}\n${result.stderr}`).toMatch(/comes from a fork/u);
expect(existsSync(fakeGh.calls)).toBe(false);
});
it("fails closed if the remote head changes while CI run indexing is pending", () => {
const result = runDispatch(createFakeGh(), {
immediateTimers: true,
mode: "pending-head-change",
});
expect(result.status).not.toBe(0);
expect(`${result.stdout}\n${result.stderr}`).toMatch(
/head changed while CI dispatch was being indexed/u,
);
});
it("rechecks the remote head before returning an observed exact-SHA run", () => {
const result = runDispatch(createFakeGh(), { mode: "observed-head-change" });
expect(result.status).not.toBe(0);
expect(`${result.stdout}\n${result.stderr}`).toMatch(
/head changed before an exact-SHA CI run became visible/u,
);
});
});
+100
View File
@@ -129,8 +129,10 @@ function installPrCliFixture(repoDir: string) {
"scripts/pr-lib/common.sh",
"scripts/pr-lib/changelog.sh",
"scripts/pr-lib/gates.sh",
"scripts/pr-lib/ci-dispatch.mjs",
"scripts/pr-lib/push.sh",
"scripts/pr-lib/review.sh",
"scripts/pr-lib/review-artifacts.mjs",
"scripts/pr-lib/prepare-core.sh",
"scripts/pr-lib/merge.sh",
];
@@ -1009,6 +1011,41 @@ describePosix("scripts/pr per-PR operation lock", () => {
expect(result.stderr).not.toContain("Retaining the operation lock");
});
it("releases a failed lock while the child is still in validation phase", async () => {
const repoDir = createRepo();
const fixture = writeOperationFixture(repoDir, "failed-validation.sh", [
"acquire_pr_operation_lock 42",
"begin_pr_operation_validation_phase",
"exit 3",
]);
const result = await runSupervisedFixture(repoDir, fixture);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(3);
expect(refExists(repoDir)).toBe(false);
expect(result.stderr).not.toContain("Retaining the operation lock");
});
it("retains a failed lock after the child leaves validation phase", async () => {
const repoDir = createRepo();
const fixture = writeOperationFixture(repoDir, "failed-after-side-effects.sh", [
"acquire_pr_operation_lock 42",
"begin_pr_operation_validation_phase",
"mark_pr_operation_side_effects_started",
"exit 3",
]);
const result = await runSupervisedFixture(repoDir, fixture);
const ownerOid = refOid(repoDir);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(3);
expect(result.stderr).toContain("reason: child exited with code 3");
expect(refOid(repoDir)).toBe(ownerOid);
const recovered = runLockShell(repoDir, [
`recover_pr_operation_lock 42 '${ownerOid}' --confirmed-no-running-tools`,
]);
expect(recovered.status, `${recovered.stdout}\n${recovered.stderr}`).toBe(0);
});
it("reports the child exit code when retaining a failed operation", async () => {
const repoDir = createRepo();
const fixture = writeOperationFixture(repoDir, "failed-operation.sh", [
@@ -1029,6 +1066,69 @@ describePosix("scripts/pr per-PR operation lock", () => {
expect(refExists(repoDir)).toBe(false);
});
it("does not re-enter validation after side effects have started", async () => {
const repoDir = createRepo();
const fixture = writeOperationFixture(repoDir, "failed-after-forged-validation.sh", [
"acquire_pr_operation_lock 42",
"begin_pr_operation_validation_phase",
"mark_pr_operation_side_effects_started",
"notify_pr_operation_phase validation-started",
"exit 3",
]);
const result = await runSupervisedFixture(repoDir, fixture);
const ownerOid = refOid(repoDir);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(3);
expect(result.stderr).toContain("reason: child exited with code 3");
expect(refOid(repoDir)).toBe(ownerOid);
const recovered = runLockShell(repoDir, [
`recover_pr_operation_lock 42 '${ownerOid}' --confirmed-no-running-tools`,
]);
expect(recovered.status, `${recovered.stdout}\n${recovered.stderr}`).toBe(0);
});
it("retains a validation-phase lock when the child exits through a trapped signal", async () => {
const repoDir = createRepo();
const fixture = writeOperationFixture(repoDir, "signaled-validation.sh", [
"trap 'exit 143' TERM",
"acquire_pr_operation_lock 42",
"begin_pr_operation_validation_phase",
"kill -TERM $$",
]);
const result = await runSupervisedFixture(repoDir, fixture);
const ownerOid = refOid(repoDir);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(143);
expect(result.stderr).toContain("reason: child exited with code 143");
expect(refOid(repoDir)).toBe(ownerOid);
const recovered = runLockShell(repoDir, [
`recover_pr_operation_lock 42 '${ownerOid}' --confirmed-no-running-tools`,
]);
expect(recovered.status, `${recovered.stdout}\n${recovered.stderr}`).toBe(0);
});
it("retains a validation-phase lock for untrapped signal exit statuses", async () => {
const repoDir = createRepo();
const fixture = writeOperationFixture(repoDir, "killed-validation.sh", [
"acquire_pr_operation_lock 42",
"begin_pr_operation_validation_phase",
"exit 137",
]);
const result = await runSupervisedFixture(repoDir, fixture);
const ownerOid = refOid(repoDir);
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(137);
expect(result.stderr).toContain("reason: child exited with code 137");
expect(refOid(repoDir)).toBe(ownerOid);
const recovered = runLockShell(repoDir, [
`recover_pr_operation_lock 42 '${ownerOid}' --confirmed-no-running-tools`,
]);
expect(recovered.status, `${recovered.stdout}\n${recovered.stderr}`).toBe(0);
});
it("releases the lock after the operation deletes its runner worktree", async () => {
const repoDir = createRepo();
const doomedDir = tempDirs.make("openclaw-pr-self-deleting-runner-");
+39 -1
View File
@@ -63,6 +63,7 @@ function runGatesBash(
`script_parent_dir='${repoRoot}/scripts'`,
`source '${repoRoot}/scripts/pr-lib/common.sh'`,
`source '${repoRoot}/scripts/pr-lib/gates.sh'`,
"mark_pr_operation_side_effects_started() { :; }",
...(options.sourcePush ? [`source '${repoRoot}/scripts/pr-lib/push.sh'`] : []),
...(options.sourcePrepareCore
? [`source '${repoRoot}/scripts/pr-lib/prepare-core.sh'`]
@@ -696,7 +697,7 @@ describe("prepare gate stamp transitions", () => {
}).stdout.trim();
const result = runGatesBash(
[
`gh() { if [ "$1" = pr ]; then printf '${currentHead}\\n'; else printf 'openclaw/openclaw\\n'; fi; }`,
`gh() { if [ "$1" = pr ]; then printf '{"headRefName":"topic","headRefOid":"${currentHead}","isCrossRepository":false}\\n'; else printf 'openclaw/openclaw\\n'; fi; }`,
"run_quiet_logged() { printf 'ARG:%s\\n' \"$@\"; }",
`run_hosted_prepare_gates 100606 ${currentHead} false`,
].join("\n"),
@@ -711,6 +712,43 @@ describe("prepare gate stamp transitions", () => {
}
});
it("prints the exact recovery command when hosted CI is missing", () => {
const { repoDir, headSha } = makeRetryRepo();
const result = runGatesBash(
[
`gh() { if [ "$1" = pr ]; then printf '{"headRefName":"topic","headRefOid":"${headSha}","isCrossRepository":false}\\n'; else printf 'openclaw/openclaw\\n'; fi; }`,
'rg() { command grep -F -q "$3" "$4"; }',
`run_quiet_logged() { printf 'Missing successful recent CI workflow for ${headSha}. Observed: none\\n' > "$2"; return 1; }`,
`run_hosted_prepare_gates 100606 ${headSha} false`,
].join("\n"),
{ cwd: repoDir },
);
expect(result.status).toBe(1);
expect(result.stdout).toContain("scripts/pr ci-dispatch 100606");
expect(result.stdout).toContain(
`gh workflow run ci.yml --ref topic -f target_ref=${headSha} -f release_gate=true -f pull_request_number=100606`,
);
});
it("does not advertise an unusable dispatch command for fork PRs", () => {
const { repoDir, headSha } = makeRetryRepo();
const result = runGatesBash(
[
`gh() { if [ "$1" = pr ]; then printf '{"headRefName":"topic","headRefOid":"${headSha}","isCrossRepository":true}\\n'; else printf 'openclaw/openclaw\\n'; fi; }`,
'rg() { command grep -F -q "$3" "$4"; }',
`run_quiet_logged() { printf 'Missing successful recent CI workflow for ${headSha}. Observed: none\\n' > "$2"; return 1; }`,
`run_hosted_prepare_gates 100606 ${headSha} false`,
].join("\n"),
{ cwd: repoDir },
);
expect(result.status).toBe(1);
expect(result.stdout).toContain("scripts/pr ci-dispatch 100606");
expect(result.stdout).toContain("unavailable: PR #100606 comes from a fork");
expect(result.stdout).not.toContain("gh workflow run");
});
it("clears remote stamps when fresh docs-only gates do not reuse prior proof", () => {
const { repoDir } = makeRetryRepo();
spawnSync("git", ["update-ref", "refs/remotes/origin/main", "HEAD"], { cwd: repoDir });
@@ -6,6 +6,7 @@ import { useAutoCleanupTempDirTracker } from "../helpers/temp-dir.js";
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
const reviewScript = join(process.cwd(), "scripts/pr-lib/review.sh");
const reviewArtifactsScript = join(process.cwd(), "scripts/pr-lib/review-artifacts.mjs");
const describePosix = process.platform === "win32" ? describe.skip : describe;
function validReview() {
@@ -106,4 +107,48 @@ describePosix("scripts/pr review artifact validation", () => {
'Invalid behavioral sweep status in .local/review.json: "performed" (allowed: pass|needs_work|not_applicable)',
);
});
it("reports every artifact violation before exiting", () => {
const review = validReview();
review.behavioralSweep.status = "performed";
review.behavioralSweep.branches = "src/example.ts" as unknown as unknown[];
review.docs = "todo";
const result = runValidation(review);
expect(result.status).toBe(1);
expect(result.stdout).toContain(
'Invalid behavioral sweep status in .local/review.json: "performed" (allowed: pass|needs_work|not_applicable)',
);
expect(result.stdout).toContain(
"Invalid behavioral sweep in .local/review.json: behavioralSweep.branches must be an array",
);
expect(result.stdout).toContain(
'Invalid docs status in .local/review.json: "todo" (allowed: up_to_date|missing|not_applicable)',
);
expect(result.stdout).toContain("3 artifact violations");
});
it("derives template enum hints from the validation table", () => {
const result = spawnSync(process.execPath, [reviewArtifactsScript, "template"], {
encoding: "utf8",
});
const template = JSON.parse(result.stdout) as ReturnType<typeof validReview>;
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
expect(template.recommendation).toBe(
"NEEDS WORK (allowed: READY FOR /prepare-pr|NEEDS WORK|NEEDS DISCUSSION|NOT USEFUL (CLOSE))",
);
expect(template.nitSweep.status).toBe("none (allowed: none|has_nits)");
expect(template.behavioralSweep.status).toBe(
"not_applicable (allowed: pass|needs_work|not_applicable)",
);
expect(template.behavioralSweep.silentDropRisk).toBe("none (allowed: none|present|unknown)");
expect(template.issueValidation.source).toBe("pr_body (allowed: linked_issue|pr_body|both)");
expect(template.issueValidation.status).toBe(
"unclear (allowed: valid|unclear|invalid|already_fixed_on_main)",
);
expect(template.tests.result).toBe("pass (allowed: pass|fail|not_run)");
expect(template.docs).toBe("not_applicable (allowed: up_to_date|missing|not_applicable)");
expect(template.changelog).toBe("not_required (allowed: required|not_required)");
});
});
+2
View File
@@ -20,9 +20,11 @@ describe("scripts/pr wrappers", () => {
expect(script).toContain("gh_plain");
expect(script).toContain("scripts/pr review-init <PR>");
expect(script).toContain("scripts/pr prepare-run <PR>");
expect(script).toContain("scripts/pr ci-dispatch <PR>");
expect(script).toContain("scripts/pr merge-run <PR>");
expect(script).toContain('review_init "$pr"');
expect(script).toContain('prepare_run "$pr"');
expect(script).toContain('ci_dispatch "$pr"');
expect(script).toContain('merge_run "$pr"');
expect(script).toContain('require_main_target_pr "${1-}"');
expect(script).toContain("only support PRs targeting main");