diff --git a/docs/scratchpads/812-pr-review-comment.md b/docs/scratchpads/812-pr-review-comment.md index c98e9bc3..e50e27a3 100644 --- a/docs/scratchpads/812-pr-review-comment.md +++ b/docs/scratchpads/812-pr-review-comment.md @@ -43,7 +43,8 @@ Make the Gitea `comment` action in `packages/mosaic/framework/tools/git/pr-revie - 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. +- Review round 2 found a second subpath portability blocker: clone-derived `get_repo_slug` retained the deployment prefix, duplicating it under `/api/v1/repos/`. +- Round 3 resolves owner/repo relative to the configured Gitea base path for HTTP(S) clones while preserving root-mounted and SSH clone forms. Host matching now compares non-default ports consistently. - 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 re-review remains coordinator-owned. @@ -51,6 +52,7 @@ Make the Gitea `comment` action in `packages/mosaic/framework/tools/git/pr-revie ## Final verification evidence - 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. +- Round-3 genuine subpath regression was RED against round-2 head `1b190201` and GREEN after the fix: `https://git.example/gitea/owner/repo.git` maps to API repository `owner/repo` under configured base `/gitea`. +- Regression coverage verifies POST and read-back GET for root-mounted HTTP(S), path-prefixed HTTP(S), non-default HTTP port, scp-style SSH, and `ssh://` clone forms. - 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 23f05e7a..38a6f230 100755 --- a/packages/mosaic/framework/tools/git/detect-platform.sh +++ b/packages/mosaic/framework/tools/git/detect-platform.sh @@ -85,9 +85,21 @@ gitea_url_matches_host() { 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) +url, remote_host = sys.argv[1:] +configured = urlparse(url) +remote = urlparse(f"//{remote_host}") +if configured.scheme not in {"http", "https"} or configured.hostname != remote.hostname: + raise SystemExit(1) + +configured_port = configured.port +remote_port = remote.port +if remote_port is None: + default_port = 80 if configured.scheme == "http" else 443 + if configured_port not in {None, default_port}: + raise SystemExit(1) +elif configured_port != remote_port: + raise SystemExit(1) +raise SystemExit(0) PY } @@ -354,6 +366,47 @@ get_gitea_api_host_for_repo_override() { get_host_from_url "${GITEA_URL:-}" } +# Resolve owner/repo relative to a configured Gitea base URL. HTTP(S) clone +# URLs can include the deployment prefix (for example /gitea/owner/repo.git), +# but Gitea's /repos API expects only owner/repo. Root-mounted and SSH clone +# forms retain their existing owner/repo behavior. +get_gitea_repo_slug_for_url() { + local configured_url="$1" remote_url + remote_url=$(git remote get-url origin 2>/dev/null) || return 1 + + if [[ "$remote_url" =~ ^https?:// ]]; then + python3 - "$remote_url" "$configured_url" <<'PY' +import sys +from urllib.parse import urlparse + +remote = urlparse(sys.argv[1]) +base = urlparse(sys.argv[2]) +remote_path = remote.path.strip("/") +if remote_path.endswith(".git"): + remote_path = remote_path[:-4] +base_path = base.path.strip("/") +remote_parts = [part for part in remote_path.split("/") if part] +base_parts = [part for part in base_path.split("/") if part] + +if base_parts and remote_parts[:len(base_parts)] == base_parts: + repo_parts = remote_parts[len(base_parts):] +elif len(remote_parts) == 2: + # Preserve a root-shaped clone URL when provider API configuration carries + # a reverse-proxy prefix separately. + repo_parts = remote_parts +else: + raise SystemExit(1) + +if len(repo_parts) != 2: + raise SystemExit(1) +print("/".join(repo_parts)) +PY + return + fi + + get_repo_slug +} + get_gitea_repo_args() { local repo host login repo=$(get_repo_slug) || return 1 @@ -377,6 +430,11 @@ get_remote_host() { echo "${host##*@}" return 0 fi + if [[ "$remote_url" =~ ^ssh://([^/]+)/ ]]; then + local host="${BASH_REMATCH[1]}" + echo "${host##*@}" + return 0 + fi if [[ "$remote_url" =~ ^git@([^:]+): ]]; then echo "${BASH_REMATCH[1]}" return 0 diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index 2e0f138a..9afd1687 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -109,7 +109,6 @@ elif [[ "$PLATFORM" == "gitea" ]]; then exit 1 fi - repo=$(get_repo_slug) host=$(get_remote_host) token=$(get_gitea_token "$host") || { echo "Error: Gitea token not found for comment persistence" >&2 @@ -119,6 +118,10 @@ elif [[ "$PLATFORM" == "gitea" ]]; then echo "Error: Configured Gitea URL not found for comment persistence" >&2 exit 1 } + repo=$(get_gitea_repo_slug_for_url "$configured_url") || { + echo "Error: Could not resolve Gitea owner/repository relative to configured URL" >&2 + exit 1 + } api_base="${configured_url%/}/api/v1/repos/$repo" payload=$(COMMENT_BODY="$COMMENT" python3 -c ' import json 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 6515a756..16641f56 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 @@ -58,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|http-success|prefix-success|write-transport-failure|write-http-failure|readback-failure) + legacy-fallback|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-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' @@ -129,7 +129,7 @@ case "${PR_REVIEW_TEST_MODE:-}" in write-http-failure) write_response 500 '{"message":"simulated rejection"}' ;; - comment-success|http-success|prefix-success|readback-failure) + comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|readback-failure) if [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' import json @@ -178,7 +178,10 @@ 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" + local remote_url="${5:-https://git.mosaicstack.dev/mosaicstack/stack.git}" + local expected_repo="${6:-mosaicstack/stack}" + local expected_api_base="${configured_url%/}/api/v1/repos/$expected_repo" + git -C "$REPO_DIR" remote set-url origin "$remote_url" write_credentials "$configured_url" : > "$TEA_LOG" : > "$CURL_LOG" @@ -237,6 +240,24 @@ 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" +run_review subpath-success comment durable-body https://git.example/gitea https://git.example/gitea/owner/repo.git owner/repo +grep -q '^POST https://git.example/gitea/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" +grep -q '^GET https://git.example/gitea/api/v1/repos/owner/repo/issues/comments/456$' "$CURL_LOG" +if grep -q '/repos/gitea/owner/repo/' "$CURL_LOG"; then + echo "Configured Gitea path prefix leaked into the repository slug" >&2 + exit 1 +fi + +run_review port-success comment durable-body http://git.example:3000 http://git.example:3000/owner/repo.git owner/repo +grep -q '^POST http://git.example:3000/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" +grep -q '^GET http://git.example:3000/api/v1/repos/owner/repo/issues/comments/456$' "$CURL_LOG" + +run_review scp-ssh-success comment durable-body https://git.example git@git.example:owner/repo.git owner/repo +grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" + +run_review url-ssh-success comment durable-body https://git.example ssh://git@git.example/owner/repo.git owner/repo +grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" + if run_review write-transport-failure comment durable-body; then echo "Expected provider transport failure to return nonzero" >&2 exit 1