From 24294d3b773d4413d1708940c632a7467be22072 Mon Sep 17 00:00:00 2001 From: fred Date: Sat, 22 Aug 2026 00:23:22 +0000 Subject: [PATCH] fix(git-tools): issue-view shows comment bodies and names the real tea failure (#1357) (#1365) Co-authored-by: fred --- .woodpecker/ci.yml | 4 + .../framework/tools/git/detect-platform.sh | 8 + .../mosaic/framework/tools/git/issue-list.sh | 2 +- .../mosaic/framework/tools/git/issue-view.sh | 73 +++++++++- .../mosaic/framework/tools/git/pr-list.sh | 2 +- .../mosaic/framework/tools/git/pr-view.sh | 2 +- .../tools/git/test-gitea-login-resolution.sh | 28 ++++ .../tools/git/test-issue-view-comments.sh | 137 ++++++++++++++++++ scripts/verify-release.mjs | 3 +- 9 files changed, 247 insertions(+), 12 deletions(-) create mode 100755 packages/mosaic/framework/tools/git/test-issue-view-comments.sh diff --git a/.woodpecker/ci.yml b/.woodpecker/ci.yml index 932172a1..561112ac 100644 --- a/.woodpecker/ci.yml +++ b/.woodpecker/ci.yml @@ -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 — diff --git a/packages/mosaic/framework/tools/git/detect-platform.sh b/packages/mosaic/framework/tools/git/detect-platform.sh index c293f2e7..8398757f 100755 --- a/packages/mosaic/framework/tools/git/detect-platform.sh +++ b/packages/mosaic/framework/tools/git/detect-platform.sh @@ -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 diff --git a/packages/mosaic/framework/tools/git/issue-list.sh b/packages/mosaic/framework/tools/git/issue-list.sh index 16ad1520..4e59d3d9 100755 --- a/packages/mosaic/framework/tools/git/issue-list.sh +++ b/packages/mosaic/framework/tools/git/issue-list.sh @@ -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 diff --git a/packages/mosaic/framework/tools/git/issue-view.sh b/packages/mosaic/framework/tools/git/issue-view.sh index 81c55965..da15bc10 100755 --- a/packages/mosaic/framework/tools/git/issue-view.sh +++ b/packages/mosaic/framework/tools/git/issue-view.sh @@ -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 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 diff --git a/packages/mosaic/framework/tools/git/pr-list.sh b/packages/mosaic/framework/tools/git/pr-list.sh index 9d4320ff..11eebeab 100755 --- a/packages/mosaic/framework/tools/git/pr-list.sh +++ b/packages/mosaic/framework/tools/git/pr-list.sh @@ -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 diff --git a/packages/mosaic/framework/tools/git/pr-view.sh b/packages/mosaic/framework/tools/git/pr-view.sh index dcc3dc66..72f53074 100755 --- a/packages/mosaic/framework/tools/git/pr-view.sh +++ b/packages/mosaic/framework/tools/git/pr-view.sh @@ -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 diff --git a/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh b/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh index 5c1a9b60..10595c7f 100755 --- a/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh +++ b/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh @@ -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 diff --git a/packages/mosaic/framework/tools/git/test-issue-view-comments.sh b/packages/mosaic/framework/tools/git/test-issue-view-comments.sh new file mode 100755 index 00000000..e676b9b8 --- /dev/null +++ b/packages/mosaic/framework/tools/git/test-issue-view-comments.sh @@ -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" <> "$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" <> "$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" diff --git a/scripts/verify-release.mjs b/scripts/verify-release.mjs index 1d83c92b..f35edf14 100644 --- a/scripts/verify-release.mjs +++ b/scripts/verify-release.mjs @@ -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', ],