From a27f1fa7df723004498ae5d106b57e5710fcb5ae Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Tue, 21 Jul 2026 17:50:43 -0500 Subject: [PATCH] fix(tools): use top-level tea comment invocation and formalize --login passthrough (#865) issue-comment.sh called the non-existent `tea issue comment` subcommand form; tea 0.11.1 silently no-ops and exits 0 instead of erroring, producing a false-success write. Switch to the top-level `tea comment ` form and add fail-closed REST read-back verification so the wrapper no longer trusts tea's exit code alone. pr-review.sh's comment path was already fixed for this bug by #812/#835 (routes through a read-back-verified REST comment API instead of any tea comment subcommand); this change formalizes an explicit --login override flag there too and documents the after-detection last-wins --login ordering, consistent with issue-comment.sh. Co-Authored-By: Claude Opus 4.8 --- packages/mosaic/framework/tools/git/README.md | 13 +++ .../framework/tools/git/issue-comment.sh | 105 +++++++++++++++++- .../mosaic/framework/tools/git/pr-review.sh | 50 ++++++++- 3 files changed, 160 insertions(+), 8 deletions(-) diff --git a/packages/mosaic/framework/tools/git/README.md b/packages/mosaic/framework/tools/git/README.md index c463de14..a148a86e 100644 --- a/packages/mosaic/framework/tools/git/README.md +++ b/packages/mosaic/framework/tools/git/README.md @@ -7,3 +7,16 @@ These scripts provide host-aware GitHub and Gitea issue, pull-request, milestone A successful provider write command—or a wrapper message based only on that command's exit code—is **not** durable review provenance. Review comments count as durable provenance only after the wrapper reads the created provider record back and verifies that it belongs to the intended repository and pull request and contains the exact submitted body (or verifies the provider-returned record ID). `pr-review.sh` therefore fails closed when a Gitea comment cannot be written, its created comment ID cannot be identified, or provider read-back does not match. It reports comment success only after that read-back verification passes. + +`issue-comment.sh` applies the same fail-closed read-back verification to issue comments: after posting via `tea comment`, it independently re-fetches the issue's comments via the Gitea REST API and confirms one matches the submitted body before reporting success. + +## `tea` invocation notes (Gitea) + +- tea v0.11.1 has **no `comment` subcommand under `tea pr` or `tea issue`**. The correct invocation is the **top-level** `tea comment [--repo ...] [--login ...]`. The `tea pr comment` / `tea issue comment` forms don't error — tea silently falls through to a no-op and still exits 0, producing a false-success write (#865). Always use the top-level form. +- `tea pr approve` and `tea pr reject` take an optional review comment/reason as a **trailing positional argument**, not a `--comment`/`-comment` flag (that flag does not exist on those subcommands). `pr-review.sh` avoids this positional form entirely for the approve/reject actions and instead posts any review comment through the same durable, read-back-verified comment API used for the `comment` action (see #835/#812) — the trailing-positional form remains available to callers who invoke `tea` directly, but is not used by these wrappers. + +### `--login` passthrough + +Both `pr-review.sh` and `issue-comment.sh` accept an optional `--login ` flag that overrides the automatically detected Gitea `tea` login for that single invocation. The override is appended to the `tea` command line **after** the detected default (`get_gitea_repo_args()` / `get_gitea_login[_for_host]()`), because tea honors only the **last** `--login` flag on its command line — an override placed before the default would be silently clobbered by it. Callers who need a different login than the host default should pass `--login ` rather than relying on ordering tricks or re-invoking `tea login` globally. + +As a durable successor to this mechanism, consider giving each reviewer/approver slot its own dedicated Gitea login credential, so that author≠reviewer holds at the credential level rather than relying on wrapper-level `--login` bookkeeping. This is a recommendation for future hardening, not something implemented by this flag. diff --git a/packages/mosaic/framework/tools/git/issue-comment.sh b/packages/mosaic/framework/tools/git/issue-comment.sh index 5fef417e..0d0491b9 100755 --- a/packages/mosaic/framework/tools/git/issue-comment.sh +++ b/packages/mosaic/framework/tools/git/issue-comment.sh @@ -1,6 +1,23 @@ #!/bin/bash # issue-comment.sh - Add a comment to an issue on GitHub or Gitea -# Usage: issue-comment.sh -i -c +# Usage: issue-comment.sh -i -c [--login ] +# +# tea v0.11.1 defines no `comment` subcommand under `tea issue` (or `tea pr`); +# the correct invocation is the TOP-LEVEL `tea comment ` form. +# Calling the non-existent `tea issue comment ...` form does not error — tea +# silently falls through to a no-op and still exits 0, so a caller trusting +# the exit code alone believes a comment was posted when it was not (#865). +# Because that failure mode is silent, this script never trusts tea's exit +# code alone: after posting, it independently re-fetches the issue's comments +# via the Gitea REST API (curl — urllib is blocked by Cloudflare on this +# host) and fails closed if the posted body cannot be found. +# +# --login override: the default `--login` is resolved from the local `tea` +# login list for this repo's host (get_gitea_login). Pass --login to +# override that default for this invocation only. The override is appended +# to the tea command line AFTER the detected default, because tea honors +# only the LAST `--login` flag on the command line — a flag placed before +# the default would be silently clobbered by it. set -e @@ -10,6 +27,7 @@ source "$SCRIPT_DIR/detect-platform.sh" # Parse arguments ISSUE_NUMBER="" COMMENT="" +LOGIN_OVERRIDE="" while [[ $# -gt 0 ]]; do case $1 in @@ -21,12 +39,17 @@ while [[ $# -gt 0 ]]; do COMMENT="$2" shift 2 ;; + -l|--login) + LOGIN_OVERRIDE="$2" + shift 2 + ;; -h|--help) - echo "Usage: issue-comment.sh -i -c " + echo "Usage: issue-comment.sh -i -c [--login ]" echo "" echo "Options:" echo " -i, --issue Issue number (required)" echo " -c, --comment Comment text (required)" + echo " -l, --login Override the detected Gitea tea login for this call" echo " -h, --help Show this help" exit 0 ;; @@ -49,6 +72,68 @@ fi detect_platform >/dev/null +# Independently re-fetch the issue's comments via the Gitea REST API and +# confirm one matches the body we just posted (see header comment: tea's +# exit code is not trustworthy evidence of a durable write on its own). +# Prints the matched comment ID to stdout on success. +gitea_verify_comment_posted() { + local issue_number="$1" comment_body="$2" + local host token configured_url repo api_base readback_response_file + + host=$(get_remote_host) + token=$(get_gitea_token "$host") || { + echo "Error: Gitea token not found for comment read-back verification" >&2 + return 1 + } + configured_url=$(get_gitea_url_for_host "$host") || { + echo "Error: Configured Gitea URL not found for comment read-back verification" >&2 + return 1 + } + repo=$(get_gitea_repo_slug_for_url "$configured_url") || { + echo "Error: Could not resolve Gitea owner/repository relative to configured URL" >&2 + return 1 + } + api_base="${configured_url%/}/api/v1/repos/$repo" + + readback_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-readback.XXXXXX") + trap 'rm -f "$readback_response_file"' RETURN + + if ! readback_status=$(curl -sS -o "$readback_response_file" -w '%{http_code}' \ + -H "Authorization: token $token" \ + "$api_base/issues/$issue_number/comments"); then + echo "Error: Gitea comment read-back transport failed" >&2 + return 1 + fi + if [[ "$readback_status" != "200" ]]; then + echo "Error: Gitea comment read-back failed with HTTP $readback_status" >&2 + return 1 + fi + + EXPECTED_COMMENT_BODY="$comment_body" python3 - "$readback_response_file" <<'PY' +import json +import os +import sys + +try: + with open(sys.argv[1], encoding="utf-8") as response: + comments = json.load(response) + if not isinstance(comments, list): + raise ValueError("response is not a comment list") + expected_body = os.environ["EXPECTED_COMMENT_BODY"] + matches = [c for c in comments if isinstance(c, dict) and c.get("body") == expected_body] + if not matches: + raise ValueError("no matching comment found on read-back") + best = max(matches, key=lambda c: c.get("id") or 0) + comment_id = best.get("id") + if not isinstance(comment_id, int) or comment_id <= 0: + raise ValueError("matching comment has no usable id") +except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error: + print(f"Error: Gitea comment persistence verification failed: {error}", file=sys.stderr) + raise SystemExit(1) +print(comment_id) +PY +} + if [[ "$PLATFORM" == "github" ]]; then gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" echo "Added comment to GitHub issue #$ISSUE_NUMBER" @@ -61,8 +146,20 @@ elif [[ "$PLATFORM" == "gitea" ]]; then echo "Error: could not resolve a Gitea login for this repo; cannot comment on issue #$ISSUE_NUMBER." >&2 exit 1 } - tea issue comment "$ISSUE_NUMBER" "$COMMENT" --repo "$REPO_SLUG" --login "$GITEA_LOGIN_NAME" - echo "Added comment to Gitea issue #$ISSUE_NUMBER" + TEA_ARGS=(comment "$ISSUE_NUMBER" "$COMMENT" --repo "$REPO_SLUG" --login "$GITEA_LOGIN_NAME") + # --login override goes LAST: tea honors only the final --login on its + # command line, so an override placed before the detected default above + # would be silently clobbered by it. + if [[ -n "$LOGIN_OVERRIDE" ]]; then + TEA_ARGS+=(--login "$LOGIN_OVERRIDE") + fi + tea "${TEA_ARGS[@]}" + + comment_id=$(gitea_verify_comment_posted "$ISSUE_NUMBER" "$COMMENT") || { + echo "Error: could not verify comment landed on Gitea issue #$ISSUE_NUMBER via read-back; treating tea's exit code as untrustworthy (#865)." >&2 + exit 1 + } + echo "Added and verified comment on Gitea issue #$ISSUE_NUMBER (comment ID $comment_id)" else echo "Error: Unknown platform" exit 1 diff --git a/packages/mosaic/framework/tools/git/pr-review.sh b/packages/mosaic/framework/tools/git/pr-review.sh index 9b95c0ed..1e0e15c0 100755 --- a/packages/mosaic/framework/tools/git/pr-review.sh +++ b/packages/mosaic/framework/tools/git/pr-review.sh @@ -1,6 +1,17 @@ #!/bin/bash # pr-review.sh - Review a pull request on GitHub or Gitea -# Usage: pr-review.sh -n -a [-c ] +# Usage: pr-review.sh -n -a [-c ] [--login ] +# +# --login override: approve/request-changes on Gitea invoke `tea pr +# approve`/`tea pr reject` with a `--login` resolved from the local tea +# login list for this repo's host (get_gitea_login_for_host). Pass +# --login to override that default for this invocation only. The +# override is appended to the tea command line AFTER the detected default +# (get_gitea_repo_args()-equivalent resolution happens first), because tea +# honors only the LAST `--login` flag on its command line — a flag placed +# before the default would be silently clobbered by it. The `comment` +# action does not shell out to `tea` at all (see gitea_post_verified_comment +# below), so --login has no effect on it. set -e @@ -12,6 +23,7 @@ source "$SCRIPT_DIR/detect-platform.sh" PR_NUMBER="" ACTION="" COMMENT="" +LOGIN_OVERRIDE="" while [[ $# -gt 0 ]]; do case $1 in @@ -27,13 +39,18 @@ while [[ $# -gt 0 ]]; do COMMENT="$2" shift 2 ;; + -l|--login) + LOGIN_OVERRIDE="$2" + shift 2 + ;; -h|--help) - echo "Usage: pr-review.sh -n -a [-c ]" + echo "Usage: pr-review.sh -n -a [-c ] [--login ]" echo "" echo "Options:" echo " -n, --number PR number (required)" echo " -a, --action Review action: approve, request-changes, comment (required)" echo " -c, --comment Review comment (required for request-changes)" + echo " -l, --login Override the detected Gitea tea login (approve/request-changes only)" echo " -h, --help Show this help" exit 0 ;; @@ -210,8 +227,22 @@ elif [[ "$PLATFORM" == "gitea" ]]; then login=$(get_gitea_login_for_host "$host") # tea v0.11.1 defines no --comment/-comment flag on `pr approve`; # route any review body via the durable comment API instead (#835). - tea pr approve "$PR_NUMBER" --repo "$repo" --login "$login" + TEA_ARGS=(pr approve "$PR_NUMBER" --repo "$repo" --login "$login") + # --login override goes LAST: tea honors only the final --login on + # its command line, so an override placed before the detected + # default above would be silently clobbered by it. + if [[ -n "$LOGIN_OVERRIDE" ]]; then + TEA_ARGS+=(--login "$LOGIN_OVERRIDE") + fi + tea "${TEA_ARGS[@]}" echo "Approved Gitea PR #$PR_NUMBER" + # TODO(#865): this trusts tea's exit code for the approval STATE + # itself (no read-back of the review's approved status via the + # Gitea REST API). Only the optional accompanying COMMENT text + # below is independently read-back verified. Add a review-state + # read-back (e.g. GET /repos/{repo}/pulls/{pr}/reviews) if the + # approval state itself needs the same durable-provenance + # guarantee as comments. if [[ -n "$COMMENT" ]]; then comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1 echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)" @@ -227,8 +258,19 @@ elif [[ "$PLATFORM" == "gitea" ]]; then login=$(get_gitea_login_for_host "$host") # tea v0.11.1 defines no --comment/-comment flag on `pr reject`; # route the review body via the durable comment API instead (#835). - tea pr reject "$PR_NUMBER" --repo "$repo" --login "$login" + TEA_ARGS=(pr reject "$PR_NUMBER" --repo "$repo" --login "$login") + # --login override goes LAST: tea honors only the final --login on + # its command line, so an override placed before the detected + # default above would be silently clobbered by it. + if [[ -n "$LOGIN_OVERRIDE" ]]; then + TEA_ARGS+=(--login "$LOGIN_OVERRIDE") + fi + tea "${TEA_ARGS[@]}" echo "Requested changes on Gitea PR #$PR_NUMBER" + # TODO(#865): this trusts tea's exit code for the rejection STATE + # itself (no read-back of the review's rejected/changes-requested + # status via the Gitea REST API). Only the required accompanying + # COMMENT text below is independently read-back verified. comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1 echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)" ;;