From b4b70675b6d488aad2ad9c0fc5fb13d690e689cd Mon Sep 17 00:00:00 2001 From: Mos Date: Thu, 6 Aug 2026 18:56:03 -0500 Subject: [PATCH 1/3] fix(tools/git): detect-platform died silently outside a repo, taking every wrapper with it detect_platform() and get_repo_info() both already contained the correct error path: remote_url=$(git remote get-url origin 2>/dev/null) if [[ -z "$remote_url" ]]; then echo "error: not a git repository or no origin remote" >&2 return 1 fi but that message was UNREACHABLE. Every wrapper in this directory runs `set -e`, and outside a git repo the assignment itself returns git's 128, terminating the caller before the -z check. The diagnostic was written and could never print. Observed cost: `pr-review.sh -n N -r owner/repo -H host -a comment -c "..."` invoked from a non-repo cwd exits 128 with NO stdout and NO stderr -- including when -r and -H are supplied, the flags whose help text says "skips git-remote inference". The inference runs first regardless. Two reviewer seats hit this tonight and correctly reported `blocked` with no diagnostic available to report; one of them lost a full review. Reproduced: cd /tmp && pr-review.sh -n 1086 -r mosaicstack/stack -H git.mosaicstack.dev \ -a comment -c "x" -> rc=128, 0 bytes of output same command from inside a repo -> rc=1, "Error: Comment required" `|| true` matches the idiom already used at the other two call sites in this same file (one `|| return 1`, one `|| true`), so the file was internally inconsistent: two guarded sites, two unguarded. test-detect-platform-outside-repo.sh asserts the MESSAGE, not merely a non-zero rc -- "rc != 0" passes on the broken build too, since 128 is also non-zero. RED on main (128:no for both functions), GREEN here (1:yes), with an in-repo control proving the functions still resolve normally. Enumerated on test:framework-shell. Found-by: be-coder-08, rev0 (both reported `blocked` per Gate 8 rather than working around it) --- .../framework/tools/git/detect-platform.sh | 10 ++++- .../git/test-detect-platform-outside-repo.sh | 45 +++++++++++++++++++ packages/mosaic/package.json | 4 +- 3 files changed, 55 insertions(+), 4 deletions(-) create mode 100755 packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh diff --git a/packages/mosaic/framework/tools/git/detect-platform.sh b/packages/mosaic/framework/tools/git/detect-platform.sh index 97c6e5b3..a06452d1 100755 --- a/packages/mosaic/framework/tools/git/detect-platform.sh +++ b/packages/mosaic/framework/tools/git/detect-platform.sh @@ -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 diff --git a/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh b/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh new file mode 100755 index 00000000..fa7978c3 --- /dev/null +++ b/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh @@ -0,0 +1,45 @@ +#!/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 is deliberately not a git repo, and must not be inside one. +( cd "$TMP" && git rev-parse --git-dir >/dev/null 2>&1 ) && { echo " SKIP scratch dir is inside a repo"; exit 0; } + +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" diff --git a/packages/mosaic/package.json b/packages/mosaic/package.json index 23234ef3..bc7f750e 100644 --- a/packages/mosaic/package.json +++ b/packages/mosaic/package.json @@ -6,7 +6,7 @@ "url": "https://git.mosaicstack.dev/mosaicstack/stack.git", "directory": "packages/mosaic" }, - "description": "Mosaic agent framework — installation wizard and meta package", + "description": "Mosaic agent framework \u2014 installation wizard and meta package", "type": "module", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -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:*", -- 2.54.0 From 92a63e70275d02aff2d864d856b466ff39ff3d09 Mon Sep 17 00:00:00 2001 From: Mos Date: Thu, 6 Aug 2026 19:02:29 -0500 Subject: [PATCH 2/3] test(tools/git): a skip that exits 0 is a pass -- refuse instead be-coder-07 (#1089 review 131) showed the precondition guard exited 0 when the scratch dir turned out to be inside a git worktree: ( cd "$TMP" && git rev-parse --git-dir ) && { echo "SKIP"; exit 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 -- the same defect this suite exists to catch, in the suite itself. Now: set GIT_CEILING_DIRECTORIES to the PHYSICAL path (it is matched physically, and /tmp is commonly a symlink, so a logical path silently disables the ceiling), and if the outside-a-repo precondition still cannot be established, FAIL with an explicit message rather than skipping. Replaying be-coder-07's attack (TMPDIR beneath a worktree): before: fix rc=0, main rc=0 <- both "pass", the vulnerability after: fix rc=1, main rc=1 <- both refuse; no false pass either way Normal conditions are unchanged and still discriminate: fix rc=0, main rc=1. The test refuses to report a result it cannot establish. That is weaker than running under a hostile TMPDIR and strictly better than lying about it. Reported-by: be-coder-07 --- .../git/test-detect-platform-outside-repo.sh | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh b/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh index fa7978c3..13955f0e 100755 --- a/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh +++ b/packages/mosaic/framework/tools/git/test-detect-platform-outside-repo.sh @@ -27,8 +27,21 @@ run_outside() { # $1=function name -> "rc:sawmessage" } check() { if [ "$2" = "$3" ]; then echo " PASS $1 ($2)"; else echo " FAIL $1: got $2, want $3"; fail=1; fi; } -# $TMP is deliberately not a git repo, and must not be inside one. -( cd "$TMP" && git rev-parse --git-dir >/dev/null 2>&1 ) && { echo " SKIP scratch dir is inside a repo"; exit 0; } +# $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" -- 2.54.0 From 8e677d1f219f64a445bb0ffa4e3553bfd4f606d2 Mon Sep 17 00:00:00 2001 From: Mos Date: Thu, 6 Aug 2026 19:11:44 -0500 Subject: [PATCH 3/3] fix: stop re-escaping non-ASCII in package.json when enumerating a test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/mosaic/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mosaic/package.json b/packages/mosaic/package.json index bc7f750e..7096794d 100644 --- a/packages/mosaic/package.json +++ b/packages/mosaic/package.json @@ -6,7 +6,7 @@ "url": "https://git.mosaicstack.dev/mosaicstack/stack.git", "directory": "packages/mosaic" }, - "description": "Mosaic agent framework \u2014 installation wizard and meta package", + "description": "Mosaic agent framework — installation wizard and meta package", "type": "module", "main": "dist/index.js", "types": "dist/index.d.ts", -- 2.54.0