fix(git): pr-review.sh — surface the provider's stated reason, drop the hardcoded #865 attribution (#1006)

Co-authored-by: mos-dt-0 <[email protected]>

packages/mosaic/framework/tools/git/pr-review.sh

@@ -109,6 +109,55 @@ else
     detect_platform >/dev/null
 fi
 
+# Render the provider's own explanation for a failed request, for appending to
+# an error message (#1004). Every HTTP arm in this file already has the response
+# body on disk; without this it was discarded unread at exactly the moment the
+# caller needed it, which pushes an operator toward re-issuing the request by
+# hand to find out what the server said. Gitea returns {"message": "..."} on a
+# refusal; anything unparseable falls back to a truncated raw first line so a
+# proxy's HTML error page still says something. Prints "" when there is nothing
+# to add, so callers can interpolate unconditionally.
+#
+# Args: $1 = path to the response body file.
+gitea_error_detail() {
+    local body_file="$1"
+    [[ -s "$body_file" ]] || return 0
+    python3 - "$body_file" <<'PY' 2>/dev/null || true
+import json
+import sys
+
+LIMIT = 300
+try:
+    with open(sys.argv[1], encoding="utf-8", errors="replace") as response:
+        raw = response.read().strip()
+except OSError:
+    raise SystemExit(0)
+if not raw:
+    raise SystemExit(0)
+detail = ""
+try:
+    parsed = json.loads(raw)
+    if isinstance(parsed, dict):
+        for key in ("message", "error", "errors"):
+            value = parsed.get(key)
+            if isinstance(value, str) and value.strip():
+                detail = value.strip()
+                break
+            if isinstance(value, list) and value:
+                detail = "; ".join(str(item) for item in value).strip()
+                break
+except ValueError:
+    pass
+if not detail:
+    detail = raw.splitlines()[0].strip()
+if not detail:
+    raise SystemExit(0)
+if len(detail) > LIMIT:
+    detail = detail[:LIMIT] + "..."
+print(f" — provider said: {detail}")
+PY
+}
+
 # Post a comment to a Gitea PR (PR comments ARE issue comments) via the
 # supported REST API and verify it against a PROVIDER-RETURNED created id. The
 # write is a direct POST that returns the created comment object, so we learn
@@ -150,7 +199,7 @@ print(json.dumps({"body": os.environ["COMMENT_BODY"]}))
         return 1
     fi
     if [[ "$write_status" != "201" ]]; then
-        echo "Error: Gitea comment write failed with HTTP $write_status" >&2
+        echo "Error: Gitea comment write failed with HTTP $write_status$(gitea_error_detail "$write_file")" >&2
         return 1
     fi
 
@@ -179,7 +228,7 @@ PY
         return 1
     fi
     if [[ "$readback_status" != "200" ]]; then
-        echo "Error: Gitea comment read-back failed with HTTP $readback_status" >&2
+        echo "Error: Gitea comment read-back failed with HTTP $readback_status$(gitea_error_detail "$readback_file")" >&2
         return 1
     fi
 
@@ -370,7 +419,7 @@ gitea_authenticated_login() {
         return 1
     fi
     if [[ "$status" != "200" ]]; then
-        echo "Error: Gitea authenticated-identity read failed with HTTP $status" >&2
+        echo "Error: Gitea authenticated-identity read failed with HTTP $status$(gitea_error_detail "$response_file")" >&2
         return 1
     fi
 
@@ -407,7 +456,7 @@ gitea_read_pr_head_into() {
         return 1
     fi
     if [[ "$status" != "200" ]]; then
-        echo "Error: Gitea PR head read failed with HTTP $status" >&2
+        echo "Error: Gitea PR head read failed with HTTP $status$(gitea_error_detail "$pr_file")" >&2
         return 1
     fi
     python3 - "$pr_file" <<'PY'
@@ -497,7 +546,7 @@ print(json.dumps({
     fi
     # Gitea returns 200 (occasionally 201) with the created review object.
     if [[ "$write_status" != "200" && "$write_status" != "201" ]]; then
-        echo "Error: Gitea review submit failed with HTTP $write_status (#865: no durable review created)" >&2
+        echo "Error: Gitea review submit failed with HTTP $write_status$(gitea_error_detail "$write_file")" >&2
         return 1
     fi
 
@@ -526,7 +575,7 @@ PY
         return 1
     fi
     if [[ "$readback_status" != "200" ]]; then
-        echo "Error: Gitea review read-back failed with HTTP $readback_status" >&2
+        echo "Error: Gitea review read-back failed with HTTP $readback_status$(gitea_error_detail "$readback_file")" >&2
         return 1
     fi
 

packages/mosaic/framework/tools/git/test-pr-review-gitea-comment.sh

@@ -58,6 +58,9 @@ CREDENTIALS_FILE="$WORK_DIR/credentials.json"
 # A dedicated scratch dir the wrapper is pointed at via TMPDIR, so the leak
 # check can assert every POST/GET body + metadata temp file is cleaned up.
 TMP_SCRATCH="$WORK_DIR/scratch"
+# Sandboxed HOME so nothing under the real $HOME (notably the per-slot Gitea token
+# store at ~/.config/mosaic/secrets/gitea-tokens/) is reachable from the wrapper.
+HOME_DIR="$WORK_DIR/home"
 
 cleanup() {
     rm -rf "$WORK_DIR"
@@ -78,9 +81,18 @@ OVERRIDE_TOKEN="override-token-placeholder"
 CROSS_HOST_LOGIN="foreign-host-reviewer"
 CROSS_HOST_TOKEN="cross-host-token-placeholder"
 
-mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$STATE_DIR" "$TMP_SCRATCH"
+mkdir -p "$REPO_DIR" "$BIN_DIR" "$XDG_DIR" "$STATE_DIR" "$TMP_SCRATCH" "$HOME_DIR"
 git -C "$REPO_DIR" init -q
 git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git
+# HERMETICITY: get_gitea_token() step 0 resolves a per-agent identity from
+# `git config --get mosaic.gitIdentity`, which on a provisioned agent seat is set
+# GLOBALLY and therefore leaks into this fresh repo. It then reads a REAL per-slot
+# token from $HOME and returns it WITHOUT ever consulting MOSAIC_CREDENTIALS_FILE,
+# so the fixture credentials below are silently ignored and the suite runs against
+# production credentials. An empty repo-local value shadows the global one and reads
+# back as empty at rc=0, restoring the shared-credential path this suite intends to
+# exercise. Paired with the sandboxed HOME in run_review().
+git -C "$REPO_DIR" config mosaic.gitIdentity ""
 
 # tea config: the override login carries its own token here. The default login
 # name ("mosaicstack") is deliberately absent, so the no-override default path
@@ -266,6 +278,24 @@ submitted = json.loads(os.environ["PR_REVIEW_PAYLOAD"])
 with open(state_path, encoding="utf-8") as handle:
     reviews = json.load(handle)
 
+# review-refused-422 (#1004): the server REFUSES the submit outright with a
+# definite, correct, machine-readable reason in the body — the shape Gitea
+# returns when the acting credential authored the PR. Nothing is created. The
+# wrapper must surface what the server said and must NOT relabel this as the
+# #865 silent-no-op defect class, which is precisely what it is not.
+if mode == "review-refused-422":
+    print("422")
+    print(json.dumps({"message": "Cannot approve your own pull request"}))
+    raise SystemExit(0)
+
+# review-refused-html (#1004): a non-JSON error body, as a fronting proxy or
+# gateway emits. The detail extraction must degrade to the first raw line rather
+# than silently dropping the only explanation available.
+if mode == "review-refused-html":
+    print("502")
+    print("<html><head><title>502 Bad Gateway</title></head>\n<body>nginx</body></html>")
+    raise SystemExit(0)
+
 # no-op-concurrent-review: the wrapper's own submit is SUPPRESSED (200, no
 # created object) even though a concurrent same-identity, same-state review at
 # the same head already exists. Nothing is persisted; no created id to verify.
@@ -517,6 +547,8 @@ run_review() {
         cd "$REPO_DIR"
         PATH="$BIN_DIR:$PATH" \
         TMPDIR="$TMP_SCRATCH" \
+        HOME="$HOME_DIR" \
+        MOSAIC_GIT_IDENTITY="" \
         XDG_CONFIG_HOME="$XDG_DIR" \
         MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
         PR_REVIEW_TEA_LOG="$TEA_LOG" \
@@ -940,4 +972,44 @@ if grep -q 'Approved and verified' "$OUTPUT_FILE"; then
 fi
 assert_no_temp_leak "review-body-null"
 
+# Case 19 (#1004): an outright server REFUSAL must report the provider's own
+# reason and must NOT be relabelled as the #865 silent-no-op defect class. The
+# old arm hardcoded "(#865: no durable review created)" for EVERY non-2xx, so a
+# 422/403/404 — all of them definite, correct refusals the server explained in
+# the discarded body — arrived at the caller wearing the name of the one defect
+# they are not. That misdirection is what makes an operator re-issue the request
+# by hand against the live object to find out what actually happened.
+if run_review review-refused-422 approve; then
+    echo "FAIL: approve reported success when the server refused the submit" >&2
+    cat "$OUTPUT_FILE" >&2
+    exit 1
+fi
+grep -q 'HTTP 422' "$OUTPUT_FILE"
+if ! grep -q 'Cannot approve your own pull request' "$OUTPUT_FILE"; then
+    echo "FAIL: the provider's stated reason was discarded (#1004)" >&2
+    cat "$OUTPUT_FILE" >&2
+    exit 1
+fi
+if grep -q '#865: no durable review created' "$OUTPUT_FILE"; then
+    echo "FAIL: a server refusal was misattributed to the #865 defect class (#1004)" >&2
+    cat "$OUTPUT_FILE" >&2
+    exit 1
+fi
+assert_no_temp_leak "review-refused-422"
+
+# Case 20 (#1004): a non-JSON error body (a fronting proxy's HTML page) must
+# still yield something the caller can act on, rather than a bare status code.
+if run_review review-refused-html approve; then
+    echo "FAIL: approve reported success on a 502" >&2
+    cat "$OUTPUT_FILE" >&2
+    exit 1
+fi
+grep -q 'HTTP 502' "$OUTPUT_FILE"
+if ! grep -q '502 Bad Gateway' "$OUTPUT_FILE"; then
+    echo "FAIL: a non-JSON error body was dropped instead of degrading to its first line (#1004)" >&2
+    cat "$OUTPUT_FILE" >&2
+    exit 1
+fi
+assert_no_temp_leak "review-refused-html"
+
 echo "pr-review.sh REST review + comment create/read-back regression passed"