Compare commits
7 Commits
feat/791-p
...
fix/795-co
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
027a99b558 | ||
|
|
0e50c694cb | ||
|
|
080a6b4663 | ||
|
|
13c50c5fa9 | ||
| fe7a468c9d | |||
| cabf02e7b9 | |||
| 9ddc6fbda8 |
@@ -22,10 +22,10 @@
|
|||||||
FROM node:24-alpine
|
FROM node:24-alpine
|
||||||
|
|
||||||
# Native toolchain required to compile node-gyp deps on musl, plus the
|
# Native toolchain required to compile node-gyp deps on musl, plus the
|
||||||
# postgresql-client used by the test step's pg_isready readiness probe. `bash`
|
# postgresql-client used by the test step's pg_isready readiness probe. `bash`,
|
||||||
# is baked here too — the sanitization step in ci.yml otherwise does a per-run
|
# `git`, and `jq` are baked here too — framework shell tests and the shipped
|
||||||
# `apk add bash`.
|
# Codex review wrappers require them without per-run installation in ci.yml.
|
||||||
RUN apk add --no-cache python3 make g++ postgresql-client bash
|
RUN apk add --no-cache python3 make g++ postgresql-client bash git jq
|
||||||
|
|
||||||
# Pin pnpm to the repo's packageManager version via corepack.
|
# Pin pnpm to the repo's packageManager version via corepack.
|
||||||
RUN corepack enable && corepack prepare pnpm@10.6.2 --activate
|
RUN corepack enable && corepack prepare pnpm@10.6.2 --activate
|
||||||
|
|||||||
@@ -70,6 +70,8 @@ Security vulnerability review focusing on:
|
|||||||
~/.config/mosaic/tools/codex/codex-security-review.sh -n 42
|
~/.config/mosaic/tools/codex/codex-security-review.sh -n 42
|
||||||
```
|
```
|
||||||
|
|
||||||
|
PR mode resolves the provider's PR diff rather than relying on the caller's checked-out branch. On Gitea, it fetches the base and `refs/pull/<number>/head` refs and diffs those explicit refs. If the refs cannot be fetched or the resulting diff is empty, the command exits nonzero before Codex runs or a review is posted.
|
||||||
|
|
||||||
### Review Against Base Branch
|
### Review Against Base Branch
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
@@ -253,7 +255,7 @@ Run the script from inside a git repository.
|
|||||||
|
|
||||||
### "No changes found to review"
|
### "No changes found to review"
|
||||||
|
|
||||||
The specified mode (--uncommitted, --base, etc.) found no changes to review.
|
The specified non-PR mode (`--uncommitted`, `--base`, etc.) found no changes to review. PR mode instead fails closed with an actionable error when it cannot construct a non-empty provider diff; verify the PR number, remote, provider login, and ref access before retrying.
|
||||||
|
|
||||||
### "Codex produced no output"
|
### "Codex produced no output"
|
||||||
|
|
||||||
|
|||||||
@@ -44,38 +44,47 @@ build_diff_context() {
|
|||||||
diff_text=$(git show "$value" 2>/dev/null)
|
diff_text=$(git show "$value" 2>/dev/null)
|
||||||
;;
|
;;
|
||||||
pr)
|
pr)
|
||||||
# For PRs, we need to fetch the PR diff
|
# Provider detection writes its result to stdout; suppress it so it cannot
|
||||||
detect_platform
|
# be mistaken for diff content when this function is used in a substitution.
|
||||||
|
detect_platform >/dev/null
|
||||||
if [[ "$PLATFORM" == "github" ]]; then
|
if [[ "$PLATFORM" == "github" ]]; then
|
||||||
diff_text=$(gh pr diff "$value" 2>/dev/null)
|
diff_text=$(gh pr diff "$value" 2>/dev/null) || {
|
||||||
|
echo "Error: Failed to fetch the diff for PR #${value}." >&2
|
||||||
|
return 1
|
||||||
|
}
|
||||||
elif [[ "$PLATFORM" == "gitea" ]]; then
|
elif [[ "$PLATFORM" == "gitea" ]]; then
|
||||||
# tea doesn't have a direct pr diff command, use git
|
local pr_base base_ref pr_head_ref
|
||||||
local pr_base
|
pr_base=$(tea pr list --fields index,base --output simple 2>/dev/null | awk -v pr="$value" '$1 == pr { print $2; exit }')
|
||||||
pr_base=$(tea pr list --fields index,base --output simple 2>/dev/null | grep "^${value}" | awk '{print $2}')
|
if [[ -z "$pr_base" ]]; then
|
||||||
if [[ -n "$pr_base" ]]; then
|
echo "Error: Could not resolve the base branch for Gitea PR #${value}." >&2
|
||||||
diff_text=$(git diff "${pr_base}...HEAD" 2>/dev/null)
|
return 1
|
||||||
else
|
|
||||||
# Fallback: fetch PR info via API
|
|
||||||
local repo_info
|
|
||||||
repo_info=$(get_repo_info)
|
|
||||||
local remote_url
|
|
||||||
remote_url=$(git remote get-url origin 2>/dev/null)
|
|
||||||
local host
|
|
||||||
host=$(echo "$remote_url" | sed -E 's|.*://([^/]+).*|\1|; s|.*@([^:]+).*|\1|')
|
|
||||||
diff_text=$(curl -s "https://${host}/api/v1/repos/${repo_info}/pulls/${value}" \
|
|
||||||
-H "Authorization: token $(tea login list --output simple 2>/dev/null | head -1 | awk '{print $2}')" \
|
|
||||||
2>/dev/null | jq -r '.diff_url // empty')
|
|
||||||
if [[ -n "$diff_text" && "$diff_text" != "null" ]]; then
|
|
||||||
diff_text=$(curl -s "$diff_text" 2>/dev/null)
|
|
||||||
else
|
|
||||||
diff_text=$(git diff "main...HEAD" 2>/dev/null)
|
|
||||||
fi
|
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
base_ref="refs/remotes/origin/${pr_base}"
|
||||||
|
pr_head_ref="refs/remotes/origin/pr/${value}/head"
|
||||||
|
if ! git fetch --quiet origin \
|
||||||
|
"+refs/heads/${pr_base}:${base_ref}" \
|
||||||
|
"+refs/pull/${value}/head:${pr_head_ref}"; then
|
||||||
|
echo "Error: Failed to fetch the base and head refs for Gitea PR #${value}." >&2
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
diff_text=$(git diff "${base_ref}...${pr_head_ref}") || {
|
||||||
|
echo "Error: Failed to diff the fetched refs for Gitea PR #${value}." >&2
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
else
|
||||||
|
echo "Error: Unsupported git platform while resolving PR #${value}." >&2
|
||||||
|
return 1
|
||||||
fi
|
fi
|
||||||
;;
|
;;
|
||||||
esac
|
esac
|
||||||
|
|
||||||
echo "$diff_text"
|
if [[ "$mode" == "pr" && -z "${diff_text//[[:space:]]/}" ]]; then
|
||||||
|
echo "Error: Unable to construct a non-empty diff for PR #${value}; verify the PR refs and provider access." >&2
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
printf '%s\n' "$diff_text"
|
||||||
}
|
}
|
||||||
|
|
||||||
# Format JSON findings as markdown for PR comments
|
# Format JSON findings as markdown for PR comments
|
||||||
|
|||||||
158
packages/mosaic/framework/tools/codex/test-pr-diff-context.sh
Normal file
158
packages/mosaic/framework/tools/codex/test-pr-diff-context.sh
Normal file
@@ -0,0 +1,158 @@
|
|||||||
|
#!/bin/bash
|
||||||
|
# Hermetic regression coverage for Gitea PR diff construction and fail-closed reviews.
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
TMP_DIR=$(mktemp -d)
|
||||||
|
trap 'rm -rf "$TMP_DIR"' EXIT
|
||||||
|
|
||||||
|
fail() {
|
||||||
|
echo "not ok - $*" >&2
|
||||||
|
exit 1
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_contains() {
|
||||||
|
local haystack="$1" needle="$2"
|
||||||
|
if [[ "$haystack" != *"$needle"* ]]; then
|
||||||
|
printf 'actual output:\n%s\n' "$haystack" >&2
|
||||||
|
fail "expected output to contain: $needle"
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
|
# Prevent CI-provided repository context from leaking into the fixture repositories.
|
||||||
|
unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \
|
||||||
|
GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR
|
||||||
|
export GIT_AUTHOR_NAME="Codex Fixture"
|
||||||
|
export GIT_AUTHOR_EMAIL="codex-fixture@example.test"
|
||||||
|
export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME"
|
||||||
|
export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
|
||||||
|
export GITEA_LOGIN="fixture"
|
||||||
|
export GITEA_TOKEN="fixture-token"
|
||||||
|
export GITEA_URL="file://$TMP_DIR"
|
||||||
|
|
||||||
|
create_pr_fixture() {
|
||||||
|
local fixture_root="$1" head_mode="$2"
|
||||||
|
local origin="$fixture_root/origin.git"
|
||||||
|
local seed="$fixture_root/seed"
|
||||||
|
local work="$fixture_root/work"
|
||||||
|
local base_sha head_sha
|
||||||
|
|
||||||
|
mkdir -p "$fixture_root"
|
||||||
|
git init --quiet --bare "$origin"
|
||||||
|
git init --quiet --initial-branch=release/next "$seed"
|
||||||
|
printf 'base\n' > "$seed/pr-change.ts"
|
||||||
|
git -C "$seed" add pr-change.ts
|
||||||
|
git -C "$seed" commit --quiet -m "fixture base"
|
||||||
|
base_sha=$(git -C "$seed" rev-parse HEAD)
|
||||||
|
git -C "$seed" remote add origin "$origin"
|
||||||
|
git -C "$seed" push --quiet origin release/next
|
||||||
|
git --git-dir="$origin" symbolic-ref HEAD refs/heads/release/next
|
||||||
|
|
||||||
|
if [[ "$head_mode" == "changed" ]]; then
|
||||||
|
git -C "$seed" switch --quiet -c feature/pr-795
|
||||||
|
printf 'actual-pr-change\n' > "$seed/pr-change.ts"
|
||||||
|
git -C "$seed" commit --quiet -am "fixture PR head"
|
||||||
|
head_sha=$(git -C "$seed" rev-parse HEAD)
|
||||||
|
git -C "$seed" push --quiet origin HEAD:refs/pull/795/head
|
||||||
|
else
|
||||||
|
head_sha="$base_sha"
|
||||||
|
git --git-dir="$origin" update-ref refs/pull/795/head "$head_sha"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Gitea's provider-owned PR head ref now exists in the local bare origin.
|
||||||
|
git clone --quiet "$origin" "$work"
|
||||||
|
printf '%s\n' "$work"
|
||||||
|
}
|
||||||
|
|
||||||
|
FAKE_BIN="$TMP_DIR/bin"
|
||||||
|
mkdir -p "$FAKE_BIN"
|
||||||
|
cat > "$FAKE_BIN/tea" <<'STUB'
|
||||||
|
#!/bin/bash
|
||||||
|
if [[ "$*" == "pr list --fields index,base --output simple" ]]; then
|
||||||
|
printf '795 release/next\n'
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
exit 1
|
||||||
|
STUB
|
||||||
|
cat > "$FAKE_BIN/codex" <<'STUB'
|
||||||
|
#!/bin/bash
|
||||||
|
printf 'CODEX %s\n' "$*" >> "$CODEX_LOG"
|
||||||
|
exit 99
|
||||||
|
STUB
|
||||||
|
chmod +x "$FAKE_BIN/tea" "$FAKE_BIN/codex"
|
||||||
|
export PATH="$FAKE_BIN:$PATH"
|
||||||
|
|
||||||
|
# The valid fixture is a fresh clone on the non-main base. The PR head exists only
|
||||||
|
# at refs/pull/795/head, so local HEAD cannot accidentally satisfy the assertion.
|
||||||
|
if [[ "${1:-all}" != "fail-closed" ]]; then
|
||||||
|
VALID_WORK=$(create_pr_fixture "$TMP_DIR/valid" changed)
|
||||||
|
(
|
||||||
|
cd "$VALID_WORK"
|
||||||
|
# shellcheck source=common.sh
|
||||||
|
source "$SCRIPT_DIR/common.sh"
|
||||||
|
diff_context=$(build_diff_context pr 795)
|
||||||
|
assert_contains "$diff_context" "actual-pr-change"
|
||||||
|
|
||||||
|
base_sha=$(git rev-parse refs/remotes/origin/release/next)
|
||||||
|
head_sha=$(git rev-parse refs/remotes/origin/pr/795/head)
|
||||||
|
local_sha=$(git rev-parse HEAD)
|
||||||
|
[[ "$local_sha" == "$base_sha" ]] || fail "fixture clone is not on the PR base"
|
||||||
|
[[ "$head_sha" != "$base_sha" ]] || fail "fixture PR head does not differ from its base"
|
||||||
|
git show-ref --verify --quiet refs/remotes/origin/pr/795/head || \
|
||||||
|
fail "fetched PR head ref is missing"
|
||||||
|
[[ "$(git diff --name-only "${base_sha}...${head_sha}")" == "pr-change.ts" ]] || \
|
||||||
|
fail "explicit PR refs do not contain the fixture change"
|
||||||
|
if git show-ref --verify --quiet refs/heads/main || \
|
||||||
|
git show-ref --verify --quiet refs/remotes/origin/main; then
|
||||||
|
fail "fixture unexpectedly contains a main ref"
|
||||||
|
fi
|
||||||
|
)
|
||||||
|
echo "ok - Gitea PR mode fetches and diffs explicit non-main base and PR head refs"
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Build an empty PR entirely inside another local repository. Both review wrappers
|
||||||
|
# must emit the PR-numbered error before Codex or the stubbed post path can execute.
|
||||||
|
if [[ "${1:-all}" != "pr-head" ]]; then
|
||||||
|
EMPTY_WORK=$(create_pr_fixture "$TMP_DIR/empty" empty)
|
||||||
|
SANDBOX="$TMP_DIR/sandbox"
|
||||||
|
mkdir -p "$SANDBOX/tools/codex/schemas" "$SANDBOX/tools/git"
|
||||||
|
cp "$SCRIPT_DIR/common.sh" \
|
||||||
|
"$SCRIPT_DIR/codex-code-review.sh" \
|
||||||
|
"$SCRIPT_DIR/codex-security-review.sh" \
|
||||||
|
"$SANDBOX/tools/codex/"
|
||||||
|
cp "$SCRIPT_DIR/schemas/code-review-schema.json" \
|
||||||
|
"$SCRIPT_DIR/schemas/security-review-schema.json" \
|
||||||
|
"$SANDBOX/tools/codex/schemas/"
|
||||||
|
cp "$SCRIPT_DIR/../git/detect-platform.sh" "$SANDBOX/tools/git/"
|
||||||
|
|
||||||
|
cat > "$SANDBOX/tools/git/pr-review.sh" <<'STUB'
|
||||||
|
#!/bin/bash
|
||||||
|
printf 'POST %s\n' "$*" >> "$POST_LOG"
|
||||||
|
STUB
|
||||||
|
chmod +x "$SANDBOX/tools/git/pr-review.sh"
|
||||||
|
|
||||||
|
POST_LOG="$TMP_DIR/post.log"
|
||||||
|
CODEX_LOG="$TMP_DIR/codex.log"
|
||||||
|
export POST_LOG CODEX_LOG
|
||||||
|
|
||||||
|
for review_kind in code security; do
|
||||||
|
: > "$POST_LOG"
|
||||||
|
: > "$CODEX_LOG"
|
||||||
|
review_script="$SANDBOX/tools/codex/codex-${review_kind}-review.sh"
|
||||||
|
set +e
|
||||||
|
(
|
||||||
|
cd "$EMPTY_WORK"
|
||||||
|
"$review_script" -n 795
|
||||||
|
) >"$TMP_DIR/${review_kind}.stdout" 2>"$TMP_DIR/${review_kind}.stderr"
|
||||||
|
review_status=$?
|
||||||
|
set -e
|
||||||
|
|
||||||
|
stderr_text=$(cat "$TMP_DIR/${review_kind}.stderr")
|
||||||
|
[[ "$review_status" -ne 0 ]] || fail "${review_kind} review returned success for an empty PR diff"
|
||||||
|
[[ ! -s "$CODEX_LOG" ]] || fail "Codex ran for an empty ${review_kind} PR diff"
|
||||||
|
[[ ! -s "$POST_LOG" ]] || fail "${review_kind} review auto-post ran for an empty PR diff"
|
||||||
|
assert_contains "$stderr_text" "Error:"
|
||||||
|
assert_contains "$stderr_text" "PR #795"
|
||||||
|
echo "ok - empty ${review_kind} PR diff fails closed before Codex and auto-post"
|
||||||
|
done
|
||||||
|
fi
|
||||||
@@ -24,7 +24,8 @@
|
|||||||
"build": "tsc",
|
"build": "tsc",
|
||||||
"lint": "eslint src",
|
"lint": "eslint src",
|
||||||
"typecheck": "tsc --noEmit",
|
"typecheck": "tsc --noEmit",
|
||||||
"test": "vitest run --passWithNoTests"
|
"test": "vitest run --passWithNoTests && pnpm run test:framework-shell",
|
||||||
|
"test:framework-shell": "bash framework/tools/codex/test-pr-diff-context.sh"
|
||||||
},
|
},
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"@mosaicstack/brain": "workspace:*",
|
"@mosaicstack/brain": "workspace:*",
|
||||||
|
|||||||
Reference in New Issue
Block a user