diff --git a/docs/scratchpads/812-pr-review-comment.md b/docs/scratchpads/812-pr-review-comment.md index 28ebec14..c98e9bc3 100644 --- a/docs/scratchpads/812-pr-review-comment.md +++ b/docs/scratchpads/812-pr-review-comment.md @@ -42,10 +42,15 @@ Make the Gitea `comment` action in `packages/mosaic/framework/tools/git/pr-revie ## Risks / blockers - No active implementation blocker. #789 reached terminal merged state and the coordination hold was lifted. +- Review round 1 found one portability blocker: the API base reconstructed `https://$host` and discarded configured schemes/path prefixes. +- Remediation adds host-matched configured Gitea URL resolution from the same credential source as token resolution; both POST and GET preserve the configured scheme and normalized path prefix. - REST transport failures, non-201 writes, malformed/missing created IDs, non-200 read-backs, and read-back mismatches all fail closed. - Existing approve/request-changes behavior remains covered. -- Independent exact-head review remains coordinator-owned. +- Independent exact-head re-review remains coordinator-owned. ## Final verification evidence -Acceptance criteria re-verified locally after clean rebase. Branch will be force-pushed with lease for coordinator verification; no PR opened. +- URL-portability regression was RED before remediation at the new `http://git.mosaicstack.dev` case and GREEN afterward. +- Regression coverage verifies POST and read-back GET for both `http://git.mosaicstack.dev` and `https://git.mosaicstack.dev/gitea/` configuration. +- Focused shell checks, all git-wrapper harnesses, and full repository test/typecheck/lint/format gates passed after remediation. +- Branch will be force-pushed with lease for coordinator re-verification; no PR opened. diff --git a/packages/mosaic/framework/tools/git/detect-platform.sh b/packages/mosaic/framework/tools/git/detect-platform.sh index 7519f11a..23f05e7a 100755 --- a/packages/mosaic/framework/tools/git/detect-platform.sh +++ b/packages/mosaic/framework/tools/git/detect-platform.sh @@ -81,7 +81,14 @@ get_repo_slug() { gitea_url_matches_host() { local url="${1:-}" host="${2:-}" [[ -n "$url" && -n "$host" ]] || return 1 - [[ "${url%/}" == "https://$host" || "${url%/}" == "http://$host" || "${url%/}" == *"//$host" ]] + python3 - "$url" "$host" <<'PY' +import sys +from urllib.parse import urlparse + +url, host = sys.argv[1:] +parsed = urlparse(url) +raise SystemExit(0 if parsed.scheme in {"http", "https"} and parsed.hostname == host else 1) +PY } get_gitea_service_for_host() { @@ -377,6 +384,51 @@ get_remote_host() { return 1 } +# Resolve the configured Gitea base URL for a host from the same credential +# source used by get_gitea_token. The scheme and any deployment path prefix are +# provider configuration and must not be reconstructed from the git remote. +get_gitea_url_for_host() { + local host="$1" script_dir cred_loader url + script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + cred_loader="$script_dir/../_lib/credentials.sh" + + if [[ -f "$cred_loader" ]]; then + url=$( + # shellcheck source=/dev/null + source "$cred_loader" + unset GITEA_TOKEN GITEA_URL + case "$host" in + git.mosaicstack.dev) load_credentials gitea-mosaicstack 2>/dev/null ;; + git.uscllc.com) load_credentials gitea-usc 2>/dev/null ;; + *) + for svc in gitea-mosaicstack gitea-usc; do + unset GITEA_TOKEN GITEA_URL + load_credentials "$svc" 2>/dev/null || continue + if gitea_url_matches_host "${GITEA_URL:-}" "$host"; then + break + fi + unset GITEA_TOKEN GITEA_URL + done + ;; + esac + if gitea_url_matches_host "${GITEA_URL:-}" "$host"; then + printf '%s' "${GITEA_URL%/}" + fi + ) + if [[ -n "$url" ]]; then + printf '%s\n' "$url" + return 0 + fi + fi + + if gitea_url_matches_host "${GITEA_URL:-}" "$host"; then + printf '%s\n' "${GITEA_URL%/}" + return 0 + fi + + return 1 +} + # Resolve a Gitea API token for the given host. # Priority: Mosaic credential loader → GITEA_TOKEN env → ~/.git-credentials get_gitea_token() { @@ -403,7 +455,7 @@ get_gitea_token() { for svc in gitea-mosaicstack gitea-usc; do unset GITEA_TOKEN GITEA_URL load_credentials "$svc" 2>/dev/null || continue - if [[ "${GITEA_URL:-}" == "https://$host" || "${GITEA_URL:-}" == "http://$host" || "${GITEA_URL:-}" == *"//$host" ]]; then + if gitea_url_matches_host "${GITEA_URL:-}" "$host"; then matched=true break fi @@ -423,7 +475,7 @@ get_gitea_token() { # 2. GITEA_TOKEN env var (only when GITEA_URL, if present, matches the remote host) if [[ -n "${GITEA_TOKEN:-}" ]]; then - if [[ -z "${GITEA_URL:-}" || "${GITEA_URL:-}" == "https://$host" || "${GITEA_URL:-}" == "http://$host" || "${GITEA_URL:-}" == *"//$host" ]]; then + if [[ -z "${GITEA_URL:-}" ]] || gitea_url_matches_host "$GITEA_URL" "$host"; then echo "$GITEA_TOKEN" return 0 fi diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index 20a37fbd..2e0f138a 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -115,7 +115,11 @@ elif [[ "$PLATFORM" == "gitea" ]]; then echo "Error: Gitea token not found for comment persistence" >&2 exit 1 } - api_base="https://$host/api/v1/repos/$repo" + configured_url=$(get_gitea_url_for_host "$host") || { + echo "Error: Configured Gitea URL not found for comment persistence" >&2 + exit 1 + } + api_base="${configured_url%/}/api/v1/repos/$repo" payload=$(COMMENT_BODY="$COMMENT" python3 -c ' import json import os diff --git a/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh b/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh index 8887a065..6515a756 100644 --- a/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh +++ b/packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh @@ -21,16 +21,24 @@ mkdir -p "$REPO_DIR" "$BIN_DIR" git -C "$REPO_DIR" init -q git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git -cat > "$CREDENTIALS_FILE" <<'JSON' -{ - "gitea": { - "mosaicstack": { - "url": "https://git.mosaicstack.dev", - "token": "test-only-placeholder" - } - } +write_credentials() { + local configured_url="$1" + CONFIGURED_GITEA_URL="$configured_url" python3 - "$CREDENTIALS_FILE" <<'PY' +import json +import os +import sys + +with open(sys.argv[1], "w", encoding="utf-8") as credentials: + json.dump({ + "gitea": { + "mosaicstack": { + "url": os.environ["CONFIGURED_GITEA_URL"], + "token": "test-only-placeholder", + } + } + }, credentials) +PY } -JSON cat > "$BIN_DIR/tea" <<'SH' #!/usr/bin/env bash @@ -50,7 +58,7 @@ case "${PR_REVIEW_TEST_MODE:-}" in request-changes) [[ "$*" == "pr reject 123 --repo mosaicstack/stack --login mosaicstack --comment changes-required" ]] || exit 91 ;; - legacy-fallback|comment-success|write-transport-failure|write-http-failure|readback-failure) + legacy-fallback|comment-success|http-success|prefix-success|write-transport-failure|write-http-failure|readback-failure) if [[ "$*" == pr\ comment* ]]; then # tea v0.11.1 treats the nonexistent subcommand as `tea pr list` and exits 0. printf '%s\n' 'INDEX TITLE STATE' @@ -121,8 +129,8 @@ case "${PR_REVIEW_TEST_MODE:-}" in write-http-failure) write_response 500 '{"message":"simulated rejection"}' ;; - comment-success|readback-failure) - if [[ "$method" == "POST" && "$url" == "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments" ]]; then + comment-success|http-success|prefix-success|readback-failure) + if [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' import json import os @@ -137,7 +145,7 @@ print(json.dumps({"id": 456, "body": os.environ["PR_REVIEW_EXPECTED_BODY"]})) PY ) write_response 201 "$response" - elif [[ "$method" == "GET" && "$url" == "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456" ]]; then + elif [[ "$method" == "GET" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/comments/456" ]]; then if [[ "$PR_REVIEW_TEST_MODE" == "readback-failure" ]]; then body="different-body" else @@ -150,7 +158,7 @@ import os print(json.dumps({ "id": 456, "body": os.environ["PR_REVIEW_BODY"], - "issue_url": "https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123", + "issue_url": os.environ["PR_REVIEW_EXPECTED_API_BASE"] + "/issues/123", })) PY ) @@ -169,6 +177,9 @@ chmod +x "$BIN_DIR/curl" run_review() { local mode="$1" action="$2" comment="${3:-}" + local configured_url="${4:-https://git.mosaicstack.dev}" + local expected_api_base="${configured_url%/}/api/v1/repos/mosaicstack/stack" + write_credentials "$configured_url" : > "$TEA_LOG" : > "$CURL_LOG" : > "$OUTPUT_FILE" @@ -180,6 +191,7 @@ run_review() { PR_REVIEW_CURL_LOG="$CURL_LOG" \ PR_REVIEW_TEST_MODE="$mode" \ PR_REVIEW_EXPECTED_BODY="$comment" \ + PR_REVIEW_EXPECTED_API_BASE="$expected_api_base" \ "$SCRIPT_DIR/pr-review.sh" -n 123 -a "$action" ${comment:+-c "$comment"} ) > "$OUTPUT_FILE" 2>&1 } @@ -217,6 +229,14 @@ if [[ -s "$TEA_LOG" ]]; then exit 1 fi +run_review http-success comment durable-body http://git.mosaicstack.dev +grep -q '^POST http://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG" +grep -q '^GET http://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG" + +run_review prefix-success comment durable-body https://git.mosaicstack.dev/gitea/ +grep -q '^POST https://git.mosaicstack.dev/gitea/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG" +grep -q '^GET https://git.mosaicstack.dev/gitea/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG" + if run_review write-transport-failure comment durable-body; then echo "Expected provider transport failure to return nonzero" >&2 exit 1