fix(git-tools): issue-view shows comment bodies and names the real tea failure (#1357)
Four defects in issue-view.sh, each pinned by the new hermetic suite
test-issue-view-comments.sh (mock tea + curl, sandboxed repo):
F1 tea exits 1 in any repo with extensions.worktreeconfig=true. The wrapper
now names that as a git-config condition and falls back to the API.
F2 The API fallback dumped raw issue JSON, which carries only a comment
COUNT. It now fetches /comments and renders issue + comment bodies.
F3 The tea path never passed --comments, so comment bodies were never shown
non-interactively. It now does.
F4 Every tea failure printed the REVOKED OR STALE TOKEN note. The wrapper now
relays tea's own error line and only hints at credentials when tea did.
The suite joins ci.yml and the verify-release canonical list (mirror test).
Closes #1357
This commit is contained in:
fred
@@ -96,6 +96,10 @@ steps:
|
||||
# fail-closed: a seat whose login is missing gets a named error, never a
|
||||
# borrowed identity. Joins CI directly; its #1007 exclusion is burned down.
|
||||
- bash packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh
|
||||
# Hermetic regression for issue-view.sh (#1357): mock tea/curl, sandboxed
|
||||
# repo. Pins that comment BODIES render on both paths and that a tea
|
||||
# failure is named as what it was (git-config vs credential).
|
||||
- bash packages/mosaic/framework/tools/git/test-issue-view-comments.sh
|
||||
# Hermetic behavioural regression for the PreToolUse wrapper guard: proves
|
||||
# it still blocks the three mistakes AND still lets reads, unwrapped
|
||||
# endpoints and ordinary commands through. Both directions are asserted —
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
#!/bin/bash
|
||||
# issue-view.sh - View issue details on GitHub or Gitea
|
||||
# issue-view.sh - View issue details, including comments, on GitHub or Gitea
|
||||
# Usage: issue-view.sh -i <issue_number>
|
||||
|
||||
set -e
|
||||
@@ -28,11 +28,47 @@ gitea_issue_view_api() {
|
||||
}
|
||||
|
||||
url="https://${host}/api/v1/repos/${repo}/issues/${ISSUE_NUMBER}"
|
||||
if command -v python3 >/dev/null 2>&1; then
|
||||
curl -fsS -H "User-Agent: curl/8" -H "Authorization: token ${token}" "$url" | python3 -m json.tool
|
||||
else
|
||||
curl -fsS -H "User-Agent: curl/8" -H "Authorization: token ${token}" "$url"
|
||||
local -a curl_args=(-fsS -H "User-Agent: curl/8" -H "Authorization: token ${token}")
|
||||
if ! command -v python3 >/dev/null 2>&1; then
|
||||
# No renderer: raw JSON is all this path can give. Comments are a
|
||||
# second resource, so fetch them too rather than only the count.
|
||||
curl "${curl_args[@]}" "$url"
|
||||
curl "${curl_args[@]}" "${url}/comments"
|
||||
return
|
||||
fi
|
||||
# Render issue + comments as text (#1357 F2). The old fallback dumped the
|
||||
# issue JSON, which carries only a comment COUNT, so every comment body was
|
||||
# invisible on this path and the wrapper could never show what
|
||||
# `tea issues --comments` shows.
|
||||
{
|
||||
curl "${curl_args[@]}" "$url"
|
||||
echo
|
||||
echo "__MOSAIC_COMMENTS__"
|
||||
curl "${curl_args[@]}" "${url}/comments"
|
||||
} | python3 -c '
|
||||
import json, sys
|
||||
raw = sys.stdin.read()
|
||||
issue_raw, _, comments_raw = raw.partition("__MOSAIC_COMMENTS__")
|
||||
issue = json.loads(issue_raw)
|
||||
comments = json.loads(comments_raw) if comments_raw.strip() else []
|
||||
print("#%s %s" % (issue["number"], issue["title"]))
|
||||
print("State: %s Author: %s Created: %s" % (issue["state"], issue["user"]["login"], issue["created_at"]))
|
||||
labels = ", ".join(l["name"] for l in issue.get("labels") or [])
|
||||
if labels:
|
||||
print("Labels: " + labels)
|
||||
if issue.get("milestone"):
|
||||
print("Milestone: " + issue["milestone"]["title"])
|
||||
print("URL: " + issue["html_url"])
|
||||
print()
|
||||
print(issue.get("body") or "(no body)")
|
||||
if comments:
|
||||
print()
|
||||
print("--- Comments (%d) ---" % len(comments))
|
||||
for c in comments:
|
||||
print()
|
||||
print("[%s at %s]" % (c["user"]["login"], c["created_at"]))
|
||||
print(c.get("body") or "")
|
||||
'
|
||||
}
|
||||
|
||||
while [[ $# -gt 0 ]]; do
|
||||
@@ -46,6 +82,8 @@ while [[ $# -gt 0 ]]; do
|
||||
echo ""
|
||||
echo "Options:"
|
||||
echo " -i, --issue Issue number (required)"
|
||||
echo ""
|
||||
echo "Comments are always included (tea --comments / Gitea API /comments)."
|
||||
echo " -h, --help Show this help"
|
||||
exit 0
|
||||
;;
|
||||
@@ -67,11 +105,30 @@ if [[ "$PLATFORM" == "github" ]]; then
|
||||
gh issue view "$ISSUE_NUMBER"
|
||||
elif [[ "$PLATFORM" == "gitea" ]]; then
|
||||
if command -v tea >/dev/null 2>&1; then
|
||||
if tea issue "$ISSUE_NUMBER" $(get_gitea_repo_args); then
|
||||
# --comments is what makes tea print the comment bodies (#1357 F3).
|
||||
# Without it tea prompts for them interactively, which in a
|
||||
# non-interactive wrapper means they are silently never shown.
|
||||
tea_err=$(mktemp)
|
||||
if tea issue "$ISSUE_NUMBER" $(get_gitea_repo_args) --comments 2>"$tea_err"; then
|
||||
rm -f "$tea_err"
|
||||
exit 0
|
||||
fi
|
||||
echo "Warning: tea issue view failed, trying Gitea API fallback..." >&2
|
||||
{ declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist; } || true
|
||||
# Name the cause tea actually reported, not a guessed one (#1357 F1/F4).
|
||||
# tea reads the cwd's git config before honouring --repo; a repo with
|
||||
# extensions.worktreeconfig=true makes it exit 1 with a
|
||||
# repositoryformatversion error. That is a git-config condition, not a
|
||||
# credential one. The old path printed the REVOKED OR STALE TOKEN note
|
||||
# here unconditionally, which sent readers to rotate a token that was fine.
|
||||
if grep -q 'repositoryformatversion' "$tea_err"; then
|
||||
echo "Warning: tea cannot read this repo's git config (extensions.worktreeconfig); not a credential problem. Using Gitea API fallback." >&2
|
||||
elif grep -q 'user does not exist' "$tea_err"; then
|
||||
echo "Warning: tea issue view failed, trying Gitea API fallback..." >&2
|
||||
{ declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist; } || true
|
||||
else
|
||||
echo "Warning: tea issue view failed, trying Gitea API fallback..." >&2
|
||||
fi
|
||||
sed 's/^/ tea: /' "$tea_err" >&2
|
||||
rm -f "$tea_err"
|
||||
fi
|
||||
gitea_issue_view_api
|
||||
else
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
#!/usr/bin/env bash
|
||||
# Regression: issue-view.sh must show comment BODIES, on both paths, and must name
|
||||
# the failure tea actually reported instead of guessing a credential cause (#1357).
|
||||
#
|
||||
# Four defects, each with its own case below:
|
||||
# F1 tea exits 1 in any repo with extensions.worktreeconfig=true; the wrapper must
|
||||
# say so (git-config condition) and fall back to the API.
|
||||
# F2 the API fallback dumped raw issue JSON, which carries only a comment COUNT.
|
||||
# F3 the tea path never passed --comments, so tea prompted (non-interactively: nothing).
|
||||
# F4 on ANY tea failure the wrapper printed the REVOKED OR STALE TOKEN note.
|
||||
#
|
||||
# Verification bar (plan §6): assert a real comment BODY appears, not a count and not
|
||||
# `grep -c comment` (that instrument matched the issue title and read inverted).
|
||||
#
|
||||
# Hermetic: mock tea and curl on PATH, sandboxed repo. Resolves no real credentials.
|
||||
set -euo pipefail
|
||||
|
||||
WORK_ROOT="${AGENT_WORK_ROOT:-${TMPDIR:-/tmp}}"
|
||||
SANDBOX="$WORK_ROOT/issue-view-comments-test-$$"
|
||||
MOCK_BIN="$SANDBOX/bin"; REPO_DIR="$SANDBOX/repo"; CALLS="$SANDBOX/calls.log"
|
||||
cleanup() { rm -rf "$SANDBOX"; }
|
||||
trap cleanup EXIT
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
TARGET="$SCRIPT_DIR/issue-view.sh"
|
||||
[ -f "$TARGET" ] || { echo "FAIL: issue-view.sh not found beside this test"; exit 1; }
|
||||
fail() { echo "FAIL: $*"; exit 1; }
|
||||
|
||||
mkdir -p "$MOCK_BIN" "$REPO_DIR" || fail "setup: cannot create sandbox under $WORK_ROOT"
|
||||
: > "$CALLS" || fail "setup: cannot write calls log at $CALLS"
|
||||
cd "$REPO_DIR" || fail "setup: cannot cd into $REPO_DIR"
|
||||
git init -q || fail "setup: git init failed"
|
||||
git remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git || fail "setup: git remote add failed"
|
||||
export PATH="$MOCK_BIN:$PATH" CALLS
|
||||
export GITEA_URL="https://git.mosaicstack.dev"
|
||||
export GITEA_TOKEN="redacted-test-token"
|
||||
# The identity ladder must not reach for this seat's real login; the mock tea below
|
||||
# defines the only login that exists in this sandbox.
|
||||
unset MOSAIC_GIT_IDENTITY
|
||||
# No fleet in the sandbox: on a host that runs one, get_gitea_token fails closed for an
|
||||
# identity-less caller (by design), which would make this test measure the host, not
|
||||
# the wrapper. An empty brain home makes the sandbox the same on every host.
|
||||
export MOSAIC_BRAIN_HOME="$SANDBOX/brain"
|
||||
mkdir -p "$MOSAIC_BRAIN_HOME" || fail "setup: cannot create sandbox brain home"
|
||||
|
||||
# Distinctive strings: a comment body that appears nowhere else, and an issue title
|
||||
# that contains the word "comment" so a count-of-the-word instrument would misread.
|
||||
BODY_MARKER="zebra-quill-comment-body-7731"
|
||||
ISSUE_TITLE="wrapper never shows a comment"
|
||||
|
||||
# --- mock curl: serves the issue and its comments; logs every call --------------
|
||||
cat > "$MOCK_BIN/curl" <<EOF
|
||||
#!/bin/bash
|
||||
url=""
|
||||
while [ \$# -gt 0 ]; do
|
||||
case "\$1" in
|
||||
http*) url="\$1"; shift ;;
|
||||
*) shift ;;
|
||||
esac
|
||||
done
|
||||
printf 'curl %s\n' "\$url" >> "$CALLS"
|
||||
case "\$url" in
|
||||
*/issues/77/comments)
|
||||
if [ "\${MOCK_NO_COMMENTS:-}" = "1" ]; then echo '[]'; else
|
||||
echo '[{"id":1,"user":{"login":"alice"},"created_at":"2026-08-21T00:00:00Z","body":"$BODY_MARKER"}]'; fi ;;
|
||||
*/issues/77)
|
||||
echo '{"number":77,"title":"$ISSUE_TITLE","state":"open","user":{"login":"bob"},"created_at":"2026-08-21T00:00:00Z","labels":[],"milestone":null,"html_url":"https://git.mosaicstack.dev/mosaicstack/stack/issues/77","body":"issue body","comments":1}' ;;
|
||||
*) echo '{}' ;;
|
||||
esac
|
||||
exit 0
|
||||
EOF
|
||||
chmod +x "$MOCK_BIN/curl"
|
||||
|
||||
# --- mock tea: MOCK_TEA_MODE selects the behaviour under test --------------------
|
||||
# ok : prints the issue, and the comment body ONLY when --comments is passed (F3)
|
||||
# wtconfig : exits 1 with the repositoryformatversion error (F1/F4)
|
||||
# badtoken : exits 1 with tea's credential error (F4 control: credential wording allowed)
|
||||
cat > "$MOCK_BIN/tea" <<EOF
|
||||
#!/bin/bash
|
||||
printf 'tea %s\n' "\$*" >> "$CALLS"
|
||||
if [[ "\$*" == *"login list"* ]]; then
|
||||
echo '[{"name":"git.mosaicstack.dev","url":"https://git.mosaicstack.dev"}]'; exit 0
|
||||
fi
|
||||
case "\${MOCK_TEA_MODE:-ok}" in
|
||||
wtconfig) echo 'Error: core.repositoryformatversion does not support extension: worktreeconfig' >&2; exit 1 ;;
|
||||
badtoken) echo 'Failed to create Gitea client: invalid username, password or token' >&2; exit 1 ;;
|
||||
esac
|
||||
echo "# #77 $ISSUE_TITLE (open)"
|
||||
echo "issue body"
|
||||
if [[ "\$*" == *"--comments"* ]]; then echo "$BODY_MARKER"; fi
|
||||
exit 0
|
||||
EOF
|
||||
chmod +x "$MOCK_BIN/tea"
|
||||
|
||||
[ "$(command -v tea)" = "$MOCK_BIN/tea" ] || fail "setup: tea does not resolve inside the sandbox"
|
||||
[ "$(command -v curl)" = "$MOCK_BIN/curl" ] || fail "setup: curl does not resolve inside the sandbox"
|
||||
|
||||
run() { bash "$TARGET" -i 77 >"$SANDBOX/out" 2>"$SANDBOX/err"; echo $?; }
|
||||
|
||||
# F3: tea path shows the comment body, which the mock emits only under --comments.
|
||||
: > "$CALLS"
|
||||
rc=$(MOCK_TEA_MODE=ok run)
|
||||
[ "$rc" = 0 ] || fail "F3: expected rc=0 on the tea path, got $rc: $(cat "$SANDBOX/err")"
|
||||
grep -q -- '--comments' "$CALLS" || fail "F3: tea was not invoked with --comments: $(cat "$CALLS")"
|
||||
grep -q "$BODY_MARKER" "$SANDBOX/out" || fail "F3: comment body missing from tea-path output"
|
||||
if grep -q '^curl' "$CALLS"; then fail "F3: tea path succeeded but the API fallback ran anyway"; fi
|
||||
|
||||
# F1 + F2: worktreeconfig failure is named as a git-config condition, falls back to
|
||||
# the API, and the API rendering includes the comment BODY.
|
||||
: > "$CALLS"
|
||||
rc=$(MOCK_TEA_MODE=wtconfig run)
|
||||
[ "$rc" = 0 ] || fail "F1: expected rc=0 via API fallback, got $rc: $(cat "$SANDBOX/err")"
|
||||
grep -q 'worktreeconfig' "$SANDBOX/err" || fail "F1: stderr does not name the worktreeconfig cause: $(cat "$SANDBOX/err")"
|
||||
grep -q 'not a credential problem' "$SANDBOX/err" || fail "F1: stderr does not rule out the credential cause"
|
||||
grep -q 'issues/77/comments' "$CALLS" || fail "F2: API fallback never fetched /comments: $(cat "$CALLS")"
|
||||
grep -q "$BODY_MARKER" "$SANDBOX/out" || fail "F2: comment body missing from API-path output"
|
||||
grep -q "$ISSUE_TITLE" "$SANDBOX/out" || fail "F2: issue title missing from API-path output"
|
||||
if grep -q 'REVOKED OR STALE' "$SANDBOX/err"; then fail "F4: stale-token note printed for a git-config failure"; fi
|
||||
if grep -q '"comments": 1' "$SANDBOX/out"; then fail "F2: output is still raw JSON (comment count instead of bodies)"; fi
|
||||
|
||||
# F4 control: a real credential error from tea may still carry the credential note,
|
||||
# and tea's own line must be relayed so the reader sees the actual cause.
|
||||
: > "$CALLS"
|
||||
rc=$(MOCK_TEA_MODE=badtoken run)
|
||||
[ "$rc" = 0 ] || fail "F4 control: expected rc=0 via API fallback, got $rc"
|
||||
grep -q 'invalid username, password or token' "$SANDBOX/err" || fail "F4: tea's own error line was not relayed"
|
||||
if grep -q 'worktreeconfig' "$SANDBOX/err"; then fail "F4: git-config wording printed for a credential failure"; fi
|
||||
|
||||
# Negative control: an issue with no comments prints no comment section on the API
|
||||
# path. Without this, a renderer that always prints a section would pass F2.
|
||||
: > "$CALLS"
|
||||
rc=$(MOCK_TEA_MODE=wtconfig MOCK_NO_COMMENTS=1 run)
|
||||
[ "$rc" = 0 ] || fail "negative control: expected rc=0, got $rc"
|
||||
if grep -q -- '--- Comments' "$SANDBOX/out"; then fail "negative control: comment section printed for an issue with no comments"; fi
|
||||
if grep -q "$BODY_MARKER" "$SANDBOX/out"; then fail "negative control: a comment body appeared for an issue with no comments"; fi
|
||||
|
||||
echo "issue-view comments regression harness passed"
|
||||
@@ -62,6 +62,7 @@ export const STAGES = [
|
||||
'bash packages/mosaic/framework/tools/quality/scripts/check-tools-index.sh',
|
||||
'bash packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh',
|
||||
'bash packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh',
|
||||
'bash packages/mosaic/framework/tools/git/test-issue-view-comments.sh',
|
||||
'bash packages/mosaic/framework/tools/git/test-wrapper-guard.sh',
|
||||
'bash packages/mosaic/framework/tools/git/test-mosaic-worktree-large-repo.sh',
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user