Compare commits

..

4 Commits

Author SHA1 Message Date
Hermes Agent
16481ece3d fix(tools): attribute read-back to acting identity and paginate fully (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Round 2 remediation for the read-back verification in issue-comment.sh and
pr-review.sh.

BLOCKER A (invocation attribution): id-above-boundary + content/state match
only proves temporal ordering — a concurrent write from a different identity
could satisfy it while this tea invocation created nothing. Both wrappers now
resolve the acting identity once via curl GET /api/v1/user and additionally
require the accepted record's author login to equal that identity. Residual
same-identity same-body/state concurrency is documented in-code (tea 0.11.1
emits no reliable created-record id to close it further).

BLOCKER B (pagination): the comments and reviews list reads now walk every
page (?limit=&page=1,2,… until a short/empty page) for both the pre-write
boundary and the post-write read-back, so a record beyond page 1 is still
found.

Adds regressions: concurrent different-identity write fails closed (comments
and reviews); a matching review beyond page 1 is still found. README updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-21 19:02:25 -05:00
Hermes Agent
10fdd49e32 fix(tools): bound Gitea read-back to this write; verify approve/reject state (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Review remediation for two correctness holes in the #865 fix:

BLOCKER 1 — issue-comment.sh read-back was body-only across all history:
if `tea comment` silently no-opped (the #865 bug) while an identically
bodied comment already existed from a prior run, the read-back matched the
OLD comment and falsely reported success. Now record the pre-write maximum
comment id as a boundary and require a comment with id > boundary AND exact
body match; monotonic Gitea ids make id > boundary mean "created by this
write". Fails closed otherwise.

BLOCKER 2 — pr-review.sh approve/reject trusted tea's exit code for the
review STATE (same never-trust-exit-zero defect class as #865). Removed the
TODO deferral and added a real bounded read-back: record the max review id
before `tea pr approve`/`reject`, then require a review with id > boundary,
the expected state (APPROVED / REQUEST_CHANGES), and commit_id equal to the
PR's current head. Fails closed if absent.

Tests: extended test-pr-review-gitea-comment.sh to model and assert the new
review-state read-back (guardrails preserved, assertions added). Added
test-issue-comment-readback.sh proving the pre-existing-identical-body
false positive now fails closed and a genuinely new comment verifies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-21 18:21:02 -05:00
Hermes Agent
a27f1fa7df fix(tools): use top-level tea comment invocation and formalize --login passthrough (#865)
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
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 <index> <body>`
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 <noreply@anthropic.com>
2026-07-21 17:50:43 -05:00
4e5af23214 Merge pull request 'skills: add glpi-* family (solve, followup, sweep, list, create)' (#863) from feat/glpi-skills into main
All checks were successful
ci/woodpecker/push/publish Pipeline was successful
ci/woodpecker/push/ci Pipeline was successful
2026-07-21 01:09:50 +00:00
5 changed files with 971 additions and 18 deletions

View File

@@ -4,6 +4,23 @@ These scripts provide host-aware GitHub and Gitea issue, pull-request, milestone
## Durable review provenance ## Durable review provenance
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). A successful provider write command—or a wrapper message based only on that command's exit code—is **not** durable review provenance. Review comments, approvals, and change requests count as durable provenance only after the wrapper reads the created provider record back and verifies that it was created by _this_ write.
`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. `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. The `approve` and `request-changes` actions apply the same discipline to the review **state** itself: they record the maximum existing review id _before_ invoking `tea pr approve`/`reject`, then require a review whose id is strictly greater than that boundary, whose **author login equals the acting identity** (resolved via `GET /api/v1/user` for the token in use), whose state matches the requested action (`APPROVED` / `REQUEST_CHANGES`), and whose reviewed commit equals the PR's current head. tea's exit code alone is never treated as evidence the review landed.
`issue-comment.sh` applies the same fail-closed, boundary-bounded read-back to issue comments: it records the maximum existing comment id _before_ posting via `tea comment`, then re-fetches the issue's comments via the Gitea REST API and requires a comment whose id is strictly greater than that boundary, **whose author login equals the acting identity**, **and** whose body exactly matches what was submitted. Bounding the read-back by the pre-write id is essential — a body-only match across all history would falsely report success if `tea comment` silently no-ops (the #865 bug) while an identically-bodied comment already existed from a prior run. Gitea comment and review ids are monotonic, so `id > boundary` reliably means "created after this write began".
**Invocation attribution, not just temporal ordering.** `id > boundary` alone only proves a record was created after the write began; it would still be satisfied by a _concurrent_ write from a _different_ identity while this `tea` invocation created nothing. Both wrappers therefore additionally require the accepted record's author login to equal the identity the API token authenticates as, narrowing the match to this invocation's writer. The one residual window — a concurrent write by the _same_ identity with an identical body/state inside the boundary window — cannot be eliminated without a tea-emitted created-record id, which tea 0.11.1 does not reliably provide; it is strictly narrower than temporal-only matching and is documented in-code.
**Full pagination.** Gitea paginates list endpoints, so a single-page read of the comments or reviews list would false-negative once the freshly created record lands beyond the first page. Both the pre-write boundary computation and the post-write read-back walk every page (`?limit=&page=1,2,…` until a short/empty page) so the match is exhaustive regardless of how many comments or reviews already exist.
## `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 <index> <body> [--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 <name>` 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 <reviewer-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.

View File

@@ -1,6 +1,23 @@
#!/bin/bash #!/bin/bash
# issue-comment.sh - Add a comment to an issue on GitHub or Gitea # issue-comment.sh - Add a comment to an issue on GitHub or Gitea
# Usage: issue-comment.sh -i <issue_number> -c <comment> # Usage: issue-comment.sh -i <issue_number> -c <comment> [--login <name>]
#
# tea v0.11.1 defines no `comment` subcommand under `tea issue` (or `tea pr`);
# the correct invocation is the TOP-LEVEL `tea comment <index> <body>` 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 <name> 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 set -e
@@ -10,6 +27,7 @@ source "$SCRIPT_DIR/detect-platform.sh"
# Parse arguments # Parse arguments
ISSUE_NUMBER="" ISSUE_NUMBER=""
COMMENT="" COMMENT=""
LOGIN_OVERRIDE=""
while [[ $# -gt 0 ]]; do while [[ $# -gt 0 ]]; do
case $1 in case $1 in
@@ -21,12 +39,17 @@ while [[ $# -gt 0 ]]; do
COMMENT="$2" COMMENT="$2"
shift 2 shift 2
;; ;;
-l|--login)
LOGIN_OVERRIDE="$2"
shift 2
;;
-h|--help) -h|--help)
echo "Usage: issue-comment.sh -i <issue_number> -c <comment>" echo "Usage: issue-comment.sh -i <issue_number> -c <comment> [--login <name>]"
echo "" echo ""
echo "Options:" echo "Options:"
echo " -i, --issue Issue number (required)" echo " -i, --issue Issue number (required)"
echo " -c, --comment Comment text (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" echo " -h, --help Show this help"
exit 0 exit 0
;; ;;
@@ -49,6 +72,212 @@ fi
detect_platform >/dev/null detect_platform >/dev/null
# Resolve and cache the Gitea REST endpoint + token for the current remote.
# Populates GITEA_API_ROOT (…/api/v1), GITEA_API_BASE (…/api/v1/repos/<slug>),
# and GITEA_API_TOKEN. Returns non-zero (with a clear stderr message) if any
# part of the resolution fails.
gitea_resolve_api() {
local host configured_url repo
host=$(get_remote_host)
GITEA_API_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
}
GITEA_API_ROOT="${configured_url%/}/api/v1"
GITEA_API_BASE="$GITEA_API_ROOT/repos/$repo"
return 0
}
# Fetch every page of a Gitea list endpoint into $2 (merged into one JSON
# array). Gitea paginates list responses, so a single-page read would
# false-negative once a newly created record lands beyond page 1. Walks
# page=1,2,… until a short page (fewer than the requested limit) or an empty
# page is returned, so the merged array is exhaustive. $1 is the endpoint URL
# with NO query string. Returns non-zero (clear stderr) on any transport /
# HTTP / parse failure.
gitea_fetch_all() {
local base_url="$1" dest="$2" page=1 limit=50 status page_file count
printf '[]' > "$dest"
while :; do
page_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-page.XXXXXX")
if ! status=$(curl -sS -o "$page_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"${base_url}?limit=${limit}&page=${page}"); then
rm -f "$page_file"
echo "Error: Gitea list read transport failed" >&2
return 1
fi
if [[ "$status" != "200" ]]; then
rm -f "$page_file"
echo "Error: Gitea list read failed with HTTP $status" >&2
return 1
fi
count=$(DEST="$dest" python3 - "$page_file" <<'PY'
import json
import os
import sys
try:
with open(os.environ["DEST"], encoding="utf-8") as merged_file:
merged = json.load(merged_file)
with open(sys.argv[1], encoding="utf-8") as page_file:
page = json.load(page_file)
if not isinstance(page, list):
raise ValueError("page response is not a list")
merged.extend(item for item in page if isinstance(item, dict))
with open(os.environ["DEST"], "w", encoding="utf-8") as merged_file:
json.dump(merged, merged_file)
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not merge Gitea list page: {error}", file=sys.stderr)
raise SystemExit(1)
print(len(page))
PY
) || { rm -f "$page_file"; return 1; }
rm -f "$page_file"
[[ "$count" -lt "$limit" ]] && break
page=$((page + 1))
if [[ "$page" -gt 1000 ]]; then
echo "Error: Gitea list pagination exceeded 1000 pages" >&2
return 1
fi
done
return 0
}
# Resolve the login of the identity the API token authenticates as (GET
# /user). Used to attribute a read-back record to THIS invocation's writer so
# a concurrent write from a DIFFERENT identity cannot satisfy verification.
# Prints the login on success.
gitea_authenticated_login() {
local response_file status
response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-whoami.XXXXXX")
trap 'rm -f "$response_file"' RETURN
if ! status=$(curl -sS -o "$response_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"$GITEA_API_ROOT/user"); then
echo "Error: Gitea authenticated-identity read transport failed" >&2
return 1
fi
if [[ "$status" != "200" ]]; then
echo "Error: Gitea authenticated-identity read failed with HTTP $status" >&2
return 1
fi
python3 - "$response_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
user = json.load(response)
login = user.get("login") if isinstance(user, dict) else None
if not isinstance(login, str) or not login:
raise ValueError("missing authenticated login")
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not resolve authenticated Gitea identity: {error}", file=sys.stderr)
raise SystemExit(1)
print(login)
PY
}
# Print the maximum existing comment id on an issue (0 if none). This is the
# pre-write BOUNDARY: Gitea comment ids are monotonic, so any comment created
# by a subsequent write has an id strictly greater than this value.
gitea_max_comment_id() {
local issue_number="$1" merged_file
merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-boundary.XXXXXX")
trap 'rm -f "$merged_file"' RETURN
gitea_fetch_all "$GITEA_API_BASE/issues/$issue_number/comments" "$merged_file" || return 1
python3 - "$merged_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
comments = json.load(response)
ids = [c.get("id") for c in comments if isinstance(c, dict) and isinstance(c.get("id"), int)]
print(max(ids) if ids else 0)
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not compute Gitea comment boundary: {error}", file=sys.stderr)
raise SystemExit(1)
PY
}
# Independently re-fetch (all pages of) the issue's comments and require a
# comment attributable to THIS invocation: id strictly greater than the
# pre-write boundary AND author login equal to the acting identity AND exact
# body match. tea's exit code is not trustworthy evidence of a durable write
# on its own (#865); id-above-boundary alone is only temporal ordering, so the
# author-login check is what excludes a concurrent write by a DIFFERENT
# identity. Prints the matched comment ID on success.
#
# Residual (documented, not eliminable without a tea-emitted created-record
# id, which tea 0.11.1 does not reliably provide): a concurrent write by the
# SAME identity with an identical body inside the boundary window could still
# be accepted. That is a strictly narrower window than temporal-only matching.
gitea_verify_comment_posted() {
local issue_number="$1" comment_body="$2" boundary="$3" acting_login="$4"
local merged_file
merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-readback.XXXXXX")
trap 'rm -f "$merged_file"' RETURN
gitea_fetch_all "$GITEA_API_BASE/issues/$issue_number/comments" "$merged_file" || return 1
EXPECTED_COMMENT_BODY="$comment_body" BOUNDARY_COMMENT_ID="$boundary" ACTING_LOGIN="$acting_login" \
python3 - "$merged_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"]
boundary = int(os.environ["BOUNDARY_COMMENT_ID"])
acting_login = os.environ["ACTING_LOGIN"]
# Attribution to THIS write: created-after-boundary AND authored by the
# acting identity AND exact body match. The author check excludes a
# concurrent DIFFERENT-identity writer that id+body alone would admit.
matches = [
c for c in comments
if isinstance(c, dict)
and isinstance(c.get("id"), int)
and c.get("id") > boundary
and (c.get("user") or {}).get("login") == acting_login
and c.get("body") == expected_body
]
if not matches:
raise ValueError(
"no comment attributable to this write matched "
"(id > boundary, acting identity, exact body); "
"tea may have silently no-opped (#865)"
)
comment_id = max(c["id"] for c in matches)
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 if [[ "$PLATFORM" == "github" ]]; then
gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" gh issue comment "$ISSUE_NUMBER" --body "$COMMENT"
echo "Added comment to GitHub issue #$ISSUE_NUMBER" echo "Added comment to GitHub issue #$ISSUE_NUMBER"
@@ -61,8 +290,28 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
echo "Error: could not resolve a Gitea login for this repo; cannot comment on issue #$ISSUE_NUMBER." >&2 echo "Error: could not resolve a Gitea login for this repo; cannot comment on issue #$ISSUE_NUMBER." >&2
exit 1 exit 1
} }
tea issue comment "$ISSUE_NUMBER" "$COMMENT" --repo "$REPO_SLUG" --login "$GITEA_LOGIN_NAME"
echo "Added comment to Gitea issue #$ISSUE_NUMBER" # Resolve the REST endpoint, the acting identity, and the pre-write
# boundary BEFORE the write, so the read-back can require a strictly-newer
# comment id authored by this identity.
gitea_resolve_api || exit 1
ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
boundary=$(gitea_max_comment_id "$ISSUE_NUMBER") || exit 1
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" "$boundary" "$ACTING_LOGIN") || {
echo "Error: could not verify comment landed on Gitea issue #$ISSUE_NUMBER via bounded 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 else
echo "Error: Unknown platform" echo "Error: Unknown platform"
exit 1 exit 1

View File

@@ -1,6 +1,17 @@
#!/bin/bash #!/bin/bash
# pr-review.sh - Review a pull request on GitHub or Gitea # pr-review.sh - Review a pull request on GitHub or Gitea
# Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>] # Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>] [--login <name>]
#
# --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 <name> 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 set -e
@@ -12,6 +23,7 @@ source "$SCRIPT_DIR/detect-platform.sh"
PR_NUMBER="" PR_NUMBER=""
ACTION="" ACTION=""
COMMENT="" COMMENT=""
LOGIN_OVERRIDE=""
while [[ $# -gt 0 ]]; do while [[ $# -gt 0 ]]; do
case $1 in case $1 in
@@ -27,13 +39,18 @@ while [[ $# -gt 0 ]]; do
COMMENT="$2" COMMENT="$2"
shift 2 shift 2
;; ;;
-l|--login)
LOGIN_OVERRIDE="$2"
shift 2
;;
-h|--help) -h|--help)
echo "Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>]" echo "Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>] [--login <name>]"
echo "" echo ""
echo "Options:" echo "Options:"
echo " -n, --number PR number (required)" echo " -n, --number PR number (required)"
echo " -a, --action Review action: approve, request-changes, comment (required)" echo " -a, --action Review action: approve, request-changes, comment (required)"
echo " -c, --comment Review comment (required for request-changes)" 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" echo " -h, --help Show this help"
exit 0 exit 0
;; ;;
@@ -175,6 +192,258 @@ PY
return 0 return 0
} }
# Resolve and cache the Gitea REST endpoint + token for the current remote.
# Populates GITEA_API_ROOT (…/api/v1), GITEA_API_BASE (…/api/v1/repos/<slug>),
# and GITEA_API_TOKEN. Returns non-zero (with a clear stderr message) on any
# resolution failure.
gitea_resolve_api() {
local host configured_url repo
host=$(get_remote_host)
GITEA_API_TOKEN=$(get_gitea_token "$host") || {
echo "Error: Gitea token not found for review read-back verification" >&2
return 1
}
configured_url=$(get_gitea_url_for_host "$host") || {
echo "Error: Configured Gitea URL not found for review 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
}
GITEA_API_ROOT="${configured_url%/}/api/v1"
GITEA_API_BASE="$GITEA_API_ROOT/repos/$repo"
return 0
}
# Fetch every page of a Gitea list endpoint into $2 (merged into one JSON
# array). Gitea paginates list responses, so a single-page read would
# false-negative once a newly created review/comment lands beyond page 1.
# Walks page=1,2,… until a short or empty page is returned so the merged array
# is exhaustive. $1 is the endpoint URL with NO query string. Returns non-zero
# (clear stderr) on any transport / HTTP / parse failure.
gitea_fetch_all() {
local base_url="$1" dest="$2" page=1 limit=50 status page_file count
printf '[]' > "$dest"
while :; do
page_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-page.XXXXXX")
if ! status=$(curl -sS -o "$page_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"${base_url}?limit=${limit}&page=${page}"); then
rm -f "$page_file"
echo "Error: Gitea list read transport failed" >&2
return 1
fi
if [[ "$status" != "200" ]]; then
rm -f "$page_file"
echo "Error: Gitea list read failed with HTTP $status" >&2
return 1
fi
count=$(DEST="$dest" python3 - "$page_file" <<'PY'
import json
import os
import sys
try:
with open(os.environ["DEST"], encoding="utf-8") as merged_file:
merged = json.load(merged_file)
with open(sys.argv[1], encoding="utf-8") as page_file:
page = json.load(page_file)
if not isinstance(page, list):
raise ValueError("page response is not a list")
merged.extend(item for item in page if isinstance(item, dict))
with open(os.environ["DEST"], "w", encoding="utf-8") as merged_file:
json.dump(merged, merged_file)
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not merge Gitea list page: {error}", file=sys.stderr)
raise SystemExit(1)
print(len(page))
PY
) || { rm -f "$page_file"; return 1; }
rm -f "$page_file"
[[ "$count" -lt "$limit" ]] && break
page=$((page + 1))
if [[ "$page" -gt 1000 ]]; then
echo "Error: Gitea list pagination exceeded 1000 pages" >&2
return 1
fi
done
return 0
}
# Resolve the login of the identity the API token authenticates as (GET
# /user). Used to attribute a read-back review to THIS action's reviewer so a
# concurrent review from a DIFFERENT identity cannot satisfy verification.
# Prints the login on success.
gitea_authenticated_login() {
local response_file status
response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-whoami.XXXXXX")
trap 'rm -f "$response_file"' RETURN
if ! status=$(curl -sS -o "$response_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"$GITEA_API_ROOT/user"); then
echo "Error: Gitea authenticated-identity read transport failed" >&2
return 1
fi
if [[ "$status" != "200" ]]; then
echo "Error: Gitea authenticated-identity read failed with HTTP $status" >&2
return 1
fi
python3 - "$response_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
user = json.load(response)
login = user.get("login") if isinstance(user, dict) else None
if not isinstance(login, str) or not login:
raise ValueError("missing authenticated login")
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not resolve authenticated Gitea identity: {error}", file=sys.stderr)
raise SystemExit(1)
print(login)
PY
}
# Print the maximum existing review id on a PR (0 if none). This is the
# pre-write BOUNDARY: Gitea pull-review ids are monotonic, so any review
# submitted by a subsequent `tea pr approve`/`reject` has an id strictly
# greater than this value. Paginates fully so a boundary review beyond page 1
# is still counted.
gitea_max_review_id() {
local pr_number="$1" merged_file
merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-boundary.XXXXXX")
trap 'rm -f "$merged_file"' RETURN
gitea_fetch_all "$GITEA_API_BASE/pulls/$pr_number/reviews" "$merged_file" || return 1
python3 - "$merged_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
reviews = json.load(response)
ids = [r.get("id") for r in reviews if isinstance(r, dict) and isinstance(r.get("id"), int)]
print(max(ids) if ids else 0)
except (OSError, json.JSONDecodeError, TypeError, ValueError) as error:
print(f"Error: could not compute Gitea review boundary: {error}", file=sys.stderr)
raise SystemExit(1)
PY
}
# Independently verify that `tea pr approve`/`reject` produced a durable review
# record — never trust tea's exit code alone (#865, same defect class). Require
# a review attributable to THIS action: its id must be strictly greater than
# the pre-write boundary, its author login must equal the acting identity, its
# state must equal the expected state (APPROVED / REQUEST_CHANGES), and it must
# have been submitted against the PR's current head commit. The reviews list is
# paginated fully so a matching review beyond page 1 is still found. Prints the
# matched review id on success; fails closed (non-zero, clear stderr) if no
# such review is found.
#
# id-above-boundary alone is only temporal ordering; the author-login check is
# what excludes a concurrent review submitted by a DIFFERENT identity.
#
# Residual (documented, not eliminable without a tea-emitted created-record id,
# which tea 0.11.1 does not reliably provide for approve/reject): a concurrent
# review by the SAME identity with the same state against the same head inside
# the boundary window could still be accepted. That is strictly narrower than
# temporal-only matching.
#
# Args: $1 = PR number, $2 = expected state (APPROVED|REQUEST_CHANGES),
# $3 = pre-write boundary review id, $4 = acting reviewer login.
gitea_verify_review_submitted() {
local pr_number="$1" expected_state="$2" boundary="$3" acting_login="$4"
local pr_response_file reviews_merged_file status head_sha review_id
pr_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-head.XXXXXX")
reviews_merged_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-state.XXXXXX")
trap 'rm -f "$pr_response_file" "$reviews_merged_file"' RETURN
# Resolve the PR's current head commit so the review can be pinned to it.
if ! status=$(curl -sS -o "$pr_response_file" -w '%{http_code}' \
-H "Authorization: token $GITEA_API_TOKEN" \
"$GITEA_API_BASE/pulls/$pr_number"); then
echo "Error: Gitea PR head read transport failed" >&2
return 1
fi
if [[ "$status" != "200" ]]; then
echo "Error: Gitea PR head read failed with HTTP $status" >&2
return 1
fi
head_sha=$(python3 - "$pr_response_file" <<'PY'
import json
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
pr = json.load(response)
head_sha = pr.get("head", {}).get("sha") if isinstance(pr, dict) else None
if not isinstance(head_sha, str) or not head_sha:
raise ValueError("missing PR head sha")
except (OSError, json.JSONDecodeError, AttributeError, TypeError, ValueError) as error:
print(f"Error: could not resolve PR head commit: {error}", file=sys.stderr)
raise SystemExit(1)
print(head_sha)
PY
) || return 1
gitea_fetch_all "$GITEA_API_BASE/pulls/$pr_number/reviews" "$reviews_merged_file" || return 1
review_id=$(EXPECTED_STATE="$expected_state" BOUNDARY_REVIEW_ID="$boundary" EXPECTED_HEAD_SHA="$head_sha" ACTING_LOGIN="$acting_login" \
python3 - "$reviews_merged_file" <<'PY'
import json
import os
import sys
try:
with open(sys.argv[1], encoding="utf-8") as response:
reviews = json.load(response)
if not isinstance(reviews, list):
raise ValueError("response is not a review list")
expected_state = os.environ["EXPECTED_STATE"]
boundary = int(os.environ["BOUNDARY_REVIEW_ID"])
expected_head = os.environ["EXPECTED_HEAD_SHA"]
acting_login = os.environ["ACTING_LOGIN"]
# Attribution to THIS action: created-after-boundary AND submitted by the
# acting reviewer identity AND expected state AND pinned to the PR's
# current head commit. The author check excludes a concurrent
# DIFFERENT-identity review that id+state+head alone would admit.
matches = [
r for r in reviews
if isinstance(r, dict)
and isinstance(r.get("id"), int)
and r.get("id") > boundary
and (r.get("user") or {}).get("login") == acting_login
and r.get("state") == expected_state
and r.get("commit_id") == expected_head
]
if not matches:
raise ValueError(
f"no {expected_state} review attributable to this action found "
"(id > boundary, acting identity, expected state, current head); "
"tea may have silently failed (#865 defect class)"
)
review_id = max(r["id"] for r in matches)
except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as error:
print(f"Error: Gitea review persistence verification failed: {error}", file=sys.stderr)
raise SystemExit(1)
print(review_id)
PY
) || return 1
echo "$review_id"
return 0
}
if [[ "$PLATFORM" == "github" ]]; then if [[ "$PLATFORM" == "github" ]]; then
case $ACTION in case $ACTION in
approve) approve)
@@ -208,10 +477,28 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
repo=$(get_repo_slug) repo=$(get_repo_slug)
host=$(get_remote_host) host=$(get_remote_host)
login=$(get_gitea_login_for_host "$host") login=$(get_gitea_login_for_host "$host")
# Resolve the REST endpoint and record the pre-write review-id
# boundary BEFORE the write, so the read-back can require a
# strictly-newer review created by THIS action (never trust tea's
# exit code alone — #865 defect class applies to the review state).
gitea_resolve_api || exit 1
ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
review_boundary=$(gitea_max_review_id "$PR_NUMBER") || exit 1
# tea v0.11.1 defines no --comment/-comment flag on `pr approve`; # tea v0.11.1 defines no --comment/-comment flag on `pr approve`;
# route any review body via the durable comment API instead (#835). # 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")
echo "Approved Gitea PR #$PR_NUMBER" # --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[@]}"
review_id=$(gitea_verify_review_submitted "$PR_NUMBER" "APPROVED" "$review_boundary" "$ACTING_LOGIN") || {
echo "Error: could not verify an APPROVED review landed on Gitea PR #$PR_NUMBER via bounded read-back; treating tea's exit code as untrustworthy (#865)." >&2
exit 1
}
echo "Approved and verified Gitea PR #$PR_NUMBER (review ID $review_id)"
if [[ -n "$COMMENT" ]]; then if [[ -n "$COMMENT" ]]; then
comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1 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)" echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)"
@@ -225,10 +512,26 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
repo=$(get_repo_slug) repo=$(get_repo_slug)
host=$(get_remote_host) host=$(get_remote_host)
login=$(get_gitea_login_for_host "$host") login=$(get_gitea_login_for_host "$host")
# Record the pre-write review-id boundary BEFORE the write (see the
# approve path above for the rationale).
gitea_resolve_api || exit 1
ACTING_LOGIN=$(gitea_authenticated_login) || exit 1
review_boundary=$(gitea_max_review_id "$PR_NUMBER") || exit 1
# tea v0.11.1 defines no --comment/-comment flag on `pr reject`; # tea v0.11.1 defines no --comment/-comment flag on `pr reject`;
# route the review body via the durable comment API instead (#835). # 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")
echo "Requested changes on Gitea PR #$PR_NUMBER" # --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[@]}"
review_id=$(gitea_verify_review_submitted "$PR_NUMBER" "REQUEST_CHANGES" "$review_boundary" "$ACTING_LOGIN") || {
echo "Error: could not verify a REQUEST_CHANGES review landed on Gitea PR #$PR_NUMBER via bounded read-back; treating tea's exit code as untrustworthy (#865)." >&2
exit 1
}
echo "Requested changes and verified on Gitea PR #$PR_NUMBER (review ID $review_id)"
comment_id=$(gitea_post_verified_comment "$PR_NUMBER" "$COMMENT") || exit 1 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)" echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)"
;; ;;

View File

@@ -0,0 +1,265 @@
#!/usr/bin/env bash
# Regression harness for issue-comment.sh top-level `tea comment` invocation and
# its BOUNDED, INVOCATION-ATTRIBUTED, PAGINATED read-back verification (#865).
#
# The #865 bug: `tea issue comment ...` (a nonexistent subcommand on tea
# v0.11.1) silently no-ops and exits 0, so a comment is never posted. A naive
# read-back that matches ANY historical comment by body would falsely report
# success whenever an identically-bodied comment already exists from a prior
# run. Merely bounding by "id > pre-write max" is also insufficient: it accepts
# ANY newer matching comment, including one a CONCURRENT DIFFERENT identity
# posted while this tea invocation created nothing. This harness proves the
# wrapper:
# 1. uses the top-level `tea comment` form (never `tea issue comment`);
# 2. records the pre-write maximum comment id as a boundary and requires a
# strictly-newer comment on read-back, so a pre-existing identical body
# does NOT satisfy verification (fails closed);
# 3. attributes the matched comment to the acting identity (GET /user login),
# so a concurrent DIFFERENT-identity write does NOT satisfy verification;
# 4. reports success only when a genuinely new comment (id > boundary) with
# the exact body AND the acting author appears.
#
# The `tea` stub NEVER creates a comment (it mimics the silent no-op); the
# "server" comment state is modeled entirely by the curl stub's responses, so
# the fresh-success vs. no-op distinction is driven purely by whether the
# post-write read-back surfaces a new, correctly-attributed id.
set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/issue-comment-readback}"
REPO_DIR="$WORK_DIR/repo"
BIN_DIR="$WORK_DIR/bin"
TEA_LOG="$WORK_DIR/tea.log"
CURL_LOG="$WORK_DIR/curl.log"
OUTPUT_FILE="$WORK_DIR/output.log"
CREDENTIALS_FILE="$WORK_DIR/credentials.json"
CALLS_FILE="$WORK_DIR/comment_calls"
cleanup() {
rm -rf "$WORK_DIR"
}
trap cleanup EXIT
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
ISSUE_NUMBER=7
API_BASE="https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack"
API_ROOT="https://git.mosaicstack.dev/api/v1"
BODY='durable "note" -- marker'
ACTING_LOGIN="primary-reviewer"
FOREIGN_LOGIN="other-writer"
CONFIGURED_GITEA_URL="https://git.mosaicstack.dev" 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
# tea stub: resolves the login list, and treats `tea comment ...` as a silent
# no-op (exit 0 without creating anything) to mimic the real failure mode.
cat > "$BIN_DIR/tea" <<'SH'
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' "$*" >> "$ISSUE_COMMENT_TEA_LOG"
if [[ "$*" == "login list --output json" ]]; then
printf '%s\n' '[{"name":"mosaicstack","url":"https://git.mosaicstack.dev"}]'
exit 0
fi
# The wrapper must use the TOP-LEVEL `tea comment` form; the broken
# `tea issue comment` subcommand must never be invoked.
if [[ "$*" == issue\ comment* ]]; then
echo "wrapper invoked nonexistent 'tea issue comment' subcommand" >&2
exit 90
fi
if [[ "$*" == comment\ * ]]; then
# Mimic tea v0.11.1: exit 0. Whether a comment actually lands is modeled
# by the curl stub's post-write read-back response, not here.
exit 0
fi
echo "Unexpected tea command: $*" >&2
exit 92
SH
chmod +x "$BIN_DIR/tea"
# curl stub: serves GET /user (acting identity) and GET .../issues/7/comments
# (paginated: ?limit=&page=). The comments endpoint's first call = pre-write
# boundary, second call = post-write read-back. The boundary always contains a
# pre-existing comment (id 50) whose body is IDENTICAL to the one under test,
# which is exactly the condition a body-only match would trip over.
cat > "$BIN_DIR/curl" <<'SH'
#!/usr/bin/env bash
set -euo pipefail
output_file=""
method="GET"
url=""
while [[ $# -gt 0 ]]; do
case "$1" in
-o) output_file="$2"; shift 2 ;;
-w|-H) shift 2 ;;
-X) method="$2"; shift 2 ;;
-d|--data) shift 2 ;;
-s|-S|-sS) shift ;;
http://*|https://*) url="$1"; shift ;;
*) shift ;;
esac
done
# Strip any query string so pagination params don't defeat path matching, but
# still log the full URL (including ?limit=&page=) so the test can assert the
# read-back paginated.
path="${url%%\?*}"
printf '%s %s\n' "$method" "$url" >> "$ISSUE_COMMENT_CURL_LOG"
write_response() {
local status="$1" body="$2"
[[ -n "$output_file" ]] || exit 96
printf '%s' "$body" > "$output_file"
printf '%s' "$status"
}
if [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_ROOT/user" ]]; then
write_response 200 "$(ISSUE_COMMENT_LOGIN="$ISSUE_COMMENT_ACTING_LOGIN" python3 - <<'PY'
import json
import os
print(json.dumps({"login": os.environ["ISSUE_COMMENT_LOGIN"]}))
PY
)"
elif [[ "$method" == "GET" && "$path" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then
calls_file="$ISSUE_COMMENT_CALLS"
if [[ -f "$calls_file" ]]; then
# post-write read-back
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" \
ISSUE_COMMENT_ACTING_LOGIN="$ISSUE_COMMENT_ACTING_LOGIN" \
ISSUE_COMMENT_FOREIGN_LOGIN="$ISSUE_COMMENT_FOREIGN_LOGIN" \
ISSUE_COMMENT_TEST_MODE="$ISSUE_COMMENT_TEST_MODE" python3 - <<'PY'
import json
import os
body = os.environ["ISSUE_COMMENT_BODY"]
acting = os.environ["ISSUE_COMMENT_ACTING_LOGIN"]
foreign = os.environ["ISSUE_COMMENT_FOREIGN_LOGIN"]
mode = os.environ["ISSUE_COMMENT_TEST_MODE"]
if mode == "fresh-success":
# A genuinely new comment (id 60 > boundary 50) authored by the acting
# identity.
records = [
{"id": 50, "body": body, "user": {"login": acting}},
{"id": 60, "body": body, "user": {"login": acting}},
]
elif mode == "foreign-identity":
# A concurrent new comment (id 60 > boundary 50) with the SAME body but a
# DIFFERENT author. tea created nothing; attribution must reject this.
records = [
{"id": 50, "body": body, "user": {"login": acting}},
{"id": 60, "body": body, "user": {"login": foreign}},
]
else:
# no-op: nothing new landed; the pre-existing id-50 comment remains.
records = [{"id": 50, "body": body, "user": {"login": acting}}]
print(json.dumps(records))
PY
)
else
: > "$calls_file"
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" \
ISSUE_COMMENT_ACTING_LOGIN="$ISSUE_COMMENT_ACTING_LOGIN" python3 - <<'PY'
import json
import os
body = os.environ["ISSUE_COMMENT_BODY"]
acting = os.environ["ISSUE_COMMENT_ACTING_LOGIN"]
print(json.dumps([{"id": 50, "body": body, "user": {"login": acting}}]))
PY
)
fi
write_response 200 "$response"
else
echo "Unexpected curl request: $method $url" >&2
exit 97
fi
SH
chmod +x "$BIN_DIR/curl"
run_comment() {
local mode="$1"
: > "$TEA_LOG"
: > "$CURL_LOG"
: > "$OUTPUT_FILE"
rm -f "$CALLS_FILE"
(
cd "$REPO_DIR"
PATH="$BIN_DIR:$PATH" \
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
ISSUE_COMMENT_TEA_LOG="$TEA_LOG" \
ISSUE_COMMENT_CURL_LOG="$CURL_LOG" \
ISSUE_COMMENT_CALLS="$CALLS_FILE" \
ISSUE_COMMENT_TEST_MODE="$mode" \
ISSUE_COMMENT_EXPECTED_BODY="$BODY" \
ISSUE_COMMENT_ACTING_LOGIN="$ACTING_LOGIN" \
ISSUE_COMMENT_FOREIGN_LOGIN="$FOREIGN_LOGIN" \
ISSUE_COMMENT_API_BASE="$API_BASE" \
ISSUE_COMMENT_API_ROOT="$API_ROOT" \
"$SCRIPT_DIR/issue-comment.sh" -i "$ISSUE_NUMBER" -c "$BODY"
) > "$OUTPUT_FILE" 2>&1
}
# Case 1: silent no-op with a pre-existing identical body must FAIL CLOSED.
if run_comment noop-preexisting; then
echo "FAIL: wrapper reported success when tea no-opped but an identical body pre-existed" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
echo "FAIL: read-back matched a pre-existing comment by body only" >&2
exit 1
fi
# The wrapper must have used the top-level form and read comments back twice
# (boundary + post-write).
grep -q "^comment 7 " "$TEA_LOG"
if grep -q '^issue comment' "$TEA_LOG"; then
echo "FAIL: wrapper used the broken 'tea issue comment' subcommand" >&2
exit 1
fi
[[ "$(grep -c "^GET $API_BASE/issues/7/comments?" "$CURL_LOG")" == "2" ]]
# Read-back must be paginated (limit + page query params present).
grep -q "^GET $API_BASE/issues/7/comments?limit=[0-9]*&page=1$" "$CURL_LOG"
# Attribution must have resolved the acting identity via GET /user.
grep -q "^GET $API_ROOT/user$" "$CURL_LOG"
# Case 2: a concurrent DIFFERENT-identity write (id 60 > boundary, same body,
# foreign author) must FAIL CLOSED — temporal ordering is not attribution.
if run_comment foreign-identity; then
echo "FAIL: wrapper accepted a concurrent comment authored by a different identity" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Added and verified comment' "$OUTPUT_FILE"; then
echo "FAIL: read-back matched a different-identity comment (attribution bypassed)" >&2
exit 1
fi
# Case 3: a genuinely new comment (id 60 > boundary 50, acting author) verifies.
run_comment fresh-success
grep -q 'Added and verified comment on Gitea issue #7 (comment ID 60)' "$OUTPUT_FILE"
grep -q "^comment 7 " "$TEA_LOG"
echo "issue-comment.sh bounded + attributed read-back regression passed"

View File

@@ -23,6 +23,9 @@ cleanup() {
} }
trap cleanup EXIT trap cleanup EXIT
ACTING_LOGIN="review-bot"
FOREIGN_LOGIN="other-writer"
mkdir -p "$REPO_DIR" "$BIN_DIR" mkdir -p "$REPO_DIR" "$BIN_DIR"
git -C "$REPO_DIR" init -q git -C "$REPO_DIR" init -q
git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git git -C "$REPO_DIR" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git
@@ -67,7 +70,7 @@ if [[ "$*" == *" -comment "* || "$*" == *" --comment "* || "$*" == *" -comment"
fi fi
case "${PR_REVIEW_TEST_MODE:-}" in case "${PR_REVIEW_TEST_MODE:-}" in
approve) approve|paginated-approve|foreign-review)
[[ "$*" == "pr approve 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 90 [[ "$*" == "pr approve 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 90
;; ;;
request-changes) request-changes)
@@ -127,6 +130,12 @@ while [[ $# -gt 0 ]]; do
esac esac
done done
# Strip any query string so pagination params (?limit=&page=) don't defeat
# path matching, but keep the full URL in the log so tests can assert that the
# read-back paginated.
path="${url%%\?*}"
page="${url##*page=}"
[[ "$page" == "$url" ]] && page=1
printf '%s %s\n' "$method" "$url" >> "$PR_REVIEW_CURL_LOG" printf '%s %s\n' "$method" "$url" >> "$PR_REVIEW_CURL_LOG"
write_response() { write_response() {
@@ -144,8 +153,85 @@ case "${PR_REVIEW_TEST_MODE:-}" in
write-http-failure) write-http-failure)
write_response 500 '{"message":"simulated rejection"}' write_response 500 '{"message":"simulated rejection"}'
;; ;;
approve|request-changes|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-success|readback-failure) approve|request-changes|paginated-approve|foreign-review|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-success|readback-failure)
if [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then if [[ "$method" == "GET" && "$path" == "$PR_REVIEW_API_ROOT/user" ]]; then
# Acting reviewer identity used for invocation attribution.
write_response 200 "$(PR_REVIEW_LOGIN="$PR_REVIEW_ACTING_LOGIN" python3 - <<'PY'
import json
import os
print(json.dumps({"login": os.environ["PR_REVIEW_LOGIN"]}))
PY
)"
elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then
# First (boundary) call precedes the write; later calls are the
# post-write read-back that must surface a strictly-newer review
# created by THIS action (id 200 > boundary 100), authored by the
# acting identity, with the expected state and pinned to the PR's
# current head commit. The list is served PAGINATED so a target
# beyond page 1 is only found by a fully-paginating read-back.
calls_file="${PR_REVIEW_REVIEW_CALLS:-/dev/null}"
if [[ -f "$calls_file" ]]; then
phase="post"
else
: > "$calls_file"
phase="boundary"
fi
state="APPROVED"
[[ "$PR_REVIEW_TEST_MODE" == "request-changes" ]] && state="REQUEST_CHANGES"
response=$(PR_REVIEW_PHASE="$phase" PR_REVIEW_PAGE="$page" \
PR_REVIEW_MODE="$PR_REVIEW_TEST_MODE" PR_REVIEW_STATE="$state" \
PR_REVIEW_ACTING_LOGIN="$PR_REVIEW_ACTING_LOGIN" \
PR_REVIEW_FOREIGN_LOGIN="$PR_REVIEW_FOREIGN_LOGIN" python3 - <<'PY'
import json
import os
phase = os.environ["PR_REVIEW_PHASE"]
page = int(os.environ["PR_REVIEW_PAGE"])
mode = os.environ["PR_REVIEW_MODE"]
state = os.environ["PR_REVIEW_STATE"]
acting = os.environ["PR_REVIEW_ACTING_LOGIN"]
foreign = os.environ["PR_REVIEW_FOREIGN_LOGIN"]
def review(review_id, review_state, commit, login):
return {
"id": review_id,
"state": review_state,
"commit_id": commit,
"user": {"login": login},
}
if phase == "boundary":
records = [review(100, "COMMENT", "oldsha0000", acting)] if page == 1 else []
elif mode == "paginated-approve":
# A full first page (50 non-matching records) forces the read-back to
# request page 2, where the genuine matching review lives.
if page == 1:
records = [review(101 + i, "COMMENT", "oldsha0000", acting) for i in range(50)]
elif page == 2:
records = [review(200, state, "HEADSHA_FEEDFACE", acting)]
else:
records = []
elif mode == "foreign-review":
# A concurrent APPROVED review at the current head, but authored by a
# DIFFERENT identity. tea created nothing; attribution must reject this.
records = [review(200, state, "HEADSHA_FEEDFACE", foreign)] if page == 1 else []
else:
if page == 1:
records = [
review(100, "COMMENT", "oldsha0000", acting),
review(200, state, "HEADSHA_FEEDFACE", acting),
]
else:
records = []
print(json.dumps(records))
PY
)
write_response 200 "$response"
elif [[ "$method" == "GET" && "$path" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123" ]]; then
write_response 200 '{"head":{"sha":"HEADSHA_FEEDFACE"}}'
elif [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then
PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY'
import json import json
import os import os
@@ -196,38 +282,70 @@ run_review() {
local remote_url="${5:-https://git.mosaicstack.dev/mosaicstack/stack.git}" local remote_url="${5:-https://git.mosaicstack.dev/mosaicstack/stack.git}"
local expected_repo="${6:-mosaicstack/stack}" local expected_repo="${6:-mosaicstack/stack}"
local expected_api_base="${configured_url%/}/api/v1/repos/$expected_repo" local expected_api_base="${configured_url%/}/api/v1/repos/$expected_repo"
local expected_api_root="${configured_url%/}/api/v1"
git -C "$REPO_DIR" remote set-url origin "$remote_url" git -C "$REPO_DIR" remote set-url origin "$remote_url"
write_credentials "$configured_url" write_credentials "$configured_url"
: > "$TEA_LOG" : > "$TEA_LOG"
: > "$CURL_LOG" : > "$CURL_LOG"
: > "$OUTPUT_FILE" : > "$OUTPUT_FILE"
rm -f "$WORK_DIR/review_calls"
( (
cd "$REPO_DIR" cd "$REPO_DIR"
PATH="$BIN_DIR:$PATH" \ PATH="$BIN_DIR:$PATH" \
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
PR_REVIEW_TEA_LOG="$TEA_LOG" \ PR_REVIEW_TEA_LOG="$TEA_LOG" \
PR_REVIEW_CURL_LOG="$CURL_LOG" \ PR_REVIEW_CURL_LOG="$CURL_LOG" \
PR_REVIEW_REVIEW_CALLS="$WORK_DIR/review_calls" \
PR_REVIEW_TEST_MODE="$mode" \ PR_REVIEW_TEST_MODE="$mode" \
PR_REVIEW_EXPECTED_BODY="$comment" \ PR_REVIEW_EXPECTED_BODY="$comment" \
PR_REVIEW_EXPECTED_API_BASE="$expected_api_base" \ PR_REVIEW_EXPECTED_API_BASE="$expected_api_base" \
PR_REVIEW_API_ROOT="$expected_api_root" \
PR_REVIEW_ACTING_LOGIN="$ACTING_LOGIN" \
PR_REVIEW_FOREIGN_LOGIN="$FOREIGN_LOGIN" \
"$SCRIPT_DIR/pr-review.sh" -n 123 -a "$action" ${comment:+-c "$comment"} "$SCRIPT_DIR/pr-review.sh" -n 123 -a "$action" ${comment:+-c "$comment"}
) > "$OUTPUT_FILE" 2>&1 ) > "$OUTPUT_FILE" 2>&1
} }
run_review approve approve run_review approve approve
grep -q '^pr approve 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG" grep -q '^pr approve 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG"
grep -q 'Approved Gitea PR #123' "$OUTPUT_FILE" grep -q 'Approved and verified Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
# #865: the approval STATE itself is read back — a pre-write boundary GET and a
# post-write read-back GET on the (paginated) reviews endpoint, plus a PR head
# lookup and an acting-identity (GET /user) resolution for attribution.
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=1$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/user$' "$CURL_LOG"
if grep -q 'comment' "$TEA_LOG"; then if grep -q 'comment' "$TEA_LOG"; then
echo "Plain approve (no review body) unexpectedly touched comment persistence" >&2 echo "Plain approve (no review body) unexpectedly touched comment persistence" >&2
exit 1 exit 1
fi fi
# #865 (invocation attribution): a concurrent APPROVED review at the current
# head, authored by a DIFFERENT identity while tea created nothing, must NOT
# satisfy verification — id-above-boundary + state + head is only temporal
# ordering, not proof THIS reviewer wrote it.
if run_review foreign-review approve; then
echo "FAIL: approve accepted a review authored by a different identity" >&2
cat "$OUTPUT_FILE" >&2
exit 1
fi
if grep -q 'Approved and verified' "$OUTPUT_FILE"; then
echo "FAIL: read-back matched a different-identity review (attribution bypassed)" >&2
exit 1
fi
# #865 (pagination): a genuine matching review that lands beyond page 1 of the
# reviews list must still be found by a fully-paginating read-back.
run_review paginated-approve approve
grep -q 'Approved and verified Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=2$' "$CURL_LOG"
# #835: tea v0.11.1 defines no --comment/-comment flag on `pr approve`. A # #835: tea v0.11.1 defines no --comment/-comment flag on `pr approve`. A
# review body supplied alongside approve must be routed through the durable # review body supplied alongside approve must be routed through the durable
# comment REST API instead of being passed to `tea` directly. # comment REST API instead of being passed to `tea` directly.
run_review approve approve approve-note run_review approve approve approve-note
grep -q '^pr approve 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG" grep -q '^pr approve 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG"
grep -q 'Approved Gitea PR #123' "$OUTPUT_FILE" grep -q 'Approved and verified Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG" grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG"
grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE" grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE"
@@ -235,7 +353,8 @@ grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$
# #835: same for `pr reject` (request-changes), where a comment is required. # #835: same for `pr reject` (request-changes), where a comment is required.
run_review request-changes request-changes changes-required run_review request-changes request-changes changes-required
grep -q '^pr reject 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG" grep -q '^pr reject 123 --repo mosaicstack/stack --login mosaicstack$' "$TEA_LOG"
grep -q 'Requested changes on Gitea PR #123' "$OUTPUT_FILE" grep -q 'Requested changes and verified on Gitea PR #123 (review ID 200)' "$OUTPUT_FILE"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews?limit=[0-9]*&page=1$' "$CURL_LOG"
grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/123/comments$' "$CURL_LOG"
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG" grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/issues/comments/456$' "$CURL_LOG"
grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE" grep -q 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE"