fix: stop re-escaping non-ASCII in package.json when enumerating a test

CI step `format` failed (pipeline 2255, exit 1) -- not on the test, on formatting. I enumerated the new test by loading packages/mosaic/package.json with python's json module and writing it back. json.dumps defaults to ensure_ascii=True, so the em-dash in "description": "Mosaic agent framework — installation wizard and meta package" was rewritten as —. Valid JSON, identical when parsed, and rejected by `pnpm format:check`. Enumeration is a TEXT edit now: read the file, splice the test into the test:framework-shell string, write it back. A JSON round-trip rewrites the whole document to serialiser defaults; the only safe edit to a formatted file is one that touches the bytes it means to touch. diff vs main is now exactly one line in each branch.
test(tools/git): a skip that exits 0 is a pass -- refuse instead
2026-08-06 19:11:44 -05:00 · 2026-08-06 19:02:29 -05:00 · 2026-08-06 18:56:03 -05:00
6 changed files with 69 additions and 178 deletions
@@ -46,10 +46,6 @@ steps:
      # [0] of the pnpm chain, so severing that chain would silence it together
      # with everything it guards; this direct line keeps one instrument running.
      - bash packages/mosaic/framework/tools/quality/scripts/check-test-enumeration.sh
-      # Hermetic regression for issue-close.sh (#1081): mocks tea/curl onto PATH
-      # and sandboxes a throwaway git repo, so it resolves no real credentials and
-      # joins CI directly rather than the exclusions file.
-      - bash packages/mosaic/framework/tools/git/test-issue-close-fail-closed.sh

  # Blocking gate (#791): a framework upgrade must never write or delete an
  # operator-owned path. The HARD GATE proves an unanticipated operator sentinel
@@ -5,7 +5,10 @@

 detect_platform() {
    local remote_url
-    remote_url=$(git remote get-url origin 2>/dev/null)
+    # `|| true` is load-bearing under `set -e`: outside a git repo this returns 128 and
+    # kills the CALLER before the -z check below can run, so the error message that is
+    # already written here was unreachable. Same idiom as get_gitea_repo_args() below.
+    remote_url=$(git remote get-url origin 2>/dev/null) || true

    if [[ -z "$remote_url" ]]; then
        echo "error: not a git repository or no origin remote" >&2
@@ -39,7 +42,10 @@ detect_platform() {

 get_repo_info() {
    local remote_url
-    remote_url=$(git remote get-url origin 2>/dev/null)
+    # `|| true` is load-bearing under `set -e`: outside a git repo this returns 128 and
+    # kills the CALLER before the -z check below can run, so the error message that is
+    # already written here was unreachable. Same idiom as get_gitea_repo_args() below.
+    remote_url=$(git remote get-url origin 2>/dev/null) || true

    if [[ -z "$remote_url" ]]; then
        echo "error: not a git repository or no origin remote" >&2
@@ -91,32 +91,13 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
    GITEA_LOGIN_NAME=$(get_gitea_login || true)
    if [[ -n "$GITEA_LOGIN_NAME" ]]; then
        if [[ -n "$COMMENT" ]]; then
-            # `tea issue comment` is NOT a subcommand -- tea 0.11.x lists only
-            # list/create/edit/reopen/close under `tea issue`. Comments are the
-            # TOP-LEVEL `tea comment`, which takes the same --repo/--login flags.
-            # The old call therefore always failed, was unchecked, and the script
-            # closed the issue anyway, losing the record of WHY.
-            #
-            # Use `tea comment` rather than the API helper so the comment and the
-            # close are made by the SAME principal ($GITEA_LOGIN_NAME). Routing the
-            # comment through the token-authenticated helper here would attribute the
-            # comment to the token holder and the close to the tea login -- two
-            # principals for one operation.
-            tea comment "$ISSUE_NUMBER" "$COMMENT" --repo "$OWNER/$REPO" --login "$GITEA_LOGIN_NAME" || {
-                echo "Error: failed to post comment on #$ISSUE_NUMBER -- NOT closing (fail closed)." >&2
-                exit 1
-            }
+            tea issue comment "$ISSUE_NUMBER" "$COMMENT" --repo "$OWNER/$REPO" --login "$GITEA_LOGIN_NAME"
        fi
        tea issue close "$ISSUE_NUMBER" --repo "$OWNER/$REPO" --login "$GITEA_LOGIN_NAME"
    else
        echo "No tea login configured for $(get_remote_host); using authenticated Gitea API fallback." >&2
        if [[ -n "$COMMENT" ]]; then
-            # Fail closed here too: an unchecked comment lets the issue close without its
-            # audit trail, which is the same defect as the tea path above.
-            gitea_issue_comment_api || {
-                echo "Error: failed to post comment on #$ISSUE_NUMBER -- NOT closing (fail closed)." >&2
-                exit 1
-            }
+            gitea_issue_comment_api
        fi
        gitea_issue_close_api
    fi
@@ -0,0 +1,58 @@
+#!/bin/bash
+# Regression: detect_platform / get_repo_info must FAIL LOUDLY outside a git repo,
+# not kill the caller silently.
+#
+# Both functions already contained the right error path:
+#     if [[ -z "$remote_url" ]]; then echo "error: not a git repository..." >&2; return 1; fi
+# but under `set -e` -- which every wrapper in this directory uses -- the preceding
+# assignment `remote_url=$(git remote get-url origin 2>/dev/null)` returns git's 128
+# outside a repo and terminates the CALLER first. The message was unreachable.
+#
+# Observed cost: pr-review.sh invoked from a non-repo cwd exits 128 with NO stdout and
+# NO stderr, even when -r/--repo and -H/--host are supplied -- the flags documented as
+# "skips git-remote inference". Two reviewer seats hit this and correctly reported
+# `blocked` with no diagnostic to report.
+#
+# The control that matters is the LOUD one: asserting "rc != 0" passes on the broken
+# build too, because 128 is also non-zero. The test must assert the MESSAGE.
+set -uo pipefail
+fail=0
+HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+TMP="$(mktemp -d)"; trap 'rm -rf "$TMP"' EXIT
+
+run_outside() { # $1=function name -> "rc:sawmessage"
+    local fn="$1" out rc
+    out=$( cd "$TMP" && bash -c "set -e; source '$HERE/detect-platform.sh'; $fn" 2>&1 ); rc=$?
+    printf '%s:%s' "$rc" "$(grep -qi 'not a git repository' <<<"$out" && echo yes || echo no)"
+}
+check() { if [ "$2" = "$3" ]; then echo "  PASS  $1 ($2)"; else echo "  FAIL  $1: got $2, want $3"; fail=1; fi; }
+
+# $TMP must not be inside a git repo. Do not SKIP on failure: be-coder-07 showed the
+# original SKIP exited 0, so pointing TMPDIR beneath a git worktree made this test PASS
+# against unchanged main. A skip that exits 0 is indistinguishable from a pass.
+# GIT_CEILING_DIRECTORIES stops git walking above $TMP, making the condition hold
+# regardless of where TMPDIR lives, rather than merely detecting when it does not.
+# GIT_CEILING_DIRECTORIES is matched against the PHYSICAL path -- a symlinked TMPDIR
+# (/tmp is commonly one) makes the logical path never match, and the ceiling silently
+# does nothing. Resolve it before exporting.
+TMP="$(cd "$TMP" && pwd -P)"
+export GIT_CEILING_DIRECTORIES="$TMP"
+if ( cd "$TMP" && git rev-parse --git-dir >/dev/null 2>&1 ); then
+    echo "  FAIL  scratch dir is inside a git repo even with GIT_CEILING_DIRECTORIES set;"
+    echo "        the outside-a-repo precondition cannot be established -- refusing to report a result"
+    exit 1
+fi
+
+echo "== outside a git repo: rc=1 AND the diagnostic is emitted =="
+check "detect_platform" "$(run_outside detect_platform)" "1:yes"
+check "get_repo_info"   "$(run_outside get_repo_info)"   "1:yes"
+
+echo "== inside a git repo the functions still work =="
+git init -q "$TMP/repo" 2>/dev/null
+git -C "$TMP/repo" remote add origin https://git.mosaicstack.dev/mosaicstack/stack.git 2>/dev/null
+out=$( cd "$TMP/repo" && bash -c "set -e; source '$HERE/detect-platform.sh'; detect_platform" 2>&1 ); rc=$?
+if [ "$rc" -eq 0 ] && grep -qi 'gitea' <<<"$out"; then echo "  PASS  detect_platform in-repo (rc=0, $out)"
+else echo "  FAIL  detect_platform in-repo: rc=$rc out=$out"; fail=1; fi
+
+[ "$fail" -eq 0 ] && echo "OK  detect-platform fails loudly outside a repo" || echo "FAILED"
+exit "$fail"
@@ -1,150 +0,0 @@
-#!/usr/bin/env bash
-# Regression: issue-close.sh must NOT close an issue when the closing comment could not
-# be posted, and comment+close must be made by ONE principal.
-#
-# Guards two defects fixed together (see #1081):
-#   1. `tea issue comment` is not a subcommand -- tea exposes comments as the TOP-LEVEL
-#      `tea comment`. The old call always failed, was unchecked, and the issue closed
-#      anyway, losing the record of WHY it was closed.
-#   2. Routing the comment through the token-authenticated API helper while the close
-#      used --login would attribute one operation to two principals.
-#
-# SAFETY (rev-974, #1085 review 130): this test previously ran under `set -uo pipefail`
-# with unchecked mkdir/redirect/cd, then prepended a possibly-nonexistent $MOCK_BIN to
-# PATH -- while `git remote add origin` names the REAL repository. Forcing setup failure
-# with an unwritable AGENT_WORK_ROOT made it `git init` in its CALLER's directory and
-# invoke the real, provider-mutating issue-close.sh. Setup now fails closed, and both
-# `tea` and `curl` are asserted to resolve INSIDE $MOCK_BIN before any target run.
-set -euo pipefail
-# NOTE: with `set -e`, `grep -q X && fail "..."` is a trap -- the ABSENT case (grep rc=1,
-# which is the PASSING case for a must-not-appear assertion) is the last command of an &&
-# list and silently terminates the script with no message. Every must-not-appear check
-# below is therefore an if-block. This is the same set -e + &&-list defect be-coder-08
-# found in #1086, reintroduced here by adding `set -e` for the sandbox-safety fix.
-
-WORK_ROOT="${AGENT_WORK_ROOT:-${TMPDIR:-/tmp}}"
-SANDBOX="$WORK_ROOT/issue-close-fail-closed-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-close.sh"
-[ -f "$TARGET" ] || { echo "FAIL: issue-close.sh not found beside this test"; exit 1; }
-fail() { echo "FAIL: $*"; exit 1; }
-
-# Every setup step is checked. Under `set -e` these abort; the explicit || fail keeps the
-# reason legible instead of a bare non-zero exit.
-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"
-
-cat > "$MOCK_BIN/curl" <<'EOF'
-#!/bin/bash
-method=GET; url=""
-while [ $# -gt 0 ]; do
-    case "$1" in
-        -X) method="$2"; shift 2 ;;
-        http*|https*) url="$1"; shift ;;
-        *) shift ;;
-    esac
-done
-printf 'curl %s %s\n' "$method" "$url" >> "$CALLS"
-[ "${MOCK_CURL_FAIL:-}" = "1" ] && [ "$method" = "POST" ] && exit 22
-exit 0
-EOF
-chmod +x "$MOCK_BIN/curl"
-
-mk_tea() {  # $1 = exit code for a comment attempt; $2 = login list (empty => no login)
-    local rc="$1" login="${2-}"
-    cat > "$MOCK_BIN/tea" <<EOF
-#!/bin/bash
-printf 'tea %s\n' "\$*" >> "$CALLS"
-if [[ "\$*" == *"login list"* ]]; then
-    printf '%s\n' '${login}'; exit 0
-fi
-# Fail ANY comment attempt -- both the correct top-level \`tea comment\` and the broken
-# \`tea issue comment\` -- so an unfixed script exercises the DEFECT rather than tripping
-# a setup assertion.
-if [[ "\$1" == "comment" || ( "\$1" == "issue" && "\$2" == "comment" ) ]]; then exit $rc; fi
-exit 0
-EOF
-    chmod +x "$MOCK_BIN/tea"
-}
-LOGIN_JSON='[{"name":"git.mosaicstack.dev","url":"https://git.mosaicstack.dev"}]'
-
-# The mocks must be the ones that run. Without this, a failed setup silently falls through
-# to the real tea/curl and the "test" mutates the real provider.
-assert_mocked() {
-    local w
-    for w in tea curl; do
-        p=$(command -v "$w" || true)
-        [ -n "$p" ] || fail "SAFETY: $w does not resolve at all"
-        case "$p" in
-            "$MOCK_BIN"/*) : ;;
-            *) fail "SAFETY: $w resolves to $p, OUTSIDE the sandbox -- refusing to invoke the target" ;;
-        esac
-    done
-}
-
-run_target() {  # never let a target failure abort the test; we assert on rc
-    # Call sites MUST use `rc=0; run_target ... || rc=$?` -- a bare `run_target ...; rc=$?`
-    # lets the non-zero RETURN trip set -e in the CALLER before rc is ever read.
-    set +e; bash "$TARGET" "$@" >/dev/null 2>&1; local rc=$?; set -e; return $rc
-}
-
-# ── tea path ────────────────────────────────────────────────────────────────────────
-# 1. NEGATIVE (the regression): comment fails => must NOT close, must exit non-zero
-mk_tea 1 "$LOGIN_JSON"; : > "$CALLS"; assert_mocked
-rc=0; run_target -i 42 -c "closing note" || rc=$?
-grep -qE 'tea (issue )?comment' "$CALLS" || fail "no comment attempt -- setup did not reach the tea branch"
-if grep -q 'tea issue close' "$CALLS"; then fail "ISSUE CLOSED AFTER THE COMMENT FAILED -- the regression"; fi
-[ "$rc" -ne 0 ] || fail "comment failed but issue-close exited 0 -- FAIL-OPEN"
-
-# 2. POSITIVE: comment succeeds => close proceeds, exit 0
-mk_tea 0 "$LOGIN_JSON"; : > "$CALLS"; assert_mocked
-rc=0; run_target -i 42 -c "closing note" || rc=$?
-[ "$rc" -eq 0 ] || fail "comment succeeded but issue-close exited $rc"
-grep -q 'tea issue close' "$CALLS" || fail "issue not closed even though the comment succeeded"
-
-# 3. must use top-level `tea comment`, never `tea issue comment`
-if grep -q 'tea issue comment' "$CALLS"; then fail "used 'tea issue comment' -- not a valid subcommand"; fi
-
-# 4. ONE PRINCIPAL: comment and close must carry the SAME --login
-c=$(grep -m1 '^tea comment'     "$CALLS" | grep -o -- '--login [^ ]*' | awk '{print $2}')
-k=$(grep -m1 '^tea issue close' "$CALLS" | grep -o -- '--login [^ ]*' | awk '{print $2}')
-[ -n "$c" ] || fail "comment carried no --login"
-[ "$c" = "$k" ] || fail "MIXED PRINCIPALS: comment=$c close=$k"
-
-# ── no-login / API fallback path ────────────────────────────────────────────────────
-# rev-974: the delta also adds fail-closed behaviour to this branch, and the suite never
-# reached it -- replacing the whole fallback contract with an unconditional close still
-# passed. These assert the POSTCONDITION (which HTTP calls happened, in what order),
-# not merely that a command ran.
-# 5. no login + comment FAILS => POST attempted, NO PATCH, non-zero
-mk_tea 0 ""; : > "$CALLS"; assert_mocked
-rc=0; MOCK_CURL_FAIL=1 run_target -i 42 -c "closing note" || rc=$?
-grep -q 'curl POST' "$CALLS" || fail "API path: no comment POST attempted"
-if grep -q 'curl PATCH' "$CALLS"; then fail "API path: ISSUE CLOSED (PATCH) AFTER THE COMMENT POST FAILED"; fi
-[ "$rc" -ne 0 ] || fail "API path: comment failed but exited 0 -- FAIL-OPEN"
-
-# 6. no login + comment SUCCEEDS => POST strictly BEFORE PATCH, exit 0
-mk_tea 0 ""; : > "$CALLS"; assert_mocked
-rc=0; run_target -i 42 -c "closing note" || rc=$?
-[ "$rc" -eq 0 ] || fail "API path: comment succeeded but exited $rc"
-order=$(grep -oE 'curl (POST|PATCH)' "$CALLS" | awk '{print $2}' | paste -sd, -)
-[ "$order" = "POST,PATCH" ] || fail "API path: expected POST,PATCH -- got '${order:-<none>}'"
-
-# 7. no login + NO comment => PATCH only, never a POST
-mk_tea 0 ""; : > "$CALLS"; assert_mocked
-rc=0; run_target -i 42 || rc=$?
-[ "$rc" -eq 0 ] || fail "API path: no-comment close exited $rc"
-if grep -q 'curl POST' "$CALLS"; then fail "API path: posted a comment when none was requested"; fi
-grep -q 'curl PATCH' "$CALLS" || fail "API path: issue not closed when no comment was requested"
-
-echo "issue-close.sh fail-closed + single-principal regression passed"
@@ -25,7 +25,7 @@
    "lint": "eslint src",
    "typecheck": "tsc --noEmit",
    "test": "vitest run --passWithNoTests && pnpm run test:framework-shell",
-    "test:framework-shell": "bash framework/tools/quality/scripts/check-test-enumeration.sh && bash framework/tools/quality/scripts/test-check-test-enumeration.sh && 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 src/mutator-gate/version_coupling_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 && bash framework/tools/git/test-pr-review-repo-host-override.sh && bash framework/tools/git/test-ci-queue-wait-branch-absent.sh && bash framework/tools/git/test-ci-queue-wait-tristate.sh && bash framework/tools/git/test-ci-queue-wait-github-checks.sh && bash framework/tools/git/test-pr-merge-queue-branch.sh && bash framework/tools/git/test-pr-merge-head-pin.sh && bash framework/tools/git/test-pr-merge-message-field.sh && bash framework/tools/git/test-git-credential-mosaic.sh && bash framework/tools/git/test-gitea-token-identity.sh && bash framework/tools/woodpecker/test-terminal-green-contract.sh && bash framework/tools/_scripts/test-install-ordering-guard.sh && bash framework/tools/tmux/agent-send.test.sh && bash framework/tools/wake/test-wake-store-ack.sh && bash framework/tools/wake/test-wake-store-enqueue-race.sh && bash framework/tools/wake/test-wake-digest-hmac.sh && bash framework/tools/wake/test-wake-digest-quarantine.sh && bash framework/tools/wake/test-wake-detector.sh && bash framework/tools/wake/test-wake-fn-oracle.sh && bash framework/tools/wake/test-wake-reconcile.sh && bash framework/tools/wake/test-wake-beacon.sh && bash framework/tools/wake/test-wake-preimage.sh && bash framework/tools/wake/test-wake-install.sh"
+    "test:framework-shell": "bash framework/tools/quality/scripts/check-test-enumeration.sh && bash framework/tools/quality/scripts/test-check-test-enumeration.sh && 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 src/mutator-gate/version_coupling_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 && bash framework/tools/git/test-pr-review-repo-host-override.sh && bash framework/tools/git/test-ci-queue-wait-branch-absent.sh && bash framework/tools/git/test-ci-queue-wait-tristate.sh && bash framework/tools/git/test-ci-queue-wait-github-checks.sh && bash framework/tools/git/test-pr-merge-queue-branch.sh && bash framework/tools/git/test-pr-merge-head-pin.sh && bash framework/tools/git/test-pr-merge-message-field.sh && bash framework/tools/git/test-git-credential-mosaic.sh && bash framework/tools/git/test-gitea-token-identity.sh && bash framework/tools/git/test-detect-platform-outside-repo.sh && bash framework/tools/woodpecker/test-terminal-green-contract.sh && bash framework/tools/_scripts/test-install-ordering-guard.sh && bash framework/tools/tmux/agent-send.test.sh && bash framework/tools/wake/test-wake-store-ack.sh && bash framework/tools/wake/test-wake-store-enqueue-race.sh && bash framework/tools/wake/test-wake-digest-hmac.sh && bash framework/tools/wake/test-wake-digest-quarantine.sh && bash framework/tools/wake/test-wake-detector.sh && bash framework/tools/wake/test-wake-fn-oracle.sh && bash framework/tools/wake/test-wake-reconcile.sh && bash framework/tools/wake/test-wake-beacon.sh && bash framework/tools/wake/test-wake-preimage.sh && bash framework/tools/wake/test-wake-install.sh"
  },
  "dependencies": {
    "@mosaicstack/brain": "workspace:*",