dac9838b26e35c196c6442ffe90fd2d7157f4bd1
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 mosaic-coder
|
dac9838b26 |
fix(framework): guard dispatch-level host resolution against a missing git origin (pr-review.sh)
ci/woodpecker/pr/ci Pipeline was successful
Independent review of PR #896 found the -r/--repo + -H/--host overrides had a real bug that this addresses: 1. The approve/request-changes/comment dispatch bodies each independently called a BARE `host=$(get_remote_host)` (used only for a best-effort tea-login guess) that ignored -H/--host entirely and, under `set -e`, died SILENTLY (exit 1, zero stdout/stderr) whenever there was no git origin at all — exactly the "reviewer worktree with no usable origin" case -r/-H exist to support. This was WORSE than the pre-5/5c baseline, which at least failed loud at detect_platform. Fixed by preferring -H/--host and otherwise tolerating a missing/failing git-remote lookup for that best-effort guess: `host="${HOST_OVERRIDE:-$(get_remote_host 2>/dev/null || true)}"`. 2. The regression test's "no git origin" fixture was a subdirectory NESTED inside this checkout, so `git remote get-url origin` resolved UPWARD to the checkout's own real origin — the "tolerates a missing origin" assertions never actually exercised a no-origin case. Fixed by creating that fixture via `mktemp -d` under the system /tmp (truly outside any .git ancestry) and additionally bounding it with GIT_CEILING_DIRECTORIES, plus a fixture self-check that fails the harness if `git remote get-url origin` ever resolves from inside it. Extended coverage to approve/request-changes (not just comment), each asserting the run is not silently dead (exit != 0 with zero combined output) in the true no-origin dir. Reproduce-first evidence: with the CORRECTED fixture, re-running the regression test against the previous (round-1) pr-review.sh reproduces the exact reported failure — "died SILENTLY (exit 1, zero output)" — for the `comment` action; the fix in this commit resolves it for comment, approve, and request-changes alike. Re-verified: pnpm run test:framework-shell (both pr-review tests + full chain) green; shellcheck 0 new findings; grep -niE 'jason|woltje|jarvis' 0 hits; verify-sanitized.sh passed. Part of #891 |
||
|
mosaic-coder
|
03e1cdbcd9 |
fix(framework): pr-review.sh -r/--repo + -H/--host overrides, UA header, repo preflight (Patch 5/5c)
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 |