Compare commits
9 Commits
fix/835-pr
...
fix/865-te
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
10fdd49e32 | ||
|
|
a27f1fa7df | ||
| 4e5af23214 | |||
|
|
880c28b191 | ||
|
|
7bc2dfb6c8 | ||
| b0d78d8632 | |||
| 344d86a635 | |||
| acd7d380f6 | |||
| 3b70c66c07 |
@@ -4,6 +4,19 @@ These scripts provide host-aware GitHub and Gitea issue, pull-request, milestone
|
||||
|
||||
## 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 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 **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".
|
||||
|
||||
## `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.
|
||||
|
||||
@@ -91,13 +91,19 @@ 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:
|
||||
# Normalize by scheme: an implicit (portless) HTTP(S) URL and its explicit
|
||||
# default-port form (":80" for http, ":443" for https) name the same
|
||||
# provider endpoint. Apply that equivalence symmetrically -- whichever side
|
||||
# omits the port is treated as carrying the scheme's default port -- so
|
||||
# "configured implicit vs. remote explicit" and "configured explicit vs.
|
||||
# remote implicit" both match. (The remote side here is always an HTTP(S)
|
||||
# authority; an SSH remote's transport port is stripped by get_remote_host
|
||||
# before reaching this comparison, since it identifies an unrelated
|
||||
# service on the same host, not the HTTP(S) provider port.)
|
||||
default_port = 80 if configured.scheme == "http" else 443
|
||||
normalized_configured = configured.port if configured.port is not None else default_port
|
||||
normalized_remote = remote.port if remote.port is not None else default_port
|
||||
if normalized_configured != normalized_remote:
|
||||
raise SystemExit(1)
|
||||
raise SystemExit(0)
|
||||
PY
|
||||
@@ -432,7 +438,11 @@ get_remote_host() {
|
||||
fi
|
||||
if [[ "$remote_url" =~ ^ssh://([^/]+)/ ]]; then
|
||||
local host="${BASH_REMATCH[1]}"
|
||||
echo "${host##*@}"
|
||||
host="${host##*@}"
|
||||
# Strip an SSH transport port (e.g. "git.example:2222"): it names the
|
||||
# SSH daemon port, not the HTTP(S) provider API port, and must not
|
||||
# feed gitea_url_matches_host's port comparison (#850).
|
||||
echo "${host%%:*}"
|
||||
return 0
|
||||
fi
|
||||
if [[ "$remote_url" =~ ^git@([^:]+): ]]; then
|
||||
|
||||
@@ -1,6 +1,23 @@
|
||||
#!/bin/bash
|
||||
# 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
|
||||
|
||||
@@ -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 <issue_number> -c <comment>"
|
||||
echo "Usage: issue-comment.sh -i <issue_number> -c <comment> [--login <name>]"
|
||||
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,127 @@ fi
|
||||
|
||||
detect_platform >/dev/null
|
||||
|
||||
# Resolve and cache the Gitea REST endpoint + token for the current remote.
|
||||
# Populates GITEA_API_BASE 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_BASE="${configured_url%/}/api/v1/repos/$repo"
|
||||
return 0
|
||||
}
|
||||
|
||||
# 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. Bounding
|
||||
# the read-back this way is what distinguishes a genuine fresh write from a
|
||||
# pre-existing comment that merely happens to share the same body — the exact
|
||||
# false-positive a body-only, whole-history match would miss when `tea
|
||||
# comment` silently no-ops (#865).
|
||||
gitea_max_comment_id() {
|
||||
local issue_number="$1" response_file status
|
||||
|
||||
response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-boundary.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_BASE/issues/$issue_number/comments"); then
|
||||
echo "Error: Gitea comment boundary read transport failed" >&2
|
||||
return 1
|
||||
fi
|
||||
if [[ "$status" != "200" ]]; then
|
||||
echo "Error: Gitea comment boundary 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:
|
||||
comments = json.load(response)
|
||||
if not isinstance(comments, list):
|
||||
raise ValueError("response is not a comment list")
|
||||
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 the issue's comments via the Gitea REST API and
|
||||
# require a comment that was created by THIS write: its id must be strictly
|
||||
# greater than the pre-write boundary AND its body must exactly match what we
|
||||
# submitted (see header comment: tea's exit code is not trustworthy evidence
|
||||
# of a durable write on its own). Prints the matched comment ID on success.
|
||||
gitea_verify_comment_posted() {
|
||||
local issue_number="$1" comment_body="$2" boundary="$3"
|
||||
local readback_response_file status
|
||||
|
||||
readback_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-issue-comment-readback.XXXXXX")
|
||||
trap 'rm -f "$readback_response_file"' RETURN
|
||||
|
||||
if ! status=$(curl -sS -o "$readback_response_file" -w '%{http_code}' \
|
||||
-H "Authorization: token $GITEA_API_TOKEN" \
|
||||
"$GITEA_API_BASE/issues/$issue_number/comments"); then
|
||||
echo "Error: Gitea comment read-back transport failed" >&2
|
||||
return 1
|
||||
fi
|
||||
if [[ "$status" != "200" ]]; then
|
||||
echo "Error: Gitea comment read-back failed with HTTP $status" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
EXPECTED_COMMENT_BODY="$comment_body" BOUNDARY_COMMENT_ID="$boundary" \
|
||||
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"]
|
||||
boundary = int(os.environ["BOUNDARY_COMMENT_ID"])
|
||||
# Require both: created-after-boundary (fresh write) AND exact body match.
|
||||
matches = [
|
||||
c for c in comments
|
||||
if isinstance(c, dict)
|
||||
and isinstance(c.get("id"), int)
|
||||
and c.get("id") > boundary
|
||||
and c.get("body") == expected_body
|
||||
]
|
||||
if not matches:
|
||||
raise ValueError(
|
||||
"no comment created by this write matched (id > boundary and 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
|
||||
gh issue comment "$ISSUE_NUMBER" --body "$COMMENT"
|
||||
echo "Added comment to GitHub issue #$ISSUE_NUMBER"
|
||||
@@ -61,8 +205,26 @@ 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"
|
||||
|
||||
# Resolve the REST endpoint and record the pre-write boundary BEFORE the
|
||||
# write, so the read-back can require a strictly-newer comment id.
|
||||
gitea_resolve_api || 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") || {
|
||||
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
|
||||
echo "Error: Unknown platform"
|
||||
exit 1
|
||||
|
||||
@@ -1,6 +1,17 @@
|
||||
#!/bin/bash
|
||||
# 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
|
||||
|
||||
@@ -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 <pr_number> -a <action> [-c <comment>]"
|
||||
echo "Usage: pr-review.sh -n <pr_number> -a <action> [-c <comment>] [--login <name>]"
|
||||
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
|
||||
;;
|
||||
@@ -175,6 +192,171 @@ PY
|
||||
return 0
|
||||
}
|
||||
|
||||
# Resolve and cache the Gitea REST endpoint + token for the current remote.
|
||||
# Populates GITEA_API_BASE 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_BASE="${configured_url%/}/api/v1/repos/$repo"
|
||||
return 0
|
||||
}
|
||||
|
||||
# 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. Bounding the read-back this way is what turns the
|
||||
# check into a genuine write-verification rather than a match against any
|
||||
# historical review — the same never-trust-exit-zero discipline #865 requires
|
||||
# for comments, applied to the review STATE itself.
|
||||
gitea_max_review_id() {
|
||||
local pr_number="$1" response_file status
|
||||
|
||||
response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-boundary.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_BASE/pulls/$pr_number/reviews"); then
|
||||
echo "Error: Gitea review boundary read transport failed" >&2
|
||||
return 1
|
||||
fi
|
||||
if [[ "$status" != "200" ]]; then
|
||||
echo "Error: Gitea review boundary 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:
|
||||
reviews = json.load(response)
|
||||
if not isinstance(reviews, list):
|
||||
raise ValueError("response is not a review list")
|
||||
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 that was created by THIS action: its id must be strictly greater
|
||||
# than the pre-write boundary, its state must equal the expected state
|
||||
# (APPROVED / REQUEST_CHANGES), and it must have been submitted against the
|
||||
# PR's current head commit. Prints the matched review id on success; fails
|
||||
# closed (non-zero, clear stderr) if no such review is found.
|
||||
#
|
||||
# Args: $1 = PR number, $2 = expected state (APPROVED|REQUEST_CHANGES),
|
||||
# $3 = pre-write boundary review id.
|
||||
gitea_verify_review_submitted() {
|
||||
local pr_number="$1" expected_state="$2" boundary="$3"
|
||||
local pr_response_file reviews_response_file status head_sha review_id
|
||||
|
||||
pr_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-head.XXXXXX")
|
||||
reviews_response_file=$(mktemp "${TMPDIR:-/tmp}/mosaic-pr-review-state.XXXXXX")
|
||||
trap 'rm -f "$pr_response_file" "$reviews_response_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
|
||||
|
||||
if ! status=$(curl -sS -o "$reviews_response_file" -w '%{http_code}' \
|
||||
-H "Authorization: token $GITEA_API_TOKEN" \
|
||||
"$GITEA_API_BASE/pulls/$pr_number/reviews"); then
|
||||
echo "Error: Gitea review read-back transport failed" >&2
|
||||
return 1
|
||||
fi
|
||||
if [[ "$status" != "200" ]]; then
|
||||
echo "Error: Gitea review read-back failed with HTTP $status" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
review_id=$(EXPECTED_STATE="$expected_state" BOUNDARY_REVIEW_ID="$boundary" EXPECTED_HEAD_SHA="$head_sha" \
|
||||
python3 - "$reviews_response_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"]
|
||||
# Require all of: created-after-boundary (this action's write), the
|
||||
# expected review state, and pinned to the PR's current head commit.
|
||||
# The monotonic id boundary is what proves "submitted by this action"
|
||||
# rather than matching some pre-existing historical review.
|
||||
matches = [
|
||||
r for r in reviews
|
||||
if isinstance(r, dict)
|
||||
and isinstance(r.get("id"), int)
|
||||
and r.get("id") > boundary
|
||||
and r.get("state") == expected_state
|
||||
and r.get("commit_id") == expected_head
|
||||
]
|
||||
if not matches:
|
||||
raise ValueError(
|
||||
f"no {expected_state} review created by this action found "
|
||||
"(id > boundary, 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
|
||||
case $ACTION in
|
||||
approve)
|
||||
@@ -208,10 +390,27 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
|
||||
repo=$(get_repo_slug)
|
||||
host=$(get_remote_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
|
||||
review_boundary=$(gitea_max_review_id "$PR_NUMBER") || exit 1
|
||||
# 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"
|
||||
echo "Approved Gitea PR #$PR_NUMBER"
|
||||
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[@]}"
|
||||
review_id=$(gitea_verify_review_submitted "$PR_NUMBER" "APPROVED" "$review_boundary") || {
|
||||
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
|
||||
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)"
|
||||
@@ -225,10 +424,25 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
|
||||
repo=$(get_repo_slug)
|
||||
host=$(get_remote_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
|
||||
review_boundary=$(gitea_max_review_id "$PR_NUMBER") || exit 1
|
||||
# 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"
|
||||
echo "Requested changes on Gitea PR #$PR_NUMBER"
|
||||
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[@]}"
|
||||
review_id=$(gitea_verify_review_submitted "$PR_NUMBER" "REQUEST_CHANGES" "$review_boundary") || {
|
||||
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
|
||||
echo "Added and verified review comment on Gitea PR #$PR_NUMBER (comment ID $comment_id)"
|
||||
;;
|
||||
|
||||
216
packages/mosaic/framework/tools/git/test-issue-comment-readback.sh
Executable file
216
packages/mosaic/framework/tools/git/test-issue-comment-readback.sh
Executable file
@@ -0,0 +1,216 @@
|
||||
#!/usr/bin/env bash
|
||||
# Regression harness for issue-comment.sh top-level `tea comment` invocation and
|
||||
# its BOUNDED 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. 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. reports success only when a genuinely new comment (id > boundary) with
|
||||
# the exact body 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 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"
|
||||
BODY='durable "note" -- marker'
|
||||
|
||||
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 .../issues/7/comments. 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
|
||||
|
||||
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" && "$url" == "$ISSUE_COMMENT_API_BASE/issues/7/comments" ]]; then
|
||||
calls_file="$ISSUE_COMMENT_CALLS"
|
||||
if [[ -f "$calls_file" ]]; then
|
||||
# post-write read-back
|
||||
if [[ "$ISSUE_COMMENT_TEST_MODE" == "fresh-success" ]]; then
|
||||
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" python3 - <<'PY'
|
||||
import json
|
||||
import os
|
||||
|
||||
body = os.environ["ISSUE_COMMENT_BODY"]
|
||||
print(json.dumps([
|
||||
{"id": 50, "body": body},
|
||||
{"id": 60, "body": body},
|
||||
]))
|
||||
PY
|
||||
)
|
||||
else
|
||||
# no-op: nothing new landed; the pre-existing id-50 comment remains.
|
||||
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" python3 - <<'PY'
|
||||
import json
|
||||
import os
|
||||
|
||||
body = os.environ["ISSUE_COMMENT_BODY"]
|
||||
print(json.dumps([{"id": 50, "body": body}]))
|
||||
PY
|
||||
)
|
||||
fi
|
||||
else
|
||||
: > "$calls_file"
|
||||
response=$(ISSUE_COMMENT_BODY="$ISSUE_COMMENT_EXPECTED_BODY" python3 - <<'PY'
|
||||
import json
|
||||
import os
|
||||
|
||||
body = os.environ["ISSUE_COMMENT_BODY"]
|
||||
print(json.dumps([{"id": 50, "body": body}]))
|
||||
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_API_BASE="$API_BASE" \
|
||||
"$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" ]]
|
||||
|
||||
# Case 2: a genuinely new comment (id 60 > boundary 50) verifies successfully.
|
||||
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 read-back regression passed"
|
||||
@@ -73,7 +73,7 @@ case "${PR_REVIEW_TEST_MODE:-}" in
|
||||
request-changes)
|
||||
[[ "$*" == "pr reject 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 91
|
||||
;;
|
||||
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)
|
||||
legacy-fallback|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-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'
|
||||
@@ -144,8 +144,34 @@ case "${PR_REVIEW_TEST_MODE:-}" in
|
||||
write-http-failure)
|
||||
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|readback-failure)
|
||||
if [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then
|
||||
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)
|
||||
if [[ "$method" == "GET" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/pulls/123/reviews" ]]; then
|
||||
# First GET = pre-write review-id BOUNDARY; second GET = post-write
|
||||
# read-back that must surface a strictly-newer review created by
|
||||
# THIS action (id 200 > boundary 100), with the expected state and
|
||||
# pinned to the PR's current head commit.
|
||||
calls_file="${PR_REVIEW_REVIEW_CALLS:-/dev/null}"
|
||||
if [[ -f "$calls_file" ]]; then
|
||||
state="APPROVED"
|
||||
[[ "$PR_REVIEW_TEST_MODE" == "request-changes" ]] && state="REQUEST_CHANGES"
|
||||
response=$(PR_REVIEW_STATE="$state" python3 - <<'PY'
|
||||
import json
|
||||
import os
|
||||
|
||||
print(json.dumps([
|
||||
{"id": 100, "state": "COMMENT", "commit_id": "oldsha0000"},
|
||||
{"id": 200, "state": os.environ["PR_REVIEW_STATE"], "commit_id": "HEADSHA_FEEDFACE"},
|
||||
]))
|
||||
PY
|
||||
)
|
||||
else
|
||||
: > "$calls_file"
|
||||
response='[{"id":100,"state":"COMMENT","commit_id":"oldsha0000"}]'
|
||||
fi
|
||||
write_response 200 "$response"
|
||||
elif [[ "$method" == "GET" && "$url" == "$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'
|
||||
import json
|
||||
import os
|
||||
@@ -201,12 +227,14 @@ run_review() {
|
||||
: > "$TEA_LOG"
|
||||
: > "$CURL_LOG"
|
||||
: > "$OUTPUT_FILE"
|
||||
rm -f "$WORK_DIR/review_calls"
|
||||
(
|
||||
cd "$REPO_DIR"
|
||||
PATH="$BIN_DIR:$PATH" \
|
||||
MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \
|
||||
PR_REVIEW_TEA_LOG="$TEA_LOG" \
|
||||
PR_REVIEW_CURL_LOG="$CURL_LOG" \
|
||||
PR_REVIEW_REVIEW_CALLS="$WORK_DIR/review_calls" \
|
||||
PR_REVIEW_TEST_MODE="$mode" \
|
||||
PR_REVIEW_EXPECTED_BODY="$comment" \
|
||||
PR_REVIEW_EXPECTED_API_BASE="$expected_api_base" \
|
||||
@@ -216,7 +244,11 @@ run_review() {
|
||||
|
||||
run_review approve approve
|
||||
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 reviews endpoint, plus a PR head lookup.
|
||||
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123/reviews$' "$CURL_LOG"
|
||||
grep -q '^GET https://git.mosaicstack.dev/api/v1/repos/mosaicstack/stack/pulls/123$' "$CURL_LOG"
|
||||
if grep -q 'comment' "$TEA_LOG"; then
|
||||
echo "Plain approve (no review body) unexpectedly touched comment persistence" >&2
|
||||
exit 1
|
||||
@@ -227,7 +259,7 @@ fi
|
||||
# comment REST API instead of being passed to `tea` directly.
|
||||
run_review approve approve approve-note
|
||||
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 '^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"
|
||||
@@ -235,7 +267,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.
|
||||
run_review request-changes request-changes changes-required
|
||||
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$' "$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 'Added and verified review comment on Gitea PR #123 (comment ID 456)' "$OUTPUT_FILE"
|
||||
@@ -291,6 +324,23 @@ grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$'
|
||||
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"
|
||||
|
||||
# #850 (follow-up to #812): an SSH remote's transport port (e.g. `ssh://
|
||||
# git@host:2222/...`) must NOT be compared against the configured HTTP(S) API
|
||||
# URL's port -- they identify unrelated properties (SSH daemon port vs. HTTP(S)
|
||||
# provider port) of the same Gitea host. Before the fix, host-match required
|
||||
# the configured URL to carry the identical port, so this failed closed even
|
||||
# though both remote and configured URL name the same host.
|
||||
run_review ssh-transport-port-success comment durable-body https://git.example ssh://git@git.example:2222/owner/repo.git owner/repo
|
||||
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
|
||||
|
||||
# #850 (follow-up to #812): an explicit default HTTP(S) port on the remote
|
||||
# (`https://host:443/...`) must be treated as equal to an implicit
|
||||
# (portless) configured URL on BOTH sides -- the pre-fix comparison only
|
||||
# normalized the default port when the REMOTE side was portless, so the
|
||||
# inverse (explicit remote, implicit configured) form failed closed.
|
||||
run_review explicit-default-port-success comment durable-body https://git.example https://git.example:443/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
|
||||
|
||||
@@ -50,6 +50,21 @@ if ! [[ "$FILE_PATH" =~ \.(ts|tsx|js|jsx|mjs|cjs)$ ]]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Deps preflight (#856): this hook is the common gate-entry seam the delivery
|
||||
# cycle invokes on every Edit/Write/MultiEdit — it fires before any pnpm-based
|
||||
# gate (test/lint/typecheck/format:check) runs against the edited file. In a
|
||||
# freshly created git worktree (pnpm workspaces do NOT share node_modules
|
||||
# across worktrees), node_modules/.bin is empty until `pnpm install` has run,
|
||||
# so gate binaries (tsc/eslint/prettier/vitest) fail with a raw, illegible
|
||||
# `sh: 1: <tool>: not found` that is indistinguishable from a real failure.
|
||||
# Fail legibly here instead, before that raw error has a chance to surface.
|
||||
BIN_DIR="$PROJECT_ROOT/node_modules/.bin"
|
||||
if [ ! -d "$BIN_DIR" ] || [ -z "$(ls -A "$BIN_DIR" 2>/dev/null)" ]; then
|
||||
echo "deps not installed — run pnpm install" >&2
|
||||
echo "[$(date '+%Y-%m-%d %H:%M:%S')] [ERROR] deps not installed — run pnpm install ($BIN_DIR is missing or empty)" >> "$LOG_FILE"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Call the main QA handler with extracted parameters
|
||||
if [ -f ~/.config/mosaic/tools/qa/qa-hook-handler.sh ]; then
|
||||
echo "[$(date '+%Y-%m-%d %H:%M:%S')] Calling QA handler for $FILE_PATH" >> "$LOG_FILE"
|
||||
|
||||
116
packages/mosaic/framework/tools/qa/test-deps-preflight.sh
Executable file
116
packages/mosaic/framework/tools/qa/test-deps-preflight.sh
Executable file
@@ -0,0 +1,116 @@
|
||||
#!/usr/bin/env bash
|
||||
# Regression harness for #856: worker git-worktrees under a fresh `git worktree
|
||||
# add` have no node_modules until `pnpm install` runs (pnpm workspaces do NOT
|
||||
# share node_modules across worktrees). Before the fix, the gate-entry seam
|
||||
# (qa-hook-stdin.sh, registered as the PostToolUse hook for every Edit/Write/
|
||||
# MultiEdit in runtime/claude/settings.json) silently let a raw
|
||||
# `sh: 1: <tool>: not found` surface from any downstream gate invocation —
|
||||
# indistinguishable from a real test/lint failure (false-red).
|
||||
#
|
||||
# Asserts:
|
||||
# 1. RED (documented): a completely fresh worktree with no node_modules/.bin
|
||||
# at all produces the raw "not found" for a gate binary — this is the
|
||||
# defect the fix prevents from reaching the operator un-annotated.
|
||||
# 2. With node_modules/.bin missing entirely, the seam exits nonzero with
|
||||
# the legible sentinel "deps not installed — run pnpm install" instead
|
||||
# of silently proceeding (exit 0) into a would-be raw not-found.
|
||||
# 3. With node_modules/.bin present but empty, same legible-sentinel
|
||||
# behavior (covers `git worktree add` immediately followed by an
|
||||
# as-yet-incomplete/interrupted install).
|
||||
# 4. Once node_modules/.bin is populated (post `pnpm install`), the seam
|
||||
# proceeds normally (exit 0) — the preflight does not false-positive.
|
||||
# 5. Non-JS/TS files are unaffected (existing skip behavior preserved).
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
HOOK="$SCRIPT_DIR/qa-hook-stdin.sh"
|
||||
|
||||
TMP_DIR=$(mktemp -d)
|
||||
trap 'rm -rf "$TMP_DIR"' EXIT
|
||||
|
||||
fail=0
|
||||
|
||||
fail_msg() {
|
||||
echo "FAIL: $*" >&2
|
||||
fail=1
|
||||
}
|
||||
|
||||
run_hook() {
|
||||
local file_path="$1"
|
||||
printf '{"tool_name":"Edit","tool_input":{"file_path":"%s"}}' "$file_path" | "$HOOK"
|
||||
}
|
||||
|
||||
make_fixture_repo() {
|
||||
local dir="$1"
|
||||
mkdir -p "$dir"
|
||||
git -C "$dir" init -q .
|
||||
git -C "$dir" -c user.email=fixture@test -c user.name=fixture commit -q --allow-empty -m init
|
||||
}
|
||||
|
||||
# --- Scenario 1: RED — document the pre-fix raw not-found a gate hits when
|
||||
# node_modules/.bin is entirely absent (this is what the preflight now
|
||||
# intercepts before any gate command runs).
|
||||
RED_DIR="$TMP_DIR/red-fixture"
|
||||
make_fixture_repo "$RED_DIR"
|
||||
RED_OUTPUT=$(PATH="/usr/bin:/bin" sh -c 'tsc --noEmit' 2>&1) && RED_STATUS=0 || RED_STATUS=$?
|
||||
case "$RED_OUTPUT" in
|
||||
*"not found"*) ;;
|
||||
*) fail_msg "expected the raw un-preflighted invocation to demonstrate 'not found'; got: $RED_OUTPUT" ;;
|
||||
esac
|
||||
[[ "$RED_STATUS" -ne 0 ]] || fail_msg "expected raw invocation without deps installed to fail"
|
||||
|
||||
# --- Scenario 2: node_modules/.bin missing entirely -> legible sentinel, nonzero.
|
||||
MISSING_DIR="$TMP_DIR/missing-bin"
|
||||
make_fixture_repo "$MISSING_DIR"
|
||||
echo "console.log(1)" > "$MISSING_DIR/x.ts"
|
||||
OUTPUT=$(cd "$MISSING_DIR" && run_hook "$MISSING_DIR/x.ts" 2>&1) && STATUS=0 || STATUS=$?
|
||||
[[ "$STATUS" -ne 0 ]] || fail_msg "missing node_modules/.bin: expected nonzero exit, got 0"
|
||||
case "$OUTPUT" in
|
||||
*"deps not installed"*"pnpm install"*) ;;
|
||||
*) fail_msg "missing node_modules/.bin: expected legible sentinel, got: $OUTPUT" ;;
|
||||
esac
|
||||
|
||||
# --- Scenario 3: node_modules/.bin present but empty -> legible sentinel, nonzero.
|
||||
EMPTY_DIR="$TMP_DIR/empty-bin"
|
||||
make_fixture_repo "$EMPTY_DIR"
|
||||
mkdir -p "$EMPTY_DIR/node_modules/.bin"
|
||||
echo "console.log(1)" > "$EMPTY_DIR/x.ts"
|
||||
OUTPUT=$(cd "$EMPTY_DIR" && run_hook "$EMPTY_DIR/x.ts" 2>&1) && STATUS=0 || STATUS=$?
|
||||
[[ "$STATUS" -ne 0 ]] || fail_msg "empty node_modules/.bin: expected nonzero exit, got 0"
|
||||
case "$OUTPUT" in
|
||||
*"deps not installed"*"pnpm install"*) ;;
|
||||
*) fail_msg "empty node_modules/.bin: expected legible sentinel, got: $OUTPUT" ;;
|
||||
esac
|
||||
|
||||
# --- Scenario 4: node_modules/.bin populated (post `pnpm install`) -> proceeds normally.
|
||||
OK_DIR="$TMP_DIR/installed-bin"
|
||||
make_fixture_repo "$OK_DIR"
|
||||
mkdir -p "$OK_DIR/node_modules/.bin"
|
||||
printf '#!/bin/sh\necho ok\n' > "$OK_DIR/node_modules/.bin/tsc"
|
||||
chmod +x "$OK_DIR/node_modules/.bin/tsc"
|
||||
echo "console.log(1)" > "$OK_DIR/x.ts"
|
||||
OUTPUT=$(cd "$OK_DIR" && run_hook "$OK_DIR/x.ts" 2>&1) && STATUS=0 || STATUS=$?
|
||||
[[ "$STATUS" -eq 0 ]] || fail_msg "populated node_modules/.bin: expected exit 0, got $STATUS ($OUTPUT)"
|
||||
case "$OUTPUT" in
|
||||
*"deps not installed"*) fail_msg "populated node_modules/.bin: unexpected sentinel fired: $OUTPUT" ;;
|
||||
*) ;;
|
||||
esac
|
||||
|
||||
# --- Scenario 5: non-JS/TS files are unaffected by the preflight (still
|
||||
# skipped before the deps check, regardless of node_modules state).
|
||||
NONJS_DIR="$TMP_DIR/nonjs"
|
||||
make_fixture_repo "$NONJS_DIR"
|
||||
echo "# doc" > "$NONJS_DIR/README.md"
|
||||
OUTPUT=$(cd "$NONJS_DIR" && run_hook "$NONJS_DIR/README.md" 2>&1) && STATUS=0 || STATUS=$?
|
||||
[[ "$STATUS" -eq 0 ]] || fail_msg "non-JS/TS file: expected exit 0 (skip), got $STATUS ($OUTPUT)"
|
||||
case "$OUTPUT" in
|
||||
*"deps not installed"*) fail_msg "non-JS/TS file: preflight incorrectly fired: $OUTPUT" ;;
|
||||
*) ;;
|
||||
esac
|
||||
|
||||
if [[ "$fail" -eq 0 ]]; then
|
||||
echo "deps-preflight regression passed (5/5 scenarios)"
|
||||
fi
|
||||
|
||||
exit "$fail"
|
||||
@@ -25,7 +25,7 @@
|
||||
"lint": "eslint src",
|
||||
"typecheck": "tsc --noEmit",
|
||||
"test": "vitest run --passWithNoTests && pnpm run test:framework-shell",
|
||||
"test:framework-shell": "python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh"
|
||||
"test:framework-shell": "python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh && bash framework/tools/git/test-pr-review-gitea-comment.sh"
|
||||
},
|
||||
"dependencies": {
|
||||
"@mosaicstack/brain": "workspace:*",
|
||||
|
||||
@@ -661,13 +661,27 @@ describe('whole mutator-class lease gate', () => {
|
||||
test('observer revocation and monotonic TTL expiry deny the next mutator', async () => {
|
||||
const { socket } = await startBroker();
|
||||
const sessionId = await register(socket);
|
||||
const pending = await beginVerification(socket, sessionId, 'claude', 1, 1);
|
||||
await promote(socket, sessionId, pending.receipt_challenge!);
|
||||
|
||||
// Establish the lease with a normal (non-racing) TTL first and prove it
|
||||
// authorizes. This "still valid" check is setup, not a TTL-expiry
|
||||
// assertion, so it must not share a lease with a 1-second TTL: on a
|
||||
// contended push-CI host, scheduling delay alone between promote() and
|
||||
// this authorize() call can consume that entire 1-second margin and
|
||||
// spuriously deny it (CI#1945). Using a generous TTL here removes that
|
||||
// real-time race without touching lease-gate security semantics.
|
||||
const pending = await beginVerification(socket, sessionId, 'claude');
|
||||
await promote(socket, sessionId, pending.receipt_challenge!);
|
||||
expect(await authorize(socket, sessionId, 'claude', 'Bash')).toMatchObject({
|
||||
ok: true,
|
||||
decision: 'allow',
|
||||
});
|
||||
|
||||
// A dedicated, isolated short-TTL lease drives the deliberate monotonic
|
||||
// expiry demonstration below. It is never used for anything but the
|
||||
// wait-then-expire assertion, so there is no setup work racing its
|
||||
// 1-second window.
|
||||
const shortLived = await beginVerification(socket, sessionId, 'claude', 1, 1, 2);
|
||||
await promote(socket, sessionId, shortLived.receipt_challenge!);
|
||||
await new Promise((resolve) => setTimeout(resolve, 1_100));
|
||||
expect(await authorize(socket, sessionId, 'claude', 'Bash')).toMatchObject({
|
||||
ok: false,
|
||||
@@ -675,7 +689,7 @@ describe('whole mutator-class lease gate', () => {
|
||||
decision: 'deny',
|
||||
});
|
||||
|
||||
const refreshed = await beginVerification(socket, sessionId, 'claude', 1, 300, 2);
|
||||
const refreshed = await beginVerification(socket, sessionId, 'claude', 1, 300, 3);
|
||||
await promote(socket, sessionId, refreshed.receipt_challenge!);
|
||||
expect(
|
||||
await request(socket, {
|
||||
|
||||
@@ -217,12 +217,22 @@ git fetch origin
|
||||
mkdir -p ~/src/${projectName}-worktrees
|
||||
git worktree add ~/src/${projectName}-worktrees/<task-slug> -b <branch-name> origin/main
|
||||
cd ~/src/${projectName}-worktrees/<task-slug>
|
||||
pnpm install --frozen-lockfile --prefer-offline
|
||||
# ... all work happens here ...
|
||||
git push origin <branch-name>
|
||||
cd ~/src/${projectName} && git worktree remove ~/src/${projectName}-worktrees/<task-slug>
|
||||
\`\`\`
|
||||
|
||||
Worktrees path: \`~/src/<repo>-worktrees/<task-slug>\` — NEVER use /tmp.`);
|
||||
Worktrees path: \`~/src/<repo>-worktrees/<task-slug>\` — NEVER use /tmp.
|
||||
|
||||
\`pnpm install --frozen-lockfile --prefer-offline\` MUST run immediately after
|
||||
\`git worktree add\`/\`cd\`, BEFORE any gate (\`pnpm test\`/\`lint\`/\`typecheck\`/\`format:check\`)
|
||||
is invoked. pnpm workspaces do NOT share \`node_modules\` across separate git
|
||||
worktrees — a fresh worktree has an empty \`node_modules/.bin\`, so every gate
|
||||
binary (\`tsc\`/\`eslint\`/\`prettier\`/\`vitest\`) fails \`sh: 1: <tool>: not found\`
|
||||
until deps are installed. That failure is indistinguishable from a real
|
||||
test/lint failure — a false-red gate. Never skip this step and never reorder
|
||||
it after the first gate invocation.`);
|
||||
|
||||
// 6. Completion gates
|
||||
sections.push(`# Completion Gates — ENFORCED
|
||||
|
||||
50
skills/glpi-create/SKILL.md
Normal file
50
skills/glpi-create/SKILL.md
Normal file
@@ -0,0 +1,50 @@
|
||||
# Skill: glpi-create — Open a New GLPI Ticket
|
||||
|
||||
> Create a new GLPI helpdesk ticket. Mutates GLPI — confirm the details before running.
|
||||
|
||||
## When to use
|
||||
|
||||
- Logging a new incident or request that should live in the helpdesk queue.
|
||||
|
||||
## Required information
|
||||
|
||||
- **title** — short subject line.
|
||||
- **content** — description of the issue / request.
|
||||
|
||||
## Optional
|
||||
|
||||
- **priority** — `1`=VeryLow, `2`=Low, `3`=Medium (default), `4`=High, `5`=VeryHigh, `6`=Major.
|
||||
- **type** — `1`=Incident (default), `2`=Request.
|
||||
|
||||
## Command
|
||||
|
||||
Wraps the existing tooling:
|
||||
|
||||
```bash
|
||||
~/.config/mosaic/tools/glpi/ticket-create.sh \
|
||||
-t "<title>" \
|
||||
-c "<content>" \
|
||||
[-p <priority>] \
|
||||
[-y <type>] \
|
||||
[-f json]
|
||||
```
|
||||
|
||||
Example:
|
||||
|
||||
```bash
|
||||
~/.config/mosaic/tools/glpi/ticket-create.sh \
|
||||
-t "Paint-area camera install" \
|
||||
-c "Ordered 2 cameras for Paint and stock; schedule mounting + NVR config." \
|
||||
-p 3 -y 2
|
||||
```
|
||||
|
||||
## After creating
|
||||
|
||||
- Note the returned **ticket ID** — you'll need it for **[[glpi-followup]]** and
|
||||
**[[glpi-solve]]**.
|
||||
- If it should also be tracked as brain work, add a matching task (see the `add-task` skill).
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Confirm title/content/priority with the user before creating — a ticket is outward-facing.
|
||||
- Never echo GLPI tokens.
|
||||
56
skills/glpi-followup/SKILL.md
Normal file
56
skills/glpi-followup/SKILL.md
Normal file
@@ -0,0 +1,56 @@
|
||||
# Skill: glpi-followup — Add a Followup to a GLPI Ticket
|
||||
|
||||
> Post a followup (comment / progress note / resolution writeup) to a GLPI ticket.
|
||||
> This documents work but does **not** change the ticket status — to close a ticket
|
||||
> out, follow with **[[glpi-solve]]** to set status to Solved.
|
||||
|
||||
## When to use
|
||||
|
||||
- Recording progress, a decision, or a root-cause/resolution note on a ticket.
|
||||
- The documentation step that usually precedes closing a ticket out (`glpi-solve`).
|
||||
|
||||
## Critical quirk
|
||||
|
||||
Use the **top-level `/ITILFollowup` endpoint**, NOT `/Ticket/<id>/ITILFollowup`. The
|
||||
sub-resource path returns permission errors even with a Super-Admin profile.
|
||||
|
||||
## Procedure
|
||||
|
||||
### 1. Session + creds
|
||||
|
||||
```bash
|
||||
SESSION=$(~/.config/mosaic/tools/glpi/session-init.sh -q)
|
||||
source ~/.config/mosaic/tools/_lib/credentials.sh && load_credentials glpi
|
||||
```
|
||||
|
||||
### 2. Post the followup
|
||||
|
||||
```bash
|
||||
TICKET_ID=<id>
|
||||
CONTENT="<the followup text>"
|
||||
curl -sk -X POST "${GLPI_URL}/ITILFollowup" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" \
|
||||
-H "Session-Token: $SESSION" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "$(jq -n --argjson id "$TICKET_ID" --arg c "$CONTENT" \
|
||||
'{input:{itemtype:"Ticket", items_id:$id, content:$c}}')"
|
||||
```
|
||||
|
||||
Expect HTTP 201. Building the payload with `jq` keeps quotes/newlines in the content safe.
|
||||
|
||||
### 3. Long or multi-paragraph content
|
||||
|
||||
Write the note to a file first, then read it into the payload:
|
||||
|
||||
```bash
|
||||
curl -sk -X POST "${GLPI_URL}/ITILFollowup" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" -H "Session-Token: $SESSION" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "$(jq -n --argjson id "$TICKET_ID" --rawfile c /path/to/note.md \
|
||||
'{input:{itemtype:"Ticket", items_id:$id, content:$c}}')"
|
||||
```
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Never echo the GLPI app/user/session tokens.
|
||||
- A followup alone leaves the ticket open. If the work is done, run **[[glpi-solve]]** next.
|
||||
57
skills/glpi-list/SKILL.md
Normal file
57
skills/glpi-list/SKILL.md
Normal file
@@ -0,0 +1,57 @@
|
||||
# Skill: glpi-list — Query GLPI Tickets
|
||||
|
||||
> Quick lookups of GLPI helpdesk tickets by status or recency. Read-only.
|
||||
|
||||
## When to use
|
||||
|
||||
- "What tickets are open / pending?" · "Show recent tickets" · finding a ticket ID
|
||||
before running **[[glpi-followup]]** or **[[glpi-solve]]**.
|
||||
|
||||
## Command
|
||||
|
||||
Wraps the existing tooling:
|
||||
|
||||
```bash
|
||||
GLPI=~/.config/mosaic/tools/glpi
|
||||
|
||||
# Most recent tickets (default 50, newest first)
|
||||
"$GLPI/ticket-list.sh"
|
||||
|
||||
# Filter by status: new | processing | pending | solved | closed
|
||||
"$GLPI/ticket-list.sh" -s pending
|
||||
|
||||
# JSON output (for parsing / piping to jq) and a custom limit
|
||||
"$GLPI/ticket-list.sh" -s processing -f json -l 20
|
||||
```
|
||||
|
||||
Status IDs: 1 New · 2/3 Processing · 4 Pending · 5 Solved · 6 Closed.
|
||||
|
||||
## Details lookup for one ticket
|
||||
|
||||
When you have an ID and want the full record:
|
||||
|
||||
```bash
|
||||
SESSION=$(~/.config/mosaic/tools/glpi/session-init.sh -q)
|
||||
source ~/.config/mosaic/tools/_lib/credentials.sh && load_credentials glpi
|
||||
curl -sk "${GLPI_URL}/Ticket/<id>?expand_dropdowns=true" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" -H "Session-Token: $SESSION" \
|
||||
| jq '{id, name, status, date, date_mod}'
|
||||
|
||||
# Followups on a ticket
|
||||
curl -sk "${GLPI_URL}/Ticket/<id>/ITILFollowup" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" -H "Session-Token: $SESSION" \
|
||||
| jq '.[] | {date, content}'
|
||||
```
|
||||
|
||||
(Reading followups via the sub-resource is fine — only _creating_ them requires the
|
||||
top-level `/ITILFollowup` endpoint. See **[[glpi-followup]]**.)
|
||||
|
||||
## Present to user
|
||||
|
||||
Group by status, one line per ticket: `#<id> · <title> · <status> · <last-modified>`.
|
||||
Use neutral phrasing — no "OVERDUE"/"URGENT".
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Read-only. Never echo GLPI tokens.
|
||||
- To sync tickets into brain data instead, use `python tools/sync_glpi.py` (not this skill).
|
||||
96
skills/glpi-solve/SKILL.md
Normal file
96
skills/glpi-solve/SKILL.md
Normal file
@@ -0,0 +1,96 @@
|
||||
# Skill: glpi-solve — Close Out a GLPI Ticket
|
||||
|
||||
> Properly close out a completed GLPI helpdesk ticket. Completing the work is not
|
||||
> enough — the ticket **status must be set to "Solved"**, which is what triggers
|
||||
> GLPI's config-driven auto-close. Posting a resolution followup documents the work
|
||||
> but does **not** change status, so a ticket left at Solved-less status stays open.
|
||||
|
||||
## When to use
|
||||
|
||||
- Any time work on a GLPI ticket is finished and it should be closed out.
|
||||
- After posting a root-cause / resolution writeup as an `/ITILFollowup`.
|
||||
- During a cleanup sweep of tickets that are done in reality but still open in GLPI.
|
||||
|
||||
## The rule (from an operator, 2026-07-20)
|
||||
|
||||
**"Solved" is the correct terminal state to set — not "Closed."** GLPI is configured
|
||||
to auto-close Solved tickets after its delay. If you only post a followup and never set
|
||||
status, the ticket sits open (this bit us on a real incident where resolution followups
|
||||
were posted but status was never advanced, leaving tickets open, which the operator had
|
||||
to mark Solved by hand).
|
||||
|
||||
Close-out = **followup (optional but preferred) + set status to Solved.**
|
||||
|
||||
## GLPI status IDs
|
||||
|
||||
| ID | Status | |
|
||||
| ----- | --------------------- | -------------------------------------------- |
|
||||
| 1 | New | |
|
||||
| 2 | Processing (assigned) | |
|
||||
| 3 | Processing (planned) | |
|
||||
| 4 | Pending / Waiting | |
|
||||
| **5** | **Solved** | ← set this on close-out |
|
||||
| 6 | Closed | ← happens automatically; do not set manually |
|
||||
|
||||
## Procedure
|
||||
|
||||
### 1. Get a session token
|
||||
|
||||
```bash
|
||||
SESSION=$(~/.config/mosaic/tools/glpi/session-init.sh -q)
|
||||
source ~/.config/mosaic/tools/_lib/credentials.sh && load_credentials glpi
|
||||
```
|
||||
|
||||
### 2. (Preferred) Post the resolution followup
|
||||
|
||||
Use the **top-level `/ITILFollowup` endpoint** — the `/Ticket/<id>/ITILFollowup`
|
||||
sub-resource returns permission errors even as Super-Admin (known GLPI quirk).
|
||||
|
||||
```bash
|
||||
TICKET_ID=<id>
|
||||
curl -sk -X POST "${GLPI_URL}/ITILFollowup" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" \
|
||||
-H "Session-Token: $SESSION" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "{\"input\":{\"itemtype\":\"Ticket\",\"items_id\":${TICKET_ID},\"content\":\"<resolution summary>\"}}"
|
||||
```
|
||||
|
||||
### 3. Set status to Solved (the step that actually closes it out)
|
||||
|
||||
```bash
|
||||
curl -sk -X PUT "${GLPI_URL}/Ticket/${TICKET_ID}" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" \
|
||||
-H "Session-Token: $SESSION" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "{\"input\":{\"id\":${TICKET_ID},\"status\":5}}"
|
||||
```
|
||||
|
||||
Expect HTTP 200/201. GLPI will auto-close it later per its config — leave status at 5.
|
||||
|
||||
### 4. Verify
|
||||
|
||||
```bash
|
||||
curl -sk "${GLPI_URL}/Ticket/${TICKET_ID}?expand_dropdowns=true" \
|
||||
-H "App-Token: $GLPI_APP_TOKEN" -H "Session-Token: $SESSION" \
|
||||
| jq '{id, name, status}'
|
||||
```
|
||||
|
||||
`status` should read `Solved` (or `5`).
|
||||
|
||||
## Optional: sweep for done-but-open tickets
|
||||
|
||||
List tickets still open (New/Processing/Pending) to spot ones whose work is actually
|
||||
finished but were never marked Solved:
|
||||
|
||||
```bash
|
||||
~/.config/mosaic/tools/glpi/ticket-list.sh -s processing -f table
|
||||
~/.config/mosaic/tools/glpi/ticket-list.sh -s pending -f table
|
||||
```
|
||||
|
||||
Review each; for any that are genuinely resolved, run steps 2–3.
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Read-only until you intend to close — confirm the ticket is actually done first.
|
||||
- Never echo the GLPI app/user/session tokens.
|
||||
- Set **Solved (5)**, never Closed (6) — auto-close owns that transition.
|
||||
62
skills/glpi-sweep/SKILL.md
Normal file
62
skills/glpi-sweep/SKILL.md
Normal file
@@ -0,0 +1,62 @@
|
||||
# Skill: glpi-sweep — Find Done-But-Open Tickets
|
||||
|
||||
> Read-only sweep for tickets that are finished in reality but still sitting open in
|
||||
> GLPI (never moved to Solved). Surfaces the exact miss an operator caught on 2026-07-20
|
||||
> (a real incident where an affected ticket had resolution followups posted but was left
|
||||
> open). For each one that's genuinely done, close it out with **[[glpi-solve]]**.
|
||||
|
||||
## When to use
|
||||
|
||||
- Periodic hygiene pass (e.g. before a weekly update or month-end).
|
||||
- After a burst of ticket work, to catch any you resolved-in-followup but never Solved.
|
||||
|
||||
## Why this exists
|
||||
|
||||
Posting an `/ITILFollowup` documents work but does **not** change status. Tickets only
|
||||
auto-close once set to **Solved (status 5)**. Anything left at New/Processing/Pending
|
||||
stays open indefinitely. This sweep finds those.
|
||||
|
||||
## Procedure
|
||||
|
||||
### 1. List still-open tickets by status
|
||||
|
||||
```bash
|
||||
GLPI=~/.config/mosaic/tools/glpi
|
||||
"$GLPI/ticket-list.sh" -s new -f table
|
||||
"$GLPI/ticket-list.sh" -s processing -f table
|
||||
"$GLPI/ticket-list.sh" -s pending -f table
|
||||
```
|
||||
|
||||
(GLPI status IDs: 1 New · 2/3 Processing · 4 Pending · 5 Solved · 6 Closed.)
|
||||
|
||||
### 2. Triage
|
||||
|
||||
For each open ticket, judge whether the underlying work is actually finished — check
|
||||
its latest followups and cross-reference brain tasks / recent work. Read-only here;
|
||||
change nothing yet.
|
||||
|
||||
Reasonable "probably done" signals:
|
||||
|
||||
- A resolution/root-cause followup already posted, but status never advanced.
|
||||
- The related brain task is `done`, or the fix shipped and was confirmed.
|
||||
- Requester confirmed resolution but the ticket was never Solved.
|
||||
|
||||
### 3. Present the candidates
|
||||
|
||||
List them for review before touching anything — never bulk-solve blindly:
|
||||
|
||||
```
|
||||
Open tickets that look resolved:
|
||||
- #<id> "<title>" — <why it looks done> → glpi-solve?
|
||||
```
|
||||
|
||||
### 4. Close out the confirmed ones
|
||||
|
||||
For each ticket the user (or clear evidence) confirms is done, run **[[glpi-solve]]**
|
||||
(optionally **[[glpi-followup]]** first if a closing note is warranted).
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Read-only until a ticket is confirmed done — do not auto-solve on a guess.
|
||||
- Never echo GLPI tokens.
|
||||
- Set **Solved (5)**, never Closed (6) — GLPI auto-close owns that transition.
|
||||
Reference in New Issue
Block a user