Compare commits

...
Author SHA1 Message Date
fred 62321700a3 fix(git): review follow-ups for #1357 (S1, S2, indent)
ci/woodpecker/pr/ci Pipeline was successful
S1: get_gitea_login_for_repo_override() now distinguishes "tea is not
installed" from "no tea login named X exists", mirroring the host path.
The old message diagnosed a cause that was never checked and pointed at
seat-logins.sh, which cannot run without tea. Branch 6 in
test-gitea-login-resolution.sh pins it (tea removed from PATH); reverting
the fix fails that branch.

S2: issue-list/pr-list/pr-view override-path error now points at the
lines above for the cause instead of suggesting a default tea login.

verify-release.mjs: indent of the #1356 test line fixed (cosmetic).
2026-08-21 18:12:12 -05:00
fred 341be60723 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
2026-08-21 18:06:06 -05:00
9 changed files with 247 additions and 12 deletions
+4
View File
@@ -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 —
@@ -468,6 +468,14 @@ get_gitea_login_for_repo_override() {
echo "$canon"
return 0
fi
# Same split as the host path above (#1357 S1): a missing tea binary
# is not a missing login, and the "create it with" advice cannot be
# followed without tea.
if ! command -v tea >/dev/null 2>&1; then
echo "Error: git identity '$ident' (via $ident_src) requested for owner '${owner%%/*}', but tea is not installed," >&2
echo " so no login can be resolved. Refusing to guess an identity." >&2
return 1
fi
echo "Error: git identity '$ident' (via $ident_src) has no tea login '$canon' for owner '${owner%%/*}'." >&2
echo " Create it with: ~/.config/mosaic/tools/fleet/seat-logins.sh --apply --seat $ident" >&2
return 1
@@ -100,7 +100,7 @@ case "$PLATFORM" in
gitea)
if [[ -n "$REPO_OVERRIDE" ]]; then
GITEA_LOGIN_NAME=$(get_gitea_login_for_repo_override "$REPO_OVERRIDE") || {
echo "Error: Could not resolve Gitea login for --repo override. Set GITEA_LOGIN or configure a default tea login." >&2
echo "Error: could not resolve a Gitea login for the --repo override (the lines above say why). Set GITEA_LOGIN to name one explicitly." >&2
exit 1
}
else
@@ -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
@@ -95,7 +95,7 @@ case "$PLATFORM" in
gitea)
if [[ -n "$REPO_OVERRIDE" ]]; then
GITEA_LOGIN_NAME=$(get_gitea_login_for_repo_override "$REPO_OVERRIDE") || {
echo "Error: Could not resolve Gitea login for --repo override. Set GITEA_LOGIN or configure a default tea login." >&2
echo "Error: could not resolve a Gitea login for the --repo override (the lines above say why). Set GITEA_LOGIN to name one explicitly." >&2
exit 1
}
else
@@ -60,7 +60,7 @@ if [[ "$PLATFORM" == "github" ]]; then
elif [[ "$PLATFORM" == "gitea" ]]; then
if [[ -n "$REPO_OVERRIDE" ]]; then
GITEA_LOGIN_NAME=$(get_gitea_login_for_repo_override "$REPO_OVERRIDE") || {
echo "Error: Could not resolve Gitea login for --repo override. Set GITEA_LOGIN or configure a default tea login." >&2
echo "Error: could not resolve a Gitea login for the --repo override (the lines above say why). Set GITEA_LOGIN to name one explicitly." >&2
exit 1
}
else
@@ -463,6 +463,34 @@ if [[ "$override_explicit" != "mosaicstack" ]]; then
exit 1
fi
# Branch 6 (#1357 S1): with tea ABSENT from PATH, the override path must say tea is
# missing, not "no tea login named X exists" (a cause that was never checked) and
# not the seat-logins.sh advice, which cannot be followed without tea.
NOTEA_BIN="$WORK_DIR/notea-bin"; mkdir -p "$NOTEA_BIN"
for t in bash git python3 sed grep cat mktemp dirname basename readlink env sort head tr cut; do
_p="$(command -v "$t" 2>/dev/null || true)"; [[ -n "$_p" ]] && ln -sf "$_p" "$NOTEA_BIN/$t"
done
override_notea_rc=0
override_notea_err=$(cd "$REPO_DIR" && env -u GITEA_LOGIN \
PATH="$NOTEA_BIN" HOME="$HOME_DIR" MOSAIC_GIT_IDENTITY=testseat \
bash -c '
command -v tea >/dev/null 2>&1 && { echo "SETUP: tea still on PATH"; exit 99; }
source "'"$SCRIPT_DIR"'/detect-platform.sh"
get_gitea_login_for_repo_override mosaicstack/stack
' 2>&1 >/dev/null) || override_notea_rc=$?
if [[ "$override_notea_rc" != 1 ]]; then
echo "Expected --repo override path to fail (rc=1) with tea absent; got rc=$override_notea_rc: $override_notea_err" >&2
exit 1
fi
if ! grep -q 'tea is not installed' <<<"$override_notea_err"; then
echo "Expected --repo override path to name tea as absent; got: $override_notea_err" >&2
exit 1
fi
if grep -q 'has no tea login\|seat-logins.sh' <<<"$override_notea_err"; then
echo "Override path diagnosed a missing LOGIN while tea itself is absent: $override_notea_err" >&2
exit 1
fi
git -C "$REPO_DIR" remote set-url origin https://git.uscllc.com/USC/uconnect.git
@@ -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"
+2 -1
View File
@@ -61,7 +61,8 @@ export const STAGES = [
'bash packages/mosaic/framework/tools/quality/scripts/check-tools-index.sh --self-test',
'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-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',
],