Some checks failed
ci/woodpecker/pr/ci Pipeline was canceled
Upstreams the pr-review.sh -r/--repo and -H/--host override family from the host-local tooling-patches kit (patch 5 + 5c), adapted to the framework's current REST-native review/comment implementation: - -r/--repo <owner/repo>: explicit slug override, skipping git-remote slug inference, for reviewer worktrees whose origin is nonstandard or missing. Mirrors the established -r convention of the sibling wrappers (pr-view.sh, pr-diff.sh, pr-ci-wait.sh; #867). detect_platform is now tolerant of a missing/foreign git origin when -r is given (assumes gitea), matching the same sibling-wrapper convention. - -H/--host <host>: explicit Gitea host override, skipping remote-host inference, so ambient CWD/remote state cannot cross-wire a review/comment to the wrong instance. - Repo-exists preflight: when -r is used, GET .../repos/<slug> BEFORE any write. A wrong-host cross-wire now surfaces as a clear preflight error instead of an opaque write-404. - `User-Agent: mosaic-pr-review` header added to every Gitea curl call (write, read-back, /user, PR-head read, preflight) — some Cloudflare-fronted Gitea hosts intermittently reject curl's default User-Agent. Reproduce-first evidence (against origin/main baseline, fresh clone): - -r/-H: `./pr-review.sh -n 1 -a comment -c x -r foo/bar` / `-H example.test` both failed with "Unknown option" pre-patch; both parse post-patch. - User-Agent: `grep -c User-Agent pr-review.sh` was 0 pre-patch. - New regression test (test-pr-review-repo-host-override.sh) fails against the baseline file and passes against the patched file (red-first confirmed). Scope note — two sub-changes from the source kit's patch family (5b: an issue_url-empty html_url readback fallback; 5d: REST-native reviews replacing `tea approve/reject`) were investigated and found ALREADY SUPERSEDED on this baseline by a more robust, already-tested implementation: - 5d: the current gitea_submit_review_verified() already POSTs to /pulls/{n}/reviews (no `tea pr approve/reject` call exists in this file — `grep -c 'tea pr' pr-review.sh` is 0), with additional hardening (identity binding, PR-head commit pinning, current-head TOCTOU close-out) beyond what the source patch describes. - 5b: comment verification already keys off `pull_request_url` (not `issue_url`) with full origin+path pinning (see gitea_create_comment_verified / test-pr-review-gitea-comment.sh cases 15b, 17), which already handles the Gitea empty-issue_url-on-PR-comments behavior the source patch targets, via a stricter mechanism than a bare html_url fallback would provide. Porting the source kit's simpler fallback verbatim would have been a regression. Per HALT-on-no-repro discipline, only the two sub-changes that actually reproduced against baseline (5, 5c) are included here. Part of #891
229 lines
8.8 KiB
Bash
Executable File
229 lines
8.8 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Regression harness for pr-review.sh's -r/--repo and -H/--host overrides,
|
|
# the -r repo-exists preflight, and the mosaic-pr-review User-Agent header
|
|
# (patch family 5/5c, part of #891).
|
|
#
|
|
# -r/--repo and -H/--host skip git-remote slug/host inference entirely, for
|
|
# reviewer worktrees whose origin is nonstandard, wrong, or missing (mirrors
|
|
# the -r convention of the sibling wrappers pr-view.sh/pr-diff.sh/pr-ci-wait.sh,
|
|
# #867). When -r is given, the resolved repo is preflighted (GET
|
|
# .../repos/<slug>) BEFORE any write, so a wrong-host cross-wire surfaces as a
|
|
# clear preflight error instead of an opaque write-404. Every Gitea curl call
|
|
# (preflight, write, read-back, /user) must carry a
|
|
# `User-Agent: mosaic-pr-review` header, since some Cloudflare-fronted Gitea
|
|
# hosts intermittently reject curl's default User-Agent.
|
|
|
|
set -euo pipefail
|
|
|
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/pr-review-repo-host-override}"
|
|
BIN_DIR="$WORK_DIR/bin"
|
|
STATE_DIR="$WORK_DIR/state"
|
|
CURL_LOG="$WORK_DIR/curl.log"
|
|
UA_LOG="$WORK_DIR/ua.log"
|
|
OUTPUT_FILE="$WORK_DIR/output.log"
|
|
TMP_SCRATCH="$WORK_DIR/scratch"
|
|
CREDENTIALS_FILE="$WORK_DIR/credentials.json"
|
|
|
|
cleanup() {
|
|
rm -rf "$WORK_DIR"
|
|
}
|
|
trap cleanup EXIT
|
|
|
|
HOST_OVERRIDE_VAL="pr-review-override.test"
|
|
REPO_OVERRIDE_VAL="acme/widgets"
|
|
TOKEN_VAL="override-token-xyz"
|
|
ACTING_LOGIN="override-bot"
|
|
|
|
mkdir -p "$BIN_DIR" "$STATE_DIR" "$TMP_SCRATCH"
|
|
|
|
# tea stub: -r/--repo + -H/--host must never need tea at all (no per-host
|
|
# login list lookup is required to resolve these overrides). Any invocation is
|
|
# a hard failure so a regression that starts shelling out to tea is caught.
|
|
cat > "$BIN_DIR/tea" <<'SH'
|
|
#!/usr/bin/env bash
|
|
echo "FAIL: pr-review.sh invoked tea ($*) while using -r/--repo + -H/--host overrides" >&2
|
|
exit 91
|
|
SH
|
|
chmod +x "$BIN_DIR/tea"
|
|
|
|
# curl stub: models the target Gitea instance for the OVERRIDDEN host/repo
|
|
# only. Any request to a DIFFERENT host/repo (i.e. one derived from git-remote
|
|
# inference instead of the override) is unexpected and fails the run.
|
|
cat > "$BIN_DIR/curl" <<SH
|
|
#!/usr/bin/env bash
|
|
set -euo pipefail
|
|
|
|
output_file=""
|
|
method="GET"
|
|
url=""
|
|
config_file=""
|
|
ua_seen="0"
|
|
while [[ \$# -gt 0 ]]; do
|
|
case "\$1" in
|
|
-o) output_file="\$2"; shift 2 ;;
|
|
-H)
|
|
[[ "\$2" == "User-Agent: mosaic-pr-review" ]] && ua_seen="1"
|
|
shift 2 ;;
|
|
-K|--config) config_file="\$2"; shift 2 ;;
|
|
-w) shift 2 ;;
|
|
-X) shift 2 ;;
|
|
-d|--data) shift 2 ;;
|
|
-s|-S|-sS) shift ;;
|
|
http://*|https://*) url="\$1"; shift ;;
|
|
*) shift ;;
|
|
esac
|
|
done
|
|
|
|
printf '%s %s\n' "\$method" "\$url" >> "$CURL_LOG"
|
|
printf '%s %s\n' "\$ua_seen" "\$url" >> "$UA_LOG"
|
|
|
|
write_response() {
|
|
local status="\$1" body="\$2"
|
|
[[ -n "\$output_file" ]] || exit 96
|
|
printf '%s' "\$body" > "\$output_file"
|
|
printf '%s' "\$status"
|
|
}
|
|
|
|
mode="\$(cat "$STATE_DIR/mode" 2>/dev/null || true)"
|
|
web_base="https://$HOST_OVERRIDE_VAL"
|
|
repo="$REPO_OVERRIDE_VAL"
|
|
|
|
case "\$url" in
|
|
"https://$HOST_OVERRIDE_VAL/api/v1/repos/$REPO_OVERRIDE_VAL")
|
|
if [[ "\$mode" == "repo-missing" ]]; then
|
|
write_response 404 '{"message":"not found"}'
|
|
else
|
|
write_response 200 "{\\"full_name\\":\\"$REPO_OVERRIDE_VAL\\"}"
|
|
fi
|
|
;;
|
|
"https://$HOST_OVERRIDE_VAL/api/v1/user")
|
|
write_response 200 "{\\"login\\":\\"$ACTING_LOGIN\\"}"
|
|
;;
|
|
"https://$HOST_OVERRIDE_VAL/api/v1/repos/$REPO_OVERRIDE_VAL/issues/123/comments")
|
|
write_response 201 "{\\"id\\":456,\\"body\\":\\"durable-body\\",\\"user\\":{\\"login\\":\\"$ACTING_LOGIN\\"},\\"issue_url\\":\\"\\",\\"pull_request_url\\":\\"\${web_base}/\${repo}/pulls/123\\"}"
|
|
;;
|
|
"https://$HOST_OVERRIDE_VAL/api/v1/repos/$REPO_OVERRIDE_VAL/issues/comments/456")
|
|
write_response 200 "{\\"id\\":456,\\"body\\":\\"durable-body\\",\\"user\\":{\\"login\\":\\"$ACTING_LOGIN\\"},\\"issue_url\\":\\"\\",\\"pull_request_url\\":\\"\${web_base}/\${repo}/pulls/123\\"}"
|
|
;;
|
|
*)
|
|
echo "Unexpected curl request (host/repo not from the override — inference leaked through?): \$method \$url" >&2
|
|
exit 97
|
|
;;
|
|
esac
|
|
SH
|
|
chmod +x "$BIN_DIR/curl"
|
|
|
|
run() {
|
|
local repo_dir="$1"; shift
|
|
: > "$CURL_LOG"
|
|
: > "$UA_LOG"
|
|
: > "$OUTPUT_FILE"
|
|
(
|
|
cd "$repo_dir"
|
|
PATH="$BIN_DIR:$PATH" \
|
|
TMPDIR="$TMP_SCRATCH" \
|
|
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
|
|
GITEA_TOKEN="$TOKEN_VAL" \
|
|
GITEA_URL="https://$HOST_OVERRIDE_VAL" \
|
|
"$SCRIPT_DIR/pr-review.sh" "$@"
|
|
) > "$OUTPUT_FILE" 2>&1
|
|
}
|
|
|
|
set_mode() {
|
|
printf '%s' "$1" > "$STATE_DIR/mode"
|
|
}
|
|
|
|
# --- Case 1: -r/--repo and -H/--host are recognized flags (regression lock on
|
|
# the previous "Unknown option: -r" / "Unknown option: -H" failure). Use a
|
|
# bogus ACTION so the run fails cheaply, WITHOUT any network I/O, at the
|
|
# platform-dispatch unknown-action branch rather than at argument parsing —
|
|
# proving both flags were consumed as options, not rejected.
|
|
NO_GIT_DIR="$WORK_DIR/no-git-dir"
|
|
mkdir -p "$NO_GIT_DIR"
|
|
set_mode "repo-ok"
|
|
if run "$NO_GIT_DIR" -n 123 -a bogus-action -r "$REPO_OVERRIDE_VAL" -H "$HOST_OVERRIDE_VAL"; then
|
|
echo "FAIL: bogus action unexpectedly succeeded" >&2
|
|
cat "$OUTPUT_FILE" >&2
|
|
exit 1
|
|
fi
|
|
if grep -q 'Unknown option' "$OUTPUT_FILE"; then
|
|
echo "FAIL: -r/--repo or -H/--host was not recognized as a valid flag" >&2
|
|
cat "$OUTPUT_FILE" >&2
|
|
exit 1
|
|
fi
|
|
grep -q 'Unknown action: bogus-action' "$OUTPUT_FILE"
|
|
|
|
# --- Case 2: -h/--help documents both overrides.
|
|
HELP_TEXT="$("$SCRIPT_DIR/pr-review.sh" -h)"
|
|
echo "$HELP_TEXT" | grep -q -- '-r, --repo'
|
|
echo "$HELP_TEXT" | grep -q -- '-H, --host'
|
|
|
|
# --- Case 3: platform detection is tolerated with NO git repository at all
|
|
# when -r/--repo is given (reviewer worktree with a missing/nonstandard
|
|
# origin) — must NOT fail with "not a git repository or no origin remote".
|
|
set_mode "repo-ok"
|
|
run "$NO_GIT_DIR" -n 123 -a comment -c durable-body -r "$REPO_OVERRIDE_VAL" -H "$HOST_OVERRIDE_VAL"
|
|
if grep -qi 'not a git repository or no origin remote' "$OUTPUT_FILE"; then
|
|
echo "FAIL: -r/--repo did not tolerate a missing git origin" >&2
|
|
cat "$OUTPUT_FILE" >&2
|
|
exit 1
|
|
fi
|
|
grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
|
|
|
|
# --- Case 4: -r/--repo and -H/--host WIN over git-remote inference, not just
|
|
# tolerate its absence — set up a real git repo whose origin points at a
|
|
# COMPLETELY DIFFERENT host/repo and confirm the override, not the remote, is
|
|
# what gets used (the curl stub only answers for the override host/repo; any
|
|
# request derived from the wrong remote is an unexpected request and fails
|
|
# the stub with exit 97).
|
|
WRONG_REMOTE_DIR="$WORK_DIR/wrong-remote-repo"
|
|
mkdir -p "$WRONG_REMOTE_DIR"
|
|
git -C "$WRONG_REMOTE_DIR" init -q
|
|
git -C "$WRONG_REMOTE_DIR" remote add origin https://git.wrong-inferred-host.example/wrong/repo.git
|
|
set_mode "repo-ok"
|
|
run "$WRONG_REMOTE_DIR" -n 123 -a comment -c durable-body -r "$REPO_OVERRIDE_VAL" -H "$HOST_OVERRIDE_VAL"
|
|
grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
|
|
if grep -q 'wrong-inferred-host' "$CURL_LOG"; then
|
|
echo "FAIL: -r/--repo + -H/--host did not override git-remote inference" >&2
|
|
cat "$CURL_LOG" >&2
|
|
exit 1
|
|
fi
|
|
|
|
# --- Case 5 (5c): the repo-exists preflight runs BEFORE any write and fails
|
|
# closed with a clear diagnostic when the resolved repo is unreachable at the
|
|
# resolved host — proving a wrong-host cross-wire surfaces here, not as an
|
|
# opaque write-404.
|
|
set_mode "repo-missing"
|
|
if run "$NO_GIT_DIR" -n 123 -a comment -c durable-body -r "$REPO_OVERRIDE_VAL" -H "$HOST_OVERRIDE_VAL"; then
|
|
echo "FAIL: preflight did not fail closed on a missing repo" >&2
|
|
cat "$OUTPUT_FILE" >&2
|
|
exit 1
|
|
fi
|
|
grep -q "repo '$REPO_OVERRIDE_VAL' not reachable" "$OUTPUT_FILE"
|
|
if grep -q '/issues/123/comments' "$CURL_LOG"; then
|
|
echo "FAIL: wrapper posted a comment despite a failed repo preflight" >&2
|
|
cat "$CURL_LOG" >&2
|
|
exit 1
|
|
fi
|
|
|
|
# --- Case 6 (5): every Gitea curl call (preflight, /user, comment write,
|
|
# comment read-back) carries the User-Agent: mosaic-pr-review header.
|
|
set_mode "repo-ok"
|
|
run "$NO_GIT_DIR" -n 123 -a comment -c durable-body -r "$REPO_OVERRIDE_VAL" -H "$HOST_OVERRIDE_VAL"
|
|
grep -q 'Added and verified comment on Gitea PR #123' "$OUTPUT_FILE"
|
|
call_count="$(wc -l < "$CURL_LOG" | tr -d ' ')"
|
|
ua_count="$(awk '$1 == "1"' "$UA_LOG" | wc -l | tr -d ' ')"
|
|
if [[ "$call_count" -lt 4 ]]; then
|
|
echo "FAIL: expected at least 4 curl calls (preflight, /user, write, read-back), got $call_count" >&2
|
|
cat "$CURL_LOG" >&2
|
|
exit 1
|
|
fi
|
|
if [[ "$ua_count" != "$call_count" ]]; then
|
|
echo "FAIL: not every curl call carried User-Agent: mosaic-pr-review ($ua_count/$call_count)" >&2
|
|
cat "$UA_LOG" >&2
|
|
exit 1
|
|
fi
|
|
|
|
echo "pr-review.sh -r/--repo + -H/--host override + preflight + User-Agent regression passed"
|