From a27b7dc9beb980bad58f2c4b5c5f5adae59cbf84 Mon Sep 17 00:00:00 2001 From: marcie Date: Fri, 28 Aug 2026 16:45:07 -0500 Subject: [PATCH] framework tools/git: sync issue-comment R1/R4 + hermeticity hardening from brain Upstream of brain commits 08a00149 + 971586ef + the arm-6 sandbox fix (brain 5th commit of 2026-08-28 series): - R1: -b/--body is the canonical comment flag (matches issue-create, issue-edit, pr-create, pr-edit); -c/--comment stays a compatible alias. - R4: usage errors print to stderr and exit 2, distinct from provider, credential, and verification failures (exit 1). Value-less flags fail loudly (previously -c with no value died silently at rc=1 via set -e on the failed shift 2). gh failures on the GitHub path normalize to exit 1 (gh's own usage errors exit 2 and would collide with the reserved code). - Tests: new usage-contract suite (help rc, unknown/missing/value-less rc=2 on stderr, alias parse acceptance under a sandboxed runner, GitHub-path exit normalization with a stubbed gh, zero provider contact on parser failure); readback suite gains case 11 (full verified write via -b) and neutralizes seat-exported MOSAIC_GIT_IDENTITY / MOSAIC_BRAIN_HOME that escape the sandboxed HOME (documented HTTP 401 / fail-loud shapes). Driver: a fleet seat full-stopped on an issue-comment usage error because usage failures were indistinguishable from provider failures and the stop gate treated every wrapper failure as blocking. --- .../framework/tools/git/issue-comment.sh | 47 +++-- .../tools/git/test-issue-comment-readback.sh | 38 +++- .../git/test-issue-comment-usage-contract.sh | 165 ++++++++++++++++++ 3 files changed, 238 insertions(+), 12 deletions(-) create mode 100755 packages/mosaic/framework/tools/git/test-issue-comment-usage-contract.sh diff --git a/packages/mosaic/framework/tools/git/issue-comment.sh b/packages/mosaic/framework/tools/git/issue-comment.sh index 49b4b504..e5858b35 100755 --- a/packages/mosaic/framework/tools/git/issue-comment.sh +++ b/packages/mosaic/framework/tools/git/issue-comment.sh @@ -1,6 +1,7 @@ #!/bin/bash # issue-comment.sh - Add a comment to an issue on GitHub or Gitea -# Usage: issue-comment.sh -i -c [--login ] +# Usage: issue-comment.sh -i -b [--login ] +# (-c/--comment is a backward-compatible alias for -b/--body; R1, 2026-08-28) # # tea v0.11.1 defines no `comment` subcommand under `tea issue` (or `tea pr`); # the non-existent `tea issue comment ...` form does not error — tea silently @@ -32,45 +33,61 @@ ISSUE_NUMBER="" COMMENT="" LOGIN_OVERRIDE="" +# Usage-error contract (R4, 2026-08-28): usage errors print to STDERR and exit 2, +# distinct from provider, credential, and verification failures (exit 1), so a +# caller or stop gate can tell an invocation defect from a delivery blocker +# (CONSTITUTION gate 8 as amended; E2E-DELIVERY). +usage_error() { + echo "Error: $*" >&2 + echo "Usage: issue-comment.sh -i -b [--login ] (see --help)" >&2 + exit 2 +} + while [[ $# -gt 0 ]]; do case $1 in -i|--issue) + [[ $# -ge 2 ]] || usage_error "option $1 requires a value" ISSUE_NUMBER="$2" shift 2 ;; - -c|--comment) + -b|--body|-c|--comment) + # R1 (2026-08-28): --body is the canonical flag, matching + # issue-create/issue-edit/pr-create/pr-edit; -c/--comment stays a + # backward-compatible alias. + [[ $# -ge 2 ]] || usage_error "option $1 requires a value" COMMENT="$2" shift 2 ;; -l|--login) + [[ $# -ge 2 ]] || usage_error "option $1 requires a value" LOGIN_OVERRIDE="$2" shift 2 ;; -h|--help) - echo "Usage: issue-comment.sh -i -c [--login ]" + echo "Usage: issue-comment.sh -i -b [--login ]" echo "" echo "Options:" echo " -i, --issue Issue number (required)" - echo " -c, --comment Comment text (required)" + echo " -b, --body Comment text (required; canonical)" + echo " -c, --comment Alias for --body" echo " -l, --login Override the detected Gitea tea login for this call" echo " -h, --help Show this help" + echo "" + echo "Exit codes: 0 success; 2 usage error (stderr); 1 provider/credential/verification failure." exit 0 ;; *) - echo "Unknown option: $1" - exit 1 + usage_error "unknown option: $1" ;; esac done if [[ -z "$ISSUE_NUMBER" ]]; then - echo "Error: Issue number is required (-i)" - exit 1 + usage_error "issue number is required (-i/--issue)" fi if [[ -z "$COMMENT" ]]; then - echo "Error: Comment is required (-c)" - exit 1 + usage_error "comment is required (-b/--body, or the -c/--comment alias)" fi detect_platform >/dev/null @@ -340,7 +357,15 @@ PY } if [[ "$PLATFORM" == "github" ]]; then - gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" + # R4 exit-code contract: normalize provider failures to exit 1. gh's own + # usage errors exit 2, which would collide with this wrapper's reserved + # usage-error status if propagated raw (codex review of 08a00149). + gh_rc=0 + gh issue comment "$ISSUE_NUMBER" --body "$COMMENT" || gh_rc=$? + if [[ "$gh_rc" -ne 0 ]]; then + echo "Error: GitHub comment write failed (gh exit $gh_rc; provider/credential failure — usage errors are exit 2)" >&2 + exit 1 + fi echo "Added comment to GitHub issue #$ISSUE_NUMBER" elif [[ "$PLATFORM" == "gitea" ]]; then # A --login override selects a NAMED tea credential and is the only way to diff --git a/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh b/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh index 60037d33..754dcc60 100755 --- a/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh +++ b/packages/mosaic/framework/tools/git/test-issue-comment-readback.sh @@ -42,6 +42,8 @@ # 10. leaves NO temp files behind (POST/GET bodies + metadata) on either the # success or the failure path — nested function-scoped RETURN traps do not # clobber each other and every scratch file is removed on all exit paths. +# 11. accepts the canonical -b/--body flag exactly like the -c/--comment alias +# (R1, 2026-08-28): a full verified write via -b alone. set -euo pipefail @@ -409,11 +411,28 @@ run_comment() { seed_state "$mode" ( cd "$REPO_DIR" + # Provisioned seats export MOSAIC_GIT_IDENTITY and MOSAIC_BRAIN_HOME + # seat-wide (launcher), and both escape this harness's sandboxed HOME: + # detect-platform.sh consults MOSAIC_GIT_IDENTITY BEFORE the repo-local + # mosaic.gitIdentity pin, and resolves the brain home (whose + # fleet/agents presence arms the no-identity fail-loud branch) from + # MOSAIC_BRAIN_HOME before $HOME. Without these explicit empties the + # wrapper either resolves the REAL seat-slot token (stub curl rejects + # it: the documented HTTP 401) or fails loud before any request. + # Set-but-empty reads as unset to detect-platform's "${VAR:-}" forms. + # NOTE: keep this comment block ABOVE the assignment chain — a comment + # inside a backslash-continued prefix chain terminates the command and + # silently demotes every earlier assignment to an unexported subshell + # assignment (measured 2026-08-28: the wrapper then ran without + # MOSAIC_CREDENTIALS_FILE and the suite died at credential resolution + # with zero diagnostic output). PATH="$BIN_DIR:$PATH" \ TMPDIR="$TMP_SCRATCH" \ HOME="$HOME_DIR" \ XDG_CONFIG_HOME="$XDG_DIR" \ MOSAIC_CREDENTIALS_FILE="$CREDENTIALS_FILE" \ + MOSAIC_GIT_IDENTITY="" \ + MOSAIC_BRAIN_HOME="" \ ISSUE_COMMENT_TEA_LOG="$TEA_LOG" \ ISSUE_COMMENT_CURL_LOG="$CURL_LOG" \ ISSUE_COMMENT_CURL_ARGV_LOG="$CURL_ARGV_LOG" \ @@ -430,7 +449,7 @@ run_comment() { ISSUE_COMMENT_REPO_SLUG="$REPO_SLUG" \ ISSUE_COMMENT_API_BASE="$API_BASE" \ ISSUE_COMMENT_API_ROOT="$API_ROOT" \ - "$SCRIPT_DIR/issue-comment.sh" -i "$ISSUE_NUMBER" -c "$BODY" "$@" + "$SCRIPT_DIR/issue-comment.sh" -i "$ISSUE_NUMBER" "${BODY_FLAG:--c}" "$BODY" "$@" ) > "$OUTPUT_FILE" 2>&1 } @@ -614,4 +633,21 @@ done # issue_url (already exercised by Case 1's fresh-success), so the tightened check # is not rejecting genuine writes. +# Case 11 (R1, 2026-08-28): -b/--body is the canonical comment flag and must +# drive a full verified write exactly like the -c/--comment alias. BODY_FLAG +# swaps only the flag spelling; every assertion below is case 1's contract. +BODY_FLAG="-b" +run_comment fresh-success +grep -q 'Added and verified comment on Gitea issue #7 (comment ID 51)' "$OUTPUT_FILE" +grep -q "^POST $API_BASE/issues/7/comments$" "$CURL_LOG" +if grep -Eq '^comment |^issue comment ' "$TEA_LOG"; then + echo "FAIL: --body write went through tea instead of REST" >&2 + exit 1 +fi +grep -q "^GET $API_BASE/issues/comments/51$" "$CURL_LOG" +grep -q "^POST $API_BASE/issues/7/comments $ACTING_LOGIN$" "$AUTH_LOG" +assert_no_temp_leak "fresh-success-body-flag" +assert_token_not_in_argv "fresh-success-body-flag" +unset BODY_FLAG + echo "issue-comment.sh REST create + exact-id read-back regression passed" diff --git a/packages/mosaic/framework/tools/git/test-issue-comment-usage-contract.sh b/packages/mosaic/framework/tools/git/test-issue-comment-usage-contract.sh new file mode 100755 index 00000000..a1be441a --- /dev/null +++ b/packages/mosaic/framework/tools/git/test-issue-comment-usage-contract.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash +# Usage-error contract for issue-comment.sh (R1/R4 remediation, 2026-08-28). +# +# R4: usage errors print to STDERR and exit 2, distinct from provider, +# credential, and verification failures (exit 1), so a caller (or a stop gate) +# can tell an invocation defect from a delivery blocker. Before this contract +# the wrapper exited 1 for usage errors with messages on STDOUT, and a +# value-less flag (-c with no value) died SILENTLY at rc=1 because set -e +# killed the failed `shift 2`. That silent shape is what full-stopped a fleet +# seat: a caller could not distinguish "I invoked it wrong" from "delivery is +# blocked". +# +# R1: -b/--body is the canonical comment flag (matching issue-create, +# issue-edit, pr-create, pr-edit); -c/--comment remains a backward-compatible +# alias. +# +# Arms: +# 1. --help and -h exit 0 and print usage. +# 2. Unknown option exits 2 with the message on stderr. +# 3. Missing required -i exits 2 (stderr). +# 4. Missing required comment exits 2 (stderr). +# 5. A value-less flag (-i -b -c -l and long forms) exits 2 with a +# "requires a value" message on stderr (the former silent-death class). +# 6. -b and -c both pass parsing (the run then fails at platform detection +# in this non-repo fixture, nonzero and NOT 2), proving alias acceptance +# without any provider fixture. +# 7. No arm performs any provider request: PATH shims for gh/tea/curl +# record every invocation and the probe log must stay empty. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WORK_DIR="${MOSAIC_TEST_WORK_DIR:-$PWD/.mosaic-test-work/issue-comment-usage}" +BIN_DIR="$WORK_DIR/bin" +PROBE_LOG="$WORK_DIR/provider-probes.log" +OUT_FILE="$WORK_DIR/out.log" +ERR_FILE="$WORK_DIR/err.log" + +cleanup() { + rm -rf "$WORK_DIR" +} +trap cleanup EXIT + +mkdir -p "$BIN_DIR" +: > "$PROBE_LOG" + +# Provider shims: any invocation is recorded and fails the run at the end. +# Usage-error arms must exit during argument parsing, before detect_platform, +# so these prove "no provider request on parser failure". +for tool in gh tea curl; do + cat > "$BIN_DIR/$tool" <> "$PROBE_LOG" +# gh doubles as platform probe AND write path in arm 6b: probes exit 0; the +# comment write exits 2 (gh's own usage-error status) to prove the wrapper +# normalizes provider failures to exit 1 instead of propagating 2. +if [[ "\$1 \$2" == "issue comment" ]]; then exit 2; fi +exit 0 +STUB + chmod +x "$BIN_DIR/$tool" +done + +run_wrapper() { + ( cd "$WORK_DIR" && PATH="$BIN_DIR:$PATH" "$SCRIPT_DIR/issue-comment.sh" "$@" ) +} + +# Hermetic variant for parse-acceptance arms: neutralizes every identity/ +# credential source the wrapper consults (seat env vars, HOME, XDG tea config) +# so the arm fails at credential resolution in ANY cwd repo, never reading a +# real token or contacting a provider. Measured 2026-08-28: without this, the +# arm's outcome depended on incidental URL-resolution state (brain cwd died at +# URL-not-found; a stack worktree cwd resolved a configured URL, read the real +# seat token, and invoked the curl stub — the suite then failed its own +# no-provider-contact check, correctly). +run_wrapper_sandboxed() { + mkdir -p "$WORK_DIR/home" "$WORK_DIR/xdg" + ( + cd "$WORK_DIR" + PATH="$BIN_DIR:$PATH" HOME="$WORK_DIR/home" XDG_CONFIG_HOME="$WORK_DIR/xdg" \ + MOSAIC_GIT_IDENTITY="" MOSAIC_BRAIN_HOME="" \ + "$SCRIPT_DIR/issue-comment.sh" "$@" + ) +} + +fail() { + echo "FAIL: $*" >&2 + echo "--- stderr ---" >&2 + cat "$ERR_FILE" >&2 + exit 1 +} + +expect_rc() { # expect_rc + local want="$1" desc="$2" rc=0 + shift 2 + run_wrapper "$@" >"$OUT_FILE" 2>"$ERR_FILE" || rc=$? + [[ "$rc" -eq "$want" ]] || fail "$desc: rc=$rc, want $want" +} + +expect_stderr() { # expect_stderr + grep -q "$1" "$ERR_FILE" || fail "$desc: stderr missing '$1'" +} + +# 1. Help exits 0 and prints usage on stdout. +expect_rc 0 "--help exits 0" --help +grep -q "Usage: issue-comment.sh" "$OUT_FILE" || fail "--help did not print usage" +expect_rc 0 "-h exits 0" -h + +# 2. Unknown option: rc 2, message on stderr. +expect_rc 2 "unknown option exits 2" --bogus +expect_stderr "unknown option" "unknown option names itself on stderr" + +# 3. Missing required issue number: rc 2, stderr. +expect_rc 2 "missing -i exits 2" +expect_stderr "issue number is required" "missing -i message on stderr" + +# 4. Missing required comment: rc 2, stderr. +expect_rc 2 "missing comment exits 2" -i 5 +expect_stderr "comment is required" "missing comment message on stderr" + +# 5. Value-less flags: rc 2 with "requires a value" on stderr. The old parser +# died here silently (set -e on the failed shift 2). +for flag in -i -b -c -l --issue --body --comment --login; do + expect_rc 2 "value-less $flag exits 2" "$flag" + expect_stderr "requires a value" "value-less $flag message on stderr" +done + +# 6. Alias acceptance at parse level: both -b and -c carry a value past +# parsing; the wrapper then fails at platform detection (not a git repo) +# nonzero but NOT as a usage error (rc must not be 2). +for flag in -b -c; do + rc=0 + run_wrapper_sandboxed -i 5 "$flag" "some text" >"$OUT_FILE" 2>"$ERR_FILE" || rc=$? + [[ "$rc" -ne 0 ]] || fail "$flag arm unexpectedly succeeded in the sandbox" + [[ "$rc" -ne 2 ]] || fail "$flag arm misclassified credential failure as a usage error" +done + +# 6b. GitHub-path exit normalization (codex blocker on 08a00149): gh's own +# usage errors exit 2; the wrapper must NOT propagate that status (reserved +# for the wrapper's usage-error contract). With a github remote and a gh stub +# whose comment write exits 2, the wrapper must exit 1 with the normalized +# error on stderr. +GH_REPO="$WORK_DIR/repo-gh" +mkdir -p "$GH_REPO" +git -C "$GH_REPO" init -q +git -C "$GH_REPO" remote add origin https://github.com/acme/widgets.git +git -C "$GH_REPO" config mosaic.gitIdentity "" +rc=0 +( + cd "$GH_REPO" + PATH="$BIN_DIR:$PATH" MOSAIC_GIT_IDENTITY="" MOSAIC_BRAIN_HOME="" \ + "$SCRIPT_DIR/issue-comment.sh" -i 5 -b "text" >"$OUT_FILE" 2>"$ERR_FILE" +) || rc=$? +[[ "$rc" -eq 1 ]] || fail "GitHub path: gh exit 2 must normalize to wrapper exit 1 (got $rc)" +grep -q "GitHub comment write failed" "$ERR_FILE" || fail "GitHub path: normalized error missing from stderr" +grep -q "^gh issue comment" "$PROBE_LOG" || fail "GitHub path: gh write was not invoked" + +# 7. No provider contact from any usage-error arm (arm 6b's deliberate gh +# invocation is the only permitted entry in the probe log). +if grep -v '^gh issue comment' "$PROBE_LOG" | grep -q .; then + echo "FAIL: a parser-failure arm contacted a provider:" >&2 + grep -v '^gh issue comment' "$PROBE_LOG" >&2 + exit 1 +fi + +echo "issue-comment.sh usage-contract regression passed (R1/R4)"