Compare commits
12 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| d3a57a12f4 | |||
| 2f10a86195 | |||
| f45209e44a | |||
| 42a6ce5815 | |||
| cd91a621a2 | |||
| ad5e9cc534 | |||
| dcce5caa39 | |||
| a7ae6bb7f7 | |||
| 65058d3fba | |||
| faf67f4e34 | |||
| 8f21bdd5fd | |||
| 612e6db792 |
@@ -3,25 +3,25 @@
|
||||
# fork-e2e/pr-N branch (which lets e2e run as a `push` with the test-gateway
|
||||
# secrets). Called by .github/workflows/fork-e2e-mirror.yml.
|
||||
#
|
||||
# Gate: the PR currently carries the `e2e-approved` label AND that label was
|
||||
# last applied by a maintainer (in .github/MAINTAINER@main). GitHub only lets
|
||||
# Triage+ users apply labels, so an external fork author can never apply it; the
|
||||
# maintainer check further narrows "anyone with Triage" down to the MAINTAINER
|
||||
# list. We read the *labeler* from the issue-events timeline rather than the
|
||||
# event sender, so the check still holds on `synchronize` (where the sender is
|
||||
# the fork author pushing new commits, not the maintainer who labeled earlier).
|
||||
# Gate (either condition opens it):
|
||||
# 1. The PR has an approving review from a maintainer (in
|
||||
# .github/MAINTAINER@main), OR
|
||||
# 2. The PR carries the `e2e-approved` label applied by a maintainer.
|
||||
#
|
||||
# The label is intentionally separate from the merge gate (maintainer-approval.yml):
|
||||
# labeling runs e2e but does NOT approve the PR for merge, and approving for
|
||||
# merge does NOT run e2e. New commits while the label is present re-mirror
|
||||
# automatically (this script re-runs on `synchronize`); the security scan plus
|
||||
# the maintainer's review are the safety net for post-approval pushes. Removing
|
||||
# the label (or closing the PR) deletes the mirror branch -- see the workflow.
|
||||
# Path 1 (approval) is the primary flow: approving the PR both satisfies the
|
||||
# merge gate and triggers e2e. Path 2 (label) is a manual escape hatch for
|
||||
# running e2e without approving for merge (e.g. early CI validation).
|
||||
#
|
||||
# New commits while the gate is open re-mirror automatically (this script
|
||||
# re-runs on `synchronize`); the security scan plus the maintainer's review
|
||||
# are the safety net for post-approval pushes. Revoking approval AND removing
|
||||
# the label (or closing the PR) stops future mirrors and cleans up the mirror
|
||||
# branch -- see the workflow.
|
||||
#
|
||||
# Fail closed: any error or unexpected state leaves the gate shut, so secrets
|
||||
# never run on an unverified PR.
|
||||
#
|
||||
# Env in: GH_TOKEN, REPO, PR, LABEL (gate label name, default e2e-approved),
|
||||
# Env in: GH_TOKEN, REPO, PR,
|
||||
# MAINTAINERS (space-separated, from merge-ready/load-maintainers.sh).
|
||||
# Out: `mirror=true|false` and `reason=<text>` on $GITHUB_OUTPUT.
|
||||
|
||||
@@ -33,7 +33,6 @@ emit() {
|
||||
echo "mirror=$1 ($2)"
|
||||
}
|
||||
|
||||
LABEL="${LABEL:-e2e-approved}"
|
||||
MAINTAINERS_LC=$(echo "${MAINTAINERS:-}" | tr '[:upper:]' '[:lower:]')
|
||||
|
||||
if [[ -z "${MAINTAINERS_LC// /}" ]]; then
|
||||
@@ -41,32 +40,39 @@ if [[ -z "${MAINTAINERS_LC// /}" ]]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# 1. Label currently present? Read into a variable first so grep's early exit
|
||||
# can't SIGPIPE the producer, then match against a here-string.
|
||||
LABELS=$(gh pr view "$PR" --repo "$REPO" --json labels --jq '.labels[].name')
|
||||
if ! grep -qxF "$LABEL" <<<"$LABELS"; then
|
||||
emit false "awaiting '$LABEL' label from a maintainer"
|
||||
exit 0
|
||||
fi
|
||||
# --- Path 1: maintainer approval via PR review ---
|
||||
|
||||
# 2. Who applied it last? Latest `labeled` event for this label on the timeline.
|
||||
# (Re-applying after a removal makes the most recent labeler authoritative.)
|
||||
LABELER=$(gh api "repos/$REPO/issues/$PR/events" --paginate \
|
||||
--jq "[.[] | select(.event == \"labeled\" and .label.name == \"$LABEL\")] | last | .actor.login // empty")
|
||||
APPROVERS=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate \
|
||||
--jq '[.[] | select(.state != "COMMENTED")] | group_by(.user.login) | map(max_by(.submitted_at)) | .[] | select(.state == "APPROVED") | .user.login')
|
||||
|
||||
if [[ -z "$LABELER" ]]; then
|
||||
# Label is present but no labeled event found (e.g. created with the PR via a
|
||||
# template) -- can't attribute it to a maintainer, so stay shut.
|
||||
emit false "'$LABEL' present but no attributable labeler; treating as ungated"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
LABELER_LC=$(echo "$LABELER" | tr '[:upper:]' '[:lower:]')
|
||||
for m in $MAINTAINERS_LC; do
|
||||
if [[ "$m" == "$LABELER_LC" ]]; then
|
||||
emit true "'$LABEL' applied by maintainer @$LABELER"
|
||||
exit 0
|
||||
fi
|
||||
for u in $APPROVERS; do
|
||||
u_lc=$(echo "$u" | tr '[:upper:]' '[:lower:]')
|
||||
for m in $MAINTAINERS_LC; do
|
||||
if [[ "$m" == "$u_lc" ]]; then
|
||||
emit true "approved by maintainer @$u"
|
||||
exit 0
|
||||
fi
|
||||
done
|
||||
done
|
||||
|
||||
emit false "'$LABEL' applied by non-maintainer @$LABELER; ignoring"
|
||||
# --- Path 2: e2e-approved label applied by a maintainer ---
|
||||
|
||||
LABEL="e2e-approved"
|
||||
LABELS=$(gh pr view "$PR" --repo "$REPO" --json labels --jq '.labels[].name')
|
||||
if grep -qxF "$LABEL" <<<"$LABELS"; then
|
||||
LABELER=$(gh api "repos/$REPO/issues/$PR/events" --paginate \
|
||||
--jq "[.[] | select(.event == \"labeled\" and .label.name == \"$LABEL\")] | last | .actor.login // empty")
|
||||
|
||||
if [[ -n "$LABELER" ]]; then
|
||||
LABELER_LC=$(echo "$LABELER" | tr '[:upper:]' '[:lower:]')
|
||||
for m in $MAINTAINERS_LC; do
|
||||
if [[ "$m" == "$LABELER_LC" ]]; then
|
||||
emit true "'$LABEL' applied by maintainer @$LABELER"
|
||||
exit 0
|
||||
fi
|
||||
done
|
||||
fi
|
||||
fi
|
||||
|
||||
# Neither path opened the gate.
|
||||
emit false "awaiting approval from a maintainer or '$LABEL' label"
|
||||
|
||||
@@ -2,17 +2,19 @@
|
||||
# Single source of truth for the Merge Ready outcome. Downstream steps
|
||||
# just consume `state`, `short_desc`, and `long_desc`.
|
||||
#
|
||||
# The gate is green iff every required check is green on its own merits.
|
||||
# There is no CI bypass: to land despite red required checks, quarantine the
|
||||
# flaky test (tests/known_failures.yaml) or have a repo admin use GitHub's
|
||||
# native "merge without waiting for requirements" affordance.
|
||||
# The gate is green iff every required check is green on its own merits
|
||||
# AND (for fork PRs) a maintainer has approved. There is no CI bypass: to
|
||||
# land despite red required checks, quarantine the flaky test
|
||||
# (tests/known_failures.yaml) or have a repo admin use GitHub's native
|
||||
# "merge without waiting for requirements" affordance.
|
||||
#
|
||||
# CI eval | state | meaning
|
||||
# ---------+----------+---------------------------
|
||||
# success | success | CI green on its own merits
|
||||
# failure | failure | CI red
|
||||
# CI eval | fork approval | state | meaning
|
||||
# ---------+---------------+----------+---------------------------------
|
||||
# success | n/a or true | success | CI green on its own merits
|
||||
# success | false | failure | fork PR awaiting maintainer approval
|
||||
# failure | any | failure | CI red
|
||||
#
|
||||
# Env in: EVAL, FAILED, FORK_NEEDS_E2E_LABEL (optional, default false)
|
||||
# Env in: EVAL, FAILED, FORK_NEEDS_E2E_APPROVAL (optional, default false)
|
||||
# Out: state, short_desc, long_desc on $GITHUB_OUTPUT
|
||||
|
||||
set -euo pipefail
|
||||
@@ -28,13 +30,14 @@ else
|
||||
fi
|
||||
|
||||
# Fork PRs never run e2e on their own: the fork `pull_request` run resolves to
|
||||
# an empty shard matrix, so the suite only runs once a maintainer applies the
|
||||
# `e2e-approved` label (which mirrors the head to a trusted fork-e2e/** branch).
|
||||
# Without it the e2e checks are satisfied-via-skip and the PR can go green with
|
||||
# e2e never having executed -- so nudge a maintainer to apply the label. Appended
|
||||
# to the comment only (long_desc); short_desc is the 140-char commit status.
|
||||
if [[ "${FORK_NEEDS_E2E_LABEL:-false}" == "true" ]]; then
|
||||
LONG="$LONG"$'\n\n:information_source: e2e tests do not run automatically on fork PRs. A maintainer can apply the `e2e-approved` label to run the full e2e suite against this PR.'
|
||||
# an empty shard matrix, so the suite only runs once a maintainer approves the
|
||||
# PR (which mirrors the head to a trusted fork-e2e/** branch). Without approval
|
||||
# the e2e checks are satisfied-via-skip and the PR would go green with e2e never
|
||||
# having executed -- so block merge until a maintainer approves.
|
||||
if [[ "${FORK_NEEDS_E2E_APPROVAL:-false}" == "true" ]]; then
|
||||
STATE=failure
|
||||
SHORT="Awaiting maintainer approval for e2e"
|
||||
LONG="$LONG"$'\n\n:no_entry: **E2e tests are required for fork PRs.** A maintainer must approve this PR or apply the `e2e-approved` label to trigger the e2e suite. The merge gate will stay red until e2e passes.'
|
||||
fi
|
||||
|
||||
# GitHub commit-status descriptions max out at 140 chars.
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
# Sourced by evaluate-checks.sh. The unit/lint/type-check checks gate every PR.
|
||||
# The e2e + e2e-ui suites also gate PRs, but only run with secrets on same-repo
|
||||
# PRs (maintainer branches); fork PRs cannot read the LLM_API_KEY /
|
||||
# GATEWAY_BASE_URL secrets, so their e2e jobs skip via a workflow fork guard.
|
||||
# The e2e and integration check names are therefore in BOTH REQUIRED (a
|
||||
# same-repo PR must pass them) and ALLOW_SKIP (a fork PR's skipped check still
|
||||
# satisfies the gate).
|
||||
# GATEWAY_BASE_URL secrets, so their e2e jobs skip via a workflow fork guard
|
||||
# until a maintainer approves the PR (which triggers the fork-e2e mirror).
|
||||
# The e2e and integration check names are in BOTH REQUIRED and ALLOW_SKIP so
|
||||
# that path-filtered same-repo PRs (e.g. ap-web-only) can skip them. However,
|
||||
# fork PRs must NOT skip e2e: FORK_NEVER_SKIP overrides ALLOW_SKIP when
|
||||
# IS_FORK=true, so a fork PR's missing e2e checks block merge.
|
||||
# Generated file -- do not hand-edit; it is replaced wholesale on every sync.
|
||||
|
||||
REQUIRED=(
|
||||
@@ -60,7 +62,31 @@ ALLOW_SKIP=(
|
||||
"Integration (codex)"
|
||||
)
|
||||
|
||||
is_allow_skip() { printf '%s\n' "${ALLOW_SKIP[@]}" | grep -qxF "$1"; }
|
||||
# Checks that must NOT be skipped on fork PRs. When IS_FORK=true,
|
||||
# is_allow_skip returns false for these even though they're in ALLOW_SKIP.
|
||||
# This ensures fork PRs can't merge with e2e/integration never having run.
|
||||
FORK_NEVER_SKIP=(
|
||||
"E2E Tests (shard 0/4)"
|
||||
"E2E Tests (shard 1/4)"
|
||||
"E2E Tests (shard 2/4)"
|
||||
"E2E Tests (shard 3/4)"
|
||||
"E2E UI Tests (shard 0/3)"
|
||||
"E2E UI Tests (shard 1/3)"
|
||||
"E2E UI Tests (shard 2/3)"
|
||||
"Integration (claude-sdk)"
|
||||
"Integration (openai-agents)"
|
||||
"Integration (codex)"
|
||||
)
|
||||
|
||||
is_allow_skip() {
|
||||
# Fork PRs: never skip e2e/integration checks.
|
||||
if [[ "${IS_FORK:-false}" == "true" ]]; then
|
||||
if printf '%s\n' "${FORK_NEVER_SKIP[@]}" | grep -qxF "$1"; then
|
||||
return 1
|
||||
fi
|
||||
fi
|
||||
printf '%s\n' "${ALLOW_SKIP[@]}" | grep -qxF "$1";
|
||||
}
|
||||
|
||||
# Maps an ALLOW_SKIP check to the workflow that produces it, so
|
||||
# evaluate-checks.sh can tell a genuine skip (a CI Pytest shard path-skip, or
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
# (FIRST_TIME_CONTRIBUTOR / NONE).
|
||||
#
|
||||
# This gate is independent of fork-e2e/should-mirror.sh: that one gates secret-
|
||||
# bearing e2e on the maintainer-applied `e2e-approved` label, whereas this gate
|
||||
# bearing e2e on a maintainer's approving PR review, whereas this gate
|
||||
# decides whether to inspect for attacks and so errs toward scanning more (it
|
||||
# scans returning CONTRIBUTORs that the label gate would not by itself run).
|
||||
#
|
||||
@@ -21,21 +21,29 @@
|
||||
# repo at event time; it is not attacker-settable from PR contents.
|
||||
#
|
||||
# Maintainer escape hatch: an untrusted PR can be waived by the
|
||||
# `skip-security-scan` label, but ONLY when the waiver is maintainer-effective
|
||||
# -- the label is present AND the author is a maintainer, or a maintainer's
|
||||
# latest decisive review is APPROVED. Same semantics as e2e-ui-required's
|
||||
# `skip-e2e-ui-test`: the label alone is not enough, so a fork
|
||||
# author cannot self-waive (applying labels needs triage access anyway, and the
|
||||
# extra maintainer check is defence in depth). All state is read from the API
|
||||
# (trusted), and this script always runs from `main`, so a PR cannot edit the
|
||||
# decision. The waiver is only evaluated when MAINTAINERS is passed (the scan
|
||||
# does; the per-workflow pollers do not -- they just mirror the scan's result).
|
||||
# `skip-security-scan` label alone. Applying a label requires GitHub Triage
|
||||
# permission (or higher), which a fork author never has, so the label IS the
|
||||
# maintainer gate and no separate approval is required.
|
||||
#
|
||||
# ACCEPTED RISK (repo policy, not GitHub-enforced): GitHub allows the Triage role
|
||||
# to be granted independently of Write, so in principle a triage-only collaborator
|
||||
# could self-waive. We accept this because this repo grants Triage only to
|
||||
# write/admin collaborators -- everyone who can apply the label can already push
|
||||
# code, so the waiver grants no privilege they don't already have. This invariant
|
||||
# lives in repo settings, not in code; if Triage is ever granted without Write,
|
||||
# revisit (e.g. re-add a maintainer-list check). See the PR for the full rationale.
|
||||
#
|
||||
# The label is read from the API (trusted), and this script always runs from
|
||||
# `main`, so a PR cannot edit the decision. The waiver is only evaluated when the
|
||||
# lookup vars (GH_TOKEN/REPO/PR) are passed (the scan does; the per-workflow
|
||||
# pollers do not -- they just mirror the scan's result).
|
||||
#
|
||||
# Env in: EVENT_NAME (github.event_name)
|
||||
# AUTHOR_ASSOCIATION (github.event.pull_request.author_association)
|
||||
# MAINTAINERS (space-separated, from merge-ready/load-maintainers.sh;
|
||||
# optional -- when empty the skip label is ignored)
|
||||
# GH_TOKEN, REPO, PR (for the waiver lookup; needed only with MAINTAINERS)
|
||||
# optional -- used only to trust private-membership
|
||||
# maintainer AUTHORS, not for the label waiver)
|
||||
# GH_TOKEN, REPO, PR (for the label lookup + author check)
|
||||
# Out: `scan=true|false` and `reason=<text>` on $GITHUB_OUTPUT.
|
||||
|
||||
set -euo pipefail
|
||||
@@ -48,49 +56,27 @@ emit() {
|
||||
echo "scan=$1 ($2)"
|
||||
}
|
||||
|
||||
# 0 = the skip label is present AND backed by a maintainer; 1 otherwise.
|
||||
# Mirrors e2e-ui-required/check.sh cases 3-4. Fails closed on any gap.
|
||||
skip_label_effective() {
|
||||
# 0 = the skip label is present; 1 otherwise. Label-only: applying the label
|
||||
# already requires Triage permission (or higher), so its mere presence is the
|
||||
# maintainer gate (see the accepted-risk note in the header). Fails closed on any
|
||||
# gap (missing token, etc).
|
||||
has_skip_label() {
|
||||
[[ -n "${GH_TOKEN:-}" && -n "${REPO:-}" && -n "${PR:-}" ]] || return 1
|
||||
[[ -n "${MAINTAINERS:-}" && -n "${MAINTAINERS// /}" ]] || return 1
|
||||
|
||||
local has_label
|
||||
has_label=$(gh api "repos/$REPO/pulls/$PR" \
|
||||
--jq "[.labels[].name] | index(\"$SKIP_LABEL\") != null" 2>/dev/null || echo "false")
|
||||
[[ "$has_label" == "true" ]] || return 1
|
||||
|
||||
local maint_lc author_lc approvers u_lc
|
||||
maint_lc=$(echo "$MAINTAINERS" | tr '[:upper:]' '[:lower:]')
|
||||
|
||||
# Author is a maintainer?
|
||||
author_lc=$(gh pr view "$PR" --repo "$REPO" --json author --jq '.author.login' 2>/dev/null \
|
||||
| tr '[:upper:]' '[:lower:]')
|
||||
for m in $maint_lc; do
|
||||
[[ "$m" == "$author_lc" ]] && return 0
|
||||
done
|
||||
|
||||
# A maintainer's latest decisive (non-COMMENTED) review is APPROVED?
|
||||
approvers=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate \
|
||||
--jq '[.[] | select(.state != "COMMENTED")] | group_by(.user.login) | map(max_by(.submitted_at)) | .[] | select(.state == "APPROVED") | .user.login' 2>/dev/null || echo "")
|
||||
for u in $approvers; do
|
||||
u_lc=$(echo "$u" | tr '[:upper:]' '[:lower:]')
|
||||
for m in $maint_lc; do
|
||||
[[ "$m" == "$u_lc" ]] && return 0
|
||||
done
|
||||
done
|
||||
|
||||
return 1
|
||||
[[ "$has_label" == "true" ]]
|
||||
}
|
||||
|
||||
# Only PRs carry untrusted contributor code through the gate. Every other
|
||||
# trigger -- push to main / fork-e2e/** (the mirror branch only exists after a
|
||||
# returning-contributor / maintainer-approval gate), schedule, dispatch -- is a
|
||||
# trusted context, so proceed without scanning. pull_request_review is included
|
||||
# for two reasons: (1) the security-scan workflow itself re-runs on review so a
|
||||
# maintainer's approval can complete a skip-security-scan waiver that was labeled
|
||||
# first; (2) the fork-e2e mirror fires on a maintainer's approval and must still
|
||||
# consult the head SHA's Security Scan. Both carry the same pull_request +
|
||||
# author_association fields, so the gate is evaluated identically.
|
||||
# trusted context, so proceed without scanning. pull_request_review is still
|
||||
# accepted (it carries the same pull_request + author_association fields, so the
|
||||
# gate evaluates identically) in case a workflow_call caller is wired to it, but
|
||||
# no workflow triggers a scan on review any more: the skip-security-scan waiver
|
||||
# is label-only, so the label event alone re-runs the scan and flips the check.
|
||||
case "${EVENT_NAME:-}" in
|
||||
pull_request | pull_request_target | pull_request_review) ;;
|
||||
*)
|
||||
@@ -127,8 +113,8 @@ case "${AUTHOR_ASSOCIATION:-}" in
|
||||
*)
|
||||
if author_is_maintainer; then
|
||||
emit false "trusted author (maintainer; author_association=${AUTHOR_ASSOCIATION:-unknown})"
|
||||
elif skip_label_effective; then
|
||||
emit false "maintainer-effective '$SKIP_LABEL' waiver"
|
||||
elif has_skip_label; then
|
||||
emit false "'$SKIP_LABEL' waiver (label requires a Triage+ collaborator to apply)"
|
||||
else
|
||||
emit true "untrusted author (author_association=${AUTHOR_ASSOCIATION:-unknown})"
|
||||
fi
|
||||
|
||||
@@ -9,8 +9,9 @@ name: E2E Tests
|
||||
# `parallelism` (pytest `-n` worker count).
|
||||
# pull_request PR gate for SAME-REPO PRs only. Fork PRs skip
|
||||
# here (no secrets) and run via the fork-e2e/**
|
||||
# push after fork-e2e-mirror.yml mirrors them. The
|
||||
# four shard checks are required by merge-ready.yml.
|
||||
# push after a maintainer approves the PR and
|
||||
# fork-e2e-mirror.yml mirrors them. The four shard
|
||||
# checks are required by merge-ready.yml.
|
||||
# push (fork-e2e/**) e2e run for mirrored fork PRs (trusted branch,
|
||||
# so secrets flow).
|
||||
|
||||
|
||||
@@ -4,16 +4,28 @@ name: Fork e2e mirror
|
||||
# there as a `push` (with secrets). It's a pure git-ref update via a GitHub App
|
||||
# token (refs pushed by the default GITHUB_TOKEN don't trigger workflows); it
|
||||
# never checks out or runs fork code. Mirroring requires BOTH the contributor
|
||||
# Security Scan to pass (the blocking `gate` job, via security-gate.yml) AND the
|
||||
# `e2e-approved` label, present and applied by a maintainer (should-mirror.sh).
|
||||
# Security Scan to pass (the blocking `gate` job, via security-gate.yml) AND a
|
||||
# maintainer's approving PR review (should-mirror.sh).
|
||||
#
|
||||
# The `e2e-approved` label is the sole human gate for running secret-bearing e2e
|
||||
# on a fork PR. Only Triage+ users can apply labels, and the gate further
|
||||
# verifies the labeler is in .github/MAINTAINER, so an external fork author can
|
||||
# never open it. It is intentionally separate from the merge gate
|
||||
# (maintainer-approval.yml): labeling runs e2e but does NOT approve for merge,
|
||||
# and vice-versa. Removing the label (or closing the PR) tears down the mirror
|
||||
# branch and stops further secret runs.
|
||||
# Maintainer approval is the sole human gate for running secret-bearing e2e on a
|
||||
# fork PR. Only users with write access can submit approving reviews, and the
|
||||
# gate further verifies the approver is in .github/MAINTAINER, so an external
|
||||
# fork author can never open it. It is intentionally tied to the merge gate
|
||||
# (maintainer-approval.yml): approving the PR runs e2e AND approves for merge.
|
||||
# Requesting changes or dismissing the review stops future mirrors; closing the
|
||||
# PR tears down the mirror branch.
|
||||
#
|
||||
# Triggers:
|
||||
# pull_request_target opened/synchronize/reopened/closed — handles new
|
||||
# pushes and PR lifecycle. Reviews don't fire
|
||||
# pull_request_target, so approval reaches here via
|
||||
# workflow_dispatch (dispatched by
|
||||
# maintainer-approval-rerun-run.yml on approval).
|
||||
# workflow_dispatch re-evaluation of a single PR (used by the approval
|
||||
# relay and for manual re-runs). Safe because
|
||||
# should-mirror.sh always re-checks approval before
|
||||
# any secret-bearing run; a spurious dispatch with an
|
||||
# arbitrary PR number cannot trigger e2e.
|
||||
#
|
||||
# leak-scan-allow: pull_request_target
|
||||
on:
|
||||
@@ -22,21 +34,31 @@ on:
|
||||
# down the mirror immediately, not only on the PR's next push.
|
||||
types: [opened, synchronize, reopened, closed, labeled, unlabeled]
|
||||
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
pr:
|
||||
description: PR number to evaluate for mirroring.
|
||||
required: true
|
||||
type: string
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
concurrency:
|
||||
group: fork-e2e-mirror-${{ github.event.pull_request.number }}
|
||||
group: fork-e2e-mirror-${{ github.event.pull_request.number || inputs.pr }}
|
||||
cancel-in-progress: false
|
||||
|
||||
jobs:
|
||||
# Delete the trusted mirror branch when the PR closes or the gate label is
|
||||
# removed. Ungated -- cleanup must always run so a closed PR (or one whose
|
||||
# approval was withdrawn) never leaves a stale fork-e2e/pr-N branch behind.
|
||||
# label was removed) never leaves a stale fork-e2e/pr-N branch behind.
|
||||
# Note: approval revocation cleanup is handled by the mirror job's
|
||||
# "Delete stale mirror branch on revocation" step (workflow_dispatch path).
|
||||
cleanup:
|
||||
name: cleanup
|
||||
if: >-
|
||||
github.event.pull_request.head.repo.fork
|
||||
github.event_name == 'pull_request_target'
|
||||
&& github.event.pull_request.head.repo.fork
|
||||
&& (
|
||||
github.event.action == 'closed'
|
||||
|| (github.event.action == 'unlabeled' && github.event.label.name == 'e2e-approved')
|
||||
@@ -67,41 +89,68 @@ jobs:
|
||||
# The single contributor Security Scan, consulted as a BLOCKING gate before we
|
||||
# mirror fork code onto a trusted branch where e2e runs WITH the gateway secret.
|
||||
# The scan itself runs once on the PR (security-scan.yml); this poller mirrors
|
||||
# its result, blocking the mirror on a finding. Skipped on the teardown actions
|
||||
# (handled by `cleanup`) and on label churn other than `e2e-approved`.
|
||||
# its result, blocking the mirror on a finding. Skipped on the teardown action
|
||||
# (handled by `cleanup`).
|
||||
gate:
|
||||
name: security gate
|
||||
if: >-
|
||||
github.event.pull_request.head.repo.fork
|
||||
&& github.event.action != 'closed'
|
||||
&& github.event.action != 'unlabeled'
|
||||
&& (github.event.action != 'labeled' || github.event.label.name == 'e2e-approved')
|
||||
(
|
||||
github.event_name == 'workflow_dispatch'
|
||||
) || (
|
||||
github.event_name == 'pull_request_target'
|
||||
&& github.event.pull_request.head.repo.fork
|
||||
&& github.event.action != 'closed'
|
||||
&& github.event.action != 'unlabeled'
|
||||
&& (github.event.action != 'labeled' || github.event.label.name == 'e2e-approved')
|
||||
)
|
||||
uses: ./.github/workflows/security-gate.yml
|
||||
|
||||
mirror:
|
||||
name: mirror
|
||||
needs: gate
|
||||
# Fork PRs only -- same-repo PRs run e2e directly via `pull_request`. Mirror
|
||||
# only when not tearing down and (for label events) only for the gate label.
|
||||
# Fork PRs only -- same-repo PRs run e2e directly via `pull_request`.
|
||||
# workflow_dispatch is validated at the step level (verify fork before
|
||||
# mirroring) but runs the gate unconditionally to keep the flow simple.
|
||||
if: >-
|
||||
github.event.pull_request.head.repo.fork
|
||||
&& github.event.action != 'closed'
|
||||
&& github.event.action != 'unlabeled'
|
||||
&& (github.event.action != 'labeled' || github.event.label.name == 'e2e-approved')
|
||||
(
|
||||
github.event_name == 'workflow_dispatch'
|
||||
) || (
|
||||
github.event_name == 'pull_request_target'
|
||||
&& github.event.pull_request.head.repo.fork
|
||||
&& github.event.action != 'closed'
|
||||
&& github.event.action != 'unlabeled'
|
||||
&& (github.event.action != 'labeled' || github.event.label.name == 'e2e-approved')
|
||||
)
|
||||
permissions:
|
||||
contents: read
|
||||
issues: read # read the labeled-by timeline (issues/N/events)
|
||||
pull-requests: read
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
PR: ${{ github.event.pull_request.number }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
MIRROR_BRANCH: fork-e2e/pr-${{ github.event.pull_request.number }}
|
||||
PR: ${{ github.event.pull_request.number || inputs.pr }}
|
||||
steps:
|
||||
- name: Resolve PR context
|
||||
id: ctx
|
||||
run: |
|
||||
if [[ -n "${{ github.event.pull_request.head.sha || '' }}" ]]; then
|
||||
echo "sha=${{ github.event.pull_request.head.sha }}" >> "$GITHUB_OUTPUT"
|
||||
echo "is_fork=true" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
# workflow_dispatch: resolve from the PR object.
|
||||
INFO=$(gh pr view "$PR" --repo "$REPO" --json headRefOid,isCrossRepository)
|
||||
SHA=$(echo "$INFO" | jq -r '.headRefOid')
|
||||
IS_FORK=$(echo "$INFO" | jq -r '.isCrossRepository')
|
||||
echo "sha=$SHA" >> "$GITHUB_OUTPUT"
|
||||
echo "is_fork=$IS_FORK" >> "$GITHUB_OUTPUT"
|
||||
if [[ "$IS_FORK" != "true" ]]; then
|
||||
echo "::notice::PR #$PR is same-repo; skipping mirror (same-repo PRs run e2e directly)."
|
||||
fi
|
||||
fi
|
||||
|
||||
- name: Check out gate scripts from main
|
||||
if: steps.ctx.outputs.is_fork == 'true'
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: main # trusted; never the PR head
|
||||
@@ -109,6 +158,7 @@ jobs:
|
||||
persist-credentials: false
|
||||
|
||||
- name: Mint mirror App token
|
||||
if: steps.ctx.outputs.is_fork == 'true'
|
||||
id: app-token
|
||||
# App token, not GITHUB_TOKEN: its pushes DO trigger the downstream e2e.
|
||||
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
|
||||
@@ -116,23 +166,26 @@ jobs:
|
||||
app-id: ${{ vars.FORK_E2E_APP_ID }}
|
||||
private-key: ${{ secrets.FORK_E2E_APP_PRIVATE_KEY }}
|
||||
|
||||
# MAINTAINER@main, never the PR head: the gate verifies the *labeler* is a
|
||||
# MAINTAINER@main, never the PR head: the gate verifies the *approver* is a
|
||||
# maintainer, so a PR can't self-grant by editing its own MAINTAINER copy.
|
||||
- name: Load maintainers
|
||||
if: steps.ctx.outputs.is_fork == 'true'
|
||||
id: maintainers
|
||||
run: bash .github/scripts/merge-ready/load-maintainers.sh
|
||||
|
||||
- name: Evaluate mirror gate
|
||||
if: steps.ctx.outputs.is_fork == 'true'
|
||||
id: gate
|
||||
env:
|
||||
LABEL: e2e-approved
|
||||
MAINTAINERS: ${{ steps.maintainers.outputs.list }}
|
||||
run: bash .github/scripts/fork-e2e/should-mirror.sh
|
||||
|
||||
- name: Mirror head SHA onto trusted branch
|
||||
if: ${{ steps.gate.outputs.mirror == 'true' }}
|
||||
if: steps.ctx.outputs.is_fork == 'true' && steps.gate.outputs.mirror == 'true'
|
||||
env:
|
||||
TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
HEAD_SHA: ${{ steps.ctx.outputs.sha }}
|
||||
MIRROR_BRANCH: fork-e2e/pr-${{ env.PR }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
# Move git OBJECTS, don't just point a ref. A fork PR's head commit
|
||||
@@ -159,3 +212,16 @@ jobs:
|
||||
fi
|
||||
git -C "$work" push -q -f "$origin" "${HEAD_SHA}:refs/heads/${MIRROR_BRANCH}"
|
||||
echo "Mirrored $MIRROR_BRANCH -> $HEAD_SHA"
|
||||
|
||||
# Tear down the mirror branch when approval is revoked (review dismissed
|
||||
# or changes requested). Without this, a stale fork-e2e/pr-N branch
|
||||
# would remain until the next push or PR close.
|
||||
- name: Delete stale mirror branch on revocation
|
||||
if: steps.ctx.outputs.is_fork == 'true' && steps.gate.outputs.mirror == 'false'
|
||||
env:
|
||||
GH_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
MIRROR_BRANCH: fork-e2e/pr-${{ env.PR }}
|
||||
run: |
|
||||
gh api -X DELETE "repos/$REPO/git/refs/heads/$MIRROR_BRANCH" >/dev/null 2>&1 \
|
||||
&& echo "Deleted stale $MIRROR_BRANCH (approval revoked)" \
|
||||
|| echo "No $MIRROR_BRANCH to delete"
|
||||
|
||||
@@ -80,3 +80,30 @@ jobs:
|
||||
core.info(`Re-running Maintainer Approval run ${run_id} for PR #${pull_number}`);
|
||||
await github.rest.actions.reRunWorkflowFailedJobs({ owner, repo, run_id: Number(run_id) });
|
||||
}
|
||||
|
||||
# Fork PRs: maintainer approval also gates e2e (replacing the old
|
||||
# e2e-approved label). Dispatch the fork-e2e-mirror workflow so the
|
||||
# approval triggers e2e on the trusted mirror branch.
|
||||
- name: Dispatch fork e2e mirror for fork PRs
|
||||
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
const { owner, repo } = context.repo;
|
||||
if (!fs.existsSync('pr_number')) {
|
||||
core.info('No pr_number file; nothing to do.');
|
||||
return;
|
||||
}
|
||||
const pull_number = Number(fs.readFileSync('pr_number', 'utf8').trim());
|
||||
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number });
|
||||
if (!pr.head.repo || pr.head.repo.full_name === `${owner}/${repo}`) {
|
||||
core.info(`PR #${pull_number} is same-repo; skipping fork-e2e-mirror dispatch.`);
|
||||
return;
|
||||
}
|
||||
core.info(`PR #${pull_number} is a fork PR; dispatching fork-e2e-mirror.`);
|
||||
await github.rest.actions.createWorkflowDispatch({
|
||||
owner, repo,
|
||||
workflow_id: 'fork-e2e-mirror.yml',
|
||||
ref: 'main',
|
||||
inputs: { pr: String(pull_number) },
|
||||
});
|
||||
|
||||
@@ -20,8 +20,10 @@ concurrency:
|
||||
|
||||
jobs:
|
||||
record:
|
||||
# Only approvals can flip the check green; skip everything else.
|
||||
if: github.event.review.state == 'approved'
|
||||
# Approvals flip the check green; dismissals and changes-requested flip
|
||||
# it red and revoke the fork-e2e mirror. Skip COMMENTED reviews (they
|
||||
# don't change review state).
|
||||
if: github.event.review.state != 'commented'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
|
||||
@@ -3,7 +3,7 @@ name: Merge Ready
|
||||
# Posts the "Merge Ready" commit status on the PR head SHA -- the single
|
||||
# required branch-protection check, backed by the REQUIRED list inside
|
||||
# this workflow. Triggers: `/merge` comment (write-access commenter only),
|
||||
# `pull_request` labeled (acts only with `automerge`),
|
||||
# `pull_request_target` labeled (acts only with `automerge`),
|
||||
# `workflow_run` on same-repo CI completion, `check_suite` completion on a
|
||||
# `fork-e2e/**` branch (the mirrored fork PR e2e -- a delivery that actually
|
||||
# fires, unlike the brittle fork-PR `workflow_run` hop it replaces), and
|
||||
@@ -24,7 +24,9 @@ name: Merge Ready
|
||||
on:
|
||||
# `labeled` only; `workflow_run` re-evaluates on CI completion (same-repo PRs
|
||||
# and the fork-e2e/** mirror push -- see the job `if`).
|
||||
pull_request:
|
||||
# pull_request_target (not pull_request) so this workflow always runs from
|
||||
# main -- a PR cannot modify the gate logic by editing this file.
|
||||
pull_request_target:
|
||||
types: [labeled]
|
||||
workflow_run:
|
||||
workflows: [PR Template, CI, Lint, E2E UI Tests, E2E Tests, Integration Tests]
|
||||
@@ -71,7 +73,7 @@ jobs:
|
||||
# open PR (push to main, etc.) are dropped by the ctx step.
|
||||
if: >-
|
||||
(
|
||||
github.event_name == 'pull_request' &&
|
||||
github.event_name == 'pull_request_target' &&
|
||||
github.event.label.name == 'automerge'
|
||||
) ||
|
||||
(
|
||||
@@ -106,6 +108,7 @@ jobs:
|
||||
- name: Check out scripts
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: main # trusted gate scripts; never the PR head
|
||||
sparse-checkout: .github/scripts/merge-ready
|
||||
persist-credentials: false
|
||||
|
||||
@@ -128,7 +131,7 @@ jobs:
|
||||
gh api "repos/$REPO/commits/$1/pulls" \
|
||||
--jq 'map(select(.state == "open")) | .[0].number // empty' 2>/dev/null || true
|
||||
}
|
||||
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
|
||||
if [[ "${{ github.event_name }}" == "pull_request_target" ]]; then
|
||||
PR="${{ github.event.pull_request.number }}"
|
||||
SHA="${{ github.event.pull_request.head.sha }}"
|
||||
elif [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then
|
||||
@@ -178,13 +181,22 @@ jobs:
|
||||
echo "pr=$PR" >> "$GITHUB_OUTPUT"
|
||||
echo "sha=$SHA" >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: Read PR labels
|
||||
- name: Load maintainers
|
||||
id: maintainers
|
||||
if: steps.ctx.outputs.skip != 'true'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
run: bash .github/scripts/merge-ready/load-maintainers.sh
|
||||
|
||||
- name: Read PR labels and fork approval state
|
||||
id: labels
|
||||
if: steps.ctx.outputs.skip != 'true'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
PR: ${{ steps.ctx.outputs.pr }}
|
||||
MAINTAINERS: ${{ steps.maintainers.outputs.list }}
|
||||
run: |
|
||||
INFO=$(gh pr view "$PR" --repo "$REPO" --json labels,isCrossRepository)
|
||||
NAMES=$(echo "$INFO" | jq -r '.labels[].name')
|
||||
@@ -193,15 +205,38 @@ jobs:
|
||||
else
|
||||
echo "automerge=false" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
# A fork PR without the maintainer-only `e2e-approved` label never
|
||||
# runs e2e (the fork pull_request run is an empty matrix), so the gate
|
||||
# message nudges a maintainer to apply it. Same-repo PRs run e2e with
|
||||
# secrets directly and need no label.
|
||||
if [[ "$(echo "$INFO" | jq -r '.isCrossRepository')" == "true" ]] \
|
||||
&& ! echo "$NAMES" | grep -qx "e2e-approved"; then
|
||||
echo "fork_needs_e2e_label=true" >> "$GITHUB_OUTPUT"
|
||||
# A fork PR without a maintainer's approving review or the
|
||||
# `e2e-approved` label never runs e2e (the fork pull_request run is
|
||||
# an empty matrix), so the gate blocks until one of these is present.
|
||||
# Same-repo PRs run e2e with secrets directly and need no gate.
|
||||
IS_FORK=$(echo "$INFO" | jq -r '.isCrossRepository')
|
||||
echo "is_fork=$IS_FORK" >> "$GITHUB_OUTPUT"
|
||||
if [[ "$IS_FORK" == "true" ]]; then
|
||||
# Check path 1: maintainer approval via PR review.
|
||||
MAINTAINERS_LC=$(echo "${MAINTAINERS:-}" | tr '[:upper:]' '[:lower:]')
|
||||
APPROVERS=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate \
|
||||
--jq '[.[] | select(.state != "COMMENTED")] | group_by(.user.login) | map(max_by(.submitted_at)) | .[] | select(.state == "APPROVED") | .user.login')
|
||||
HAS_GATE=false
|
||||
for u in $APPROVERS; do
|
||||
u_lc=$(echo "$u" | tr '[:upper:]' '[:lower:]')
|
||||
for m in $MAINTAINERS_LC; do
|
||||
if [[ "$m" == "$u_lc" ]]; then
|
||||
HAS_GATE=true
|
||||
break 2
|
||||
fi
|
||||
done
|
||||
done
|
||||
# Check path 2: e2e-approved label.
|
||||
if [[ "$HAS_GATE" == "false" ]] && echo "$NAMES" | grep -qx "e2e-approved"; then
|
||||
HAS_GATE=true
|
||||
fi
|
||||
if [[ "$HAS_GATE" == "false" ]]; then
|
||||
echo "fork_needs_e2e_approval=true" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "fork_needs_e2e_approval=false" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
else
|
||||
echo "fork_needs_e2e_label=false" >> "$GITHUB_OUTPUT"
|
||||
echo "fork_needs_e2e_approval=false" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
# post_red gates posting a red status: /merge needs it, automerge opts
|
||||
@@ -231,6 +266,7 @@ jobs:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
SHA: ${{ steps.ctx.outputs.sha }}
|
||||
IS_FORK: ${{ steps.labels.outputs.is_fork }}
|
||||
run: bash .github/scripts/merge-ready/evaluate-checks.sh
|
||||
|
||||
- name: Compute gate outcome
|
||||
@@ -241,7 +277,7 @@ jobs:
|
||||
env:
|
||||
EVAL: ${{ steps.eval.outcome }}
|
||||
FAILED: ${{ steps.eval.outputs.failed }}
|
||||
FORK_NEEDS_E2E_LABEL: ${{ steps.labels.outputs.fork_needs_e2e_label }}
|
||||
FORK_NEEDS_E2E_APPROVAL: ${{ steps.labels.outputs.fork_needs_e2e_approval }}
|
||||
run: bash .github/scripts/merge-ready/compute-gate.sh
|
||||
|
||||
# Skipped when post_red is false AND gate is red: leaves prior
|
||||
@@ -298,7 +334,7 @@ jobs:
|
||||
- name: Enable auto-merge on automerge label
|
||||
if: >-
|
||||
steps.ctx.outputs.skip != 'true' &&
|
||||
github.event_name == 'pull_request' &&
|
||||
github.event_name == 'pull_request_target' &&
|
||||
github.event.action == 'labeled' &&
|
||||
github.event.label.name == 'automerge'
|
||||
env:
|
||||
@@ -307,7 +343,7 @@ jobs:
|
||||
PR: ${{ steps.ctx.outputs.pr }}
|
||||
run: bash .github/scripts/merge-ready/enable-automerge-label.sh
|
||||
|
||||
# Not on pull_request-labeled: auto-merge was enabled in an earlier
|
||||
# Not on pull_request_target-labeled: auto-merge was enabled in an earlier
|
||||
# step there, so failing here would make the label look broken even
|
||||
# though it worked. Safe on workflow_run/check_suite/workflow_dispatch.
|
||||
- name: Fail job when gate is red
|
||||
|
||||
@@ -13,7 +13,7 @@ name: Polly Review Approval Dispatch
|
||||
# workflow_dispatch entry point) for that PR.
|
||||
#
|
||||
# Maintainer approval is the trust gate that authorizes spending the LLM gateway
|
||||
# secret on fork code -- the same model as the fork-e2e `e2e-approved` gate.
|
||||
# secret on fork code -- the same model as the fork-e2e maintainer-approval gate.
|
||||
# Polly itself never runs PR code: it reviews the diff fetched via the API from
|
||||
# a default-branch checkout.
|
||||
#
|
||||
|
||||
@@ -303,7 +303,9 @@ jobs:
|
||||
IMPORTANT: Your output will be posted directly as a PR comment. Output
|
||||
ONLY the final structured review — no coordination messages, no status
|
||||
updates about dispatching sub-agents, no "waiting for results" narration.
|
||||
Start your response with the review content itself.
|
||||
Begin your response with the exact marker <!-- POLLY_REVIEW_START -->
|
||||
on its own line, then the review content. Nothing before the marker
|
||||
will be shown.
|
||||
"""
|
||||
pathlib.Path("/tmp/review_prompt.txt").write_text(prompt)
|
||||
PYEOF
|
||||
@@ -328,6 +330,23 @@ jobs:
|
||||
| tee /tmp/polly_output.txt \
|
||||
|| { echo "::warning::Polly review exited non-zero"; cat polly-stderr.log; }
|
||||
|
||||
# Strip any sub-agent coordination preamble that leaks before
|
||||
# the actual review. Primary: look for the sentinel we asked the
|
||||
# model to emit. Fallback: first markdown heading or standalone
|
||||
# horizontal rule.
|
||||
python3 -c "
|
||||
import re, pathlib
|
||||
raw = pathlib.Path('/tmp/polly_output.txt').read_text()
|
||||
sentinel = '<!-- POLLY_REVIEW_START -->'
|
||||
idx = raw.find(sentinel)
|
||||
if idx >= 0:
|
||||
cleaned = raw[idx + len(sentinel):].lstrip('\n')
|
||||
else:
|
||||
m = re.search(r'^(#{1,6} |---\s*$)', raw, re.MULTILINE)
|
||||
cleaned = raw[m.start():] if m else raw
|
||||
pathlib.Path('/tmp/polly_output.txt').write_text(cleaned)
|
||||
"
|
||||
|
||||
# Use a collision-resistant random delimiter so model output
|
||||
# containing "REVIEW_EOF" cannot truncate the output.
|
||||
delim="REVIEW_$(openssl rand -hex 8)"
|
||||
@@ -336,10 +355,18 @@ jobs:
|
||||
head -c 61440 /tmp/polly_output.txt >> "$GITHUB_OUTPUT"
|
||||
echo "${delim}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: Mint App token
|
||||
id: app-token
|
||||
if: steps.trigger.outputs.skip != 'true' && steps.creds.outputs.available == 'true' && vars.OMNIGENT_BOT_APP_ID != ''
|
||||
uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
|
||||
with:
|
||||
app-id: ${{ vars.OMNIGENT_BOT_APP_ID }}
|
||||
private-key: ${{ secrets.OMNIGENT_BOT_APP_KEY }}
|
||||
|
||||
- name: Post review comment
|
||||
if: steps.polly.outputs.review_text != ''
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ steps.pr.outputs.pr_number }}
|
||||
REVIEW_TEXT: ${{ steps.polly.outputs.review_text }}
|
||||
|
||||
@@ -9,9 +9,19 @@ name: Rerun Security Gate Run
|
||||
# `Security Gate` job failed -- so a workflow that already self-triggered on the
|
||||
# label (ci/e2e trigger on `labeled` for force-all-tests etc.) is in-progress or
|
||||
# green and skipped, avoiding a double-run. fork-e2e-mirror is excluded: it is
|
||||
# e2e-approved-driven mirror plumbing with branch side effects, not a
|
||||
# approval-driven mirror plumbing with branch side effects, not a
|
||||
# gate-mirroring check.
|
||||
#
|
||||
# RACE GUARD: the label event fires this relay AND the Security Scan re-run
|
||||
# concurrently. Before re-running anything we WAIT for the Security Scan check on
|
||||
# the head SHA to settle and only proceed once it is passing. Otherwise we would
|
||||
# re-run gate workflows while the scan is still failing / not yet recreated --
|
||||
# they would just re-mirror a non-passing check and fail again, and (as seen on
|
||||
# PR #556) those re-runs left runs in-progress that the decisive relay could no
|
||||
# longer re-run ("could not re-run", GitHub rejects rerun of an in-flight run),
|
||||
# stranding stale failing checks. Waiting for scan success makes the relay
|
||||
# deterministic: every gate it re-runs polls an already-completed passing scan.
|
||||
#
|
||||
# The triggering run may have been initiated by an untrusted fork PR, so the
|
||||
# recorded artifact is treated as untrusted input (the PR number is GitHub-
|
||||
# provided, but it is still sanitised to digits). No PR code is checked out.
|
||||
@@ -34,7 +44,10 @@ jobs:
|
||||
name: Rerun Security Gate
|
||||
if: github.event.workflow_run.conclusion == 'success'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
# >= the race guard's max wait (~6 min, below) PLUS the artifact download and
|
||||
# the per-workflow rerun loop, so a slow Security Scan can never cancel the
|
||||
# job mid-wait and strand the gate re-runs this relay exists to issue.
|
||||
timeout-minutes: 10
|
||||
permissions:
|
||||
actions: write # gh run rerun + read workflow runs/artifacts
|
||||
pull-requests: read # resolve the PR head SHA
|
||||
@@ -80,6 +93,43 @@ jobs:
|
||||
SHA="$(gh api "repos/$REPO/pulls/$PR_NUMBER" --jq '.head.sha')"
|
||||
echo "PR #$PR_NUMBER head $SHA"
|
||||
|
||||
# Race guard: re-running gate workflows is only useful once the
|
||||
# Security Scan has actually flipped to passing for this SHA. The
|
||||
# label event triggers this relay AND the scan re-run together, so wait
|
||||
# for the latest Security Scan check to complete; bail unless it passed.
|
||||
# (A non-passing scan means the gate failures are correct -- nothing to
|
||||
# re-run; and re-running now would strand in-progress runs the relay
|
||||
# can't later re-run. See the header.)
|
||||
#
|
||||
# KNOWN GAP: if the scan takes longer than this ~6-min budget, we exit
|
||||
# without re-running and the gates stay red until the next label event
|
||||
# (add/remove/re-add re-fires this relay). CI/E2E also self-recover via
|
||||
# their own `labeled` trigger. Acceptable: scans settle well under this.
|
||||
#
|
||||
# status + conclusion come from ONE response (sorted by id, monotonic)
|
||||
# so the two fields can't be read from different snapshots of "latest".
|
||||
echo "Waiting for the Security Scan check on $SHA to settle..."
|
||||
scan_q='[.check_runs[] | select(.name=="Security Scan")] | sort_by(.id) | last'
|
||||
scan_conclusion=""
|
||||
for _ in $(seq 1 72); do # up to ~6 min (72 * 5s)
|
||||
read -r scan_status scan_concl < <(
|
||||
gh api "repos/$REPO/commits/$SHA/check-runs" \
|
||||
--jq "$scan_q | \"\(.status // \"none\") \(.conclusion // \"none\")\"" 2>/dev/null || echo "")
|
||||
if [ "${scan_status:-}" = "completed" ]; then
|
||||
scan_conclusion="$scan_concl"
|
||||
break
|
||||
fi
|
||||
sleep 5
|
||||
done
|
||||
case "$scan_conclusion" in
|
||||
success | skipped | neutral)
|
||||
echo "Security Scan is '$scan_conclusion' -- proceeding to re-run failed gates." ;;
|
||||
"")
|
||||
echo "Security Scan did not complete in time; nothing to re-run."; exit 0 ;;
|
||||
*)
|
||||
echo "Security Scan is '$scan_conclusion' (not passing); gate failures are correct -- nothing to re-run."; exit 0 ;;
|
||||
esac
|
||||
|
||||
# Every workflow whose first job is the reusable Security Gate. We
|
||||
# re-run one only when its LATEST run for this SHA is a completed
|
||||
# gate-failure (below), so a workflow that already re-ran via its own
|
||||
|
||||
@@ -2,28 +2,25 @@ name: Rerun Security Gate
|
||||
|
||||
# Stage 1 of a two-stage relay (the privileged half is rerun-security-gate-run.yml).
|
||||
#
|
||||
# When a skip-security-scan waiver could change verdict -- the label is added or
|
||||
# removed, or a review is submitted/dismissed -- the per-workflow `Security Gate`
|
||||
# pollers must re-run so they re-mirror the (now-flipped) single `Security Scan`
|
||||
# check. Re-running another workflow needs `actions: write`, but on a FORK PR the
|
||||
# `pull_request_review` token is read-only and held behind the fork-approval gate,
|
||||
# so it cannot re-run anything itself (see maintainer-approval-rerun.yml, which
|
||||
# solves the identical problem the same way). So this stage only RECORDS the PR
|
||||
# number as an artifact (read-only, works on forks); the privileged re-run runs in
|
||||
# When the skip-security-scan waiver could change verdict -- the label is added
|
||||
# or removed -- the per-workflow `Security Gate` pollers must re-run so they
|
||||
# re-mirror the (now-flipped) single `Security Scan` check. Re-running another
|
||||
# workflow needs `actions: write`, but on a FORK PR the `pull_request_target`
|
||||
# token is held behind the fork-approval gate, so it cannot re-run anything
|
||||
# itself (see maintainer-approval-rerun.yml, which solves the identical problem
|
||||
# the same way). So this stage only RECORDS the PR number as an artifact
|
||||
# (read-only, works on forks); the privileged re-run runs in
|
||||
# rerun-security-gate-run.yml on `workflow_run`, which gets a writable token even
|
||||
# for forks and is not held behind the fork-approval gate.
|
||||
# https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
|
||||
#
|
||||
# Triggers (the two halves of the maintainer waiver):
|
||||
# - skip-security-scan labeled/unlabeled -> the label half changed
|
||||
# - a review submitted/dismissed -> the approval half changed
|
||||
# Trigger: skip-security-scan labeled/unlabeled is the ONLY thing that can flip
|
||||
# the waiver (it is label-only -- see should-scan.sh; there is no approval half).
|
||||
# Other labels are ignored by the job `if:` below (stage 2 then no-ops).
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [labeled, unlabeled]
|
||||
pull_request_review:
|
||||
types: [submitted, dismissed]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
@@ -39,14 +36,8 @@ jobs:
|
||||
record:
|
||||
name: Record PR for gate re-run
|
||||
# Only when the waiver state could have changed: the skip-security-scan
|
||||
# label was added/removed, or a DECISIVE review changed. A plain `commented`
|
||||
# review can't flip the waiver -- should-scan.sh keys on the latest
|
||||
# non-COMMENTED review -- so skip it; `approved`/`changes_requested` (and a
|
||||
# `dismissed` event, whose review.state is `dismissed`) all can, so they
|
||||
# pass. Unrelated labels record nothing, so stage 2 no-ops.
|
||||
if: >-
|
||||
(github.event_name == 'pull_request_review' && github.event.review.state != 'commented') ||
|
||||
github.event.label.name == 'skip-security-scan'
|
||||
# label was added/removed. Unrelated labels record nothing, so stage 2 no-ops.
|
||||
if: github.event.label.name == 'skip-security-scan'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
|
||||
@@ -61,11 +61,11 @@ jobs:
|
||||
# workflow RUN with conclusion=action_required and NO check-run, so the
|
||||
# poll below never sees it and spins the full ~6 min before failing
|
||||
# open. Detect the held state and proceed now (same fail-open outcome);
|
||||
# the gate re-runs on the next push or e2e-approved label event.
|
||||
# the gate re-runs on the next push or maintainer approval event.
|
||||
held=$(gh api "repos/$REPO/actions/runs?head_sha=$HEAD_SHA&event=pull_request" \
|
||||
--jq '[.workflow_runs[] | select(.name=="Security Scan")] | sort_by(.created_at) | last | .conclusion' 2>/dev/null || echo "")
|
||||
if [ "$held" = "action_required" ]; then
|
||||
echo "::warning::Security Scan is awaiting maintainer approval (action_required); proceeding (fail-open). It will re-gate on the next push or the e2e-approved label event."
|
||||
echo "::warning::Security Scan is awaiting maintainer approval (action_required); proceeding (fail-open). It will re-gate on the next push or maintainer approval event."
|
||||
exit 0
|
||||
fi
|
||||
q='[.check_runs[] | select(.name=="Security Scan")] | sort_by(.started_at) | last'
|
||||
|
||||
@@ -22,22 +22,16 @@ name: Security Scan
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
# labeled/unlabeled so applying or removing the maintainer skip label
|
||||
# (skip-security-scan) re-runs the scan and flips this check.
|
||||
# labeled/unlabeled so applying or removing the skip label
|
||||
# (skip-security-scan) re-runs the scan and flips this check. The waiver is
|
||||
# label-only (should-scan.sh): applying it needs Triage permission, so the
|
||||
# label alone is the maintainer gate -- no separate approval, hence no
|
||||
# pull_request_review trigger.
|
||||
types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled]
|
||||
# A maintainer's approval is the OTHER half of the skip-security-scan waiver
|
||||
# (the label alone is not maintainer-effective -- see should-scan.sh). Without
|
||||
# this trigger a PR that is labeled FIRST and approved LATER never re-runs, so
|
||||
# the stale failing check sticks. `submitted` flips the check once a maintainer
|
||||
# approves; `dismissed` re-gates if that approval is later removed. should-scan.sh
|
||||
# already accepts the pull_request_review payload (same pull_request +
|
||||
# author_association fields), so no script change is needed.
|
||||
pull_request_review:
|
||||
types: [submitted, dismissed]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read # read PR labels + reviews for the maintainer skip waiver
|
||||
pull-requests: read # read PR labels for the skip waiver
|
||||
|
||||
concurrency:
|
||||
group: security-scan-${{ github.event.pull_request.number }}
|
||||
@@ -74,7 +68,7 @@ jobs:
|
||||
env:
|
||||
EVENT_NAME: ${{ github.event_name }}
|
||||
AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }}
|
||||
# For the maintainer-effective skip-security-scan waiver (read-only).
|
||||
# For the skip-security-scan label waiver + author check (read-only).
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
REPO: ${{ github.repository }}
|
||||
PR: ${{ github.event.pull_request.number }}
|
||||
@@ -162,14 +156,14 @@ jobs:
|
||||
|
||||
# Surfaced on ANY detector failure above (sensitive-path / secret / exfil
|
||||
# / workflow-misuse / semgrep): the detectors say WHAT they found; this
|
||||
# says HOW a maintainer can waive it. The waiver needs BOTH a maintainer
|
||||
# approval AND the label -- the label alone is not maintainer-effective
|
||||
# (see should-scan.sh). Either action re-runs this scan via the labeled /
|
||||
# pull_request_review triggers above.
|
||||
# says HOW a maintainer can waive it. The waiver is label-only: applying
|
||||
# the 'skip-security-scan' label needs Triage permission, so the label is
|
||||
# itself the maintainer gate (see should-scan.sh). Applying it re-runs this
|
||||
# scan via the labeled trigger above.
|
||||
- name: Explain the maintainer waiver (on failure)
|
||||
if: ${{ failure() }}
|
||||
run: |
|
||||
MSG="A maintainer can skip the Security Scan by approving this PR AND applying the 'skip-security-scan' label (the label alone is not enough -- the author must be a maintainer or a maintainer must have approved). Either action re-runs this scan automatically."
|
||||
MSG="A maintainer can skip the Security Scan by applying the 'skip-security-scan' label (this requires Triage permission, so a fork author cannot self-waive). Applying the label re-runs this scan automatically."
|
||||
echo "::error::$MSG"
|
||||
{
|
||||
echo "### Security Scan failed"
|
||||
|
||||
+8
-4
@@ -8255,13 +8255,14 @@ def _set_antigravity_api_key() -> str | None:
|
||||
Offers an existing ``GEMINI_API_KEY`` / ``ANTIGRAVITY_API_KEY`` first
|
||||
(recorded as an ``env:`` ref, so the secret stays in the environment), else
|
||||
reads it with a hidden prompt and stores it under ``keychain:antigravity``.
|
||||
The ``AIza`` prefix is checked softly (a wrong paste is caught but can be
|
||||
forced). The key is never echoed.
|
||||
The key prefix (``AIza`` or ``AQ``) is checked softly (a wrong paste is
|
||||
caught but can be forced). The key is never echoed.
|
||||
|
||||
:returns: A status string for the menu, or ``None`` if the user aborted.
|
||||
"""
|
||||
from omnigent.onboarding import secrets as secret_store
|
||||
from omnigent.onboarding.antigravity_auth import (
|
||||
ANTIGRAVITY_API_KEY_PREFIX_HINT,
|
||||
ANTIGRAVITY_ENV_VARS,
|
||||
ANTIGRAVITY_SECRET_NAME,
|
||||
antigravity_api_key_settings,
|
||||
@@ -8275,7 +8276,9 @@ def _set_antigravity_api_key() -> str | None:
|
||||
):
|
||||
detected = os.environ[detected_var]
|
||||
if not looks_like_gemini_api_key(detected) and not click.confirm(
|
||||
f"${detected_var} doesn't start with 'AIza'. Use it anyway?", default=False
|
||||
f"${detected_var} doesn't start with {ANTIGRAVITY_API_KEY_PREFIX_HINT}. "
|
||||
"Use it anyway?",
|
||||
default=False,
|
||||
):
|
||||
return None
|
||||
_save_global_config(antigravity_api_key_settings(f"env:{detected_var}"))
|
||||
@@ -8285,7 +8288,8 @@ def _set_antigravity_api_key() -> str | None:
|
||||
if not pasted:
|
||||
return None
|
||||
if not looks_like_gemini_api_key(pasted) and not click.confirm(
|
||||
"That doesn't start with 'AIza'. Store it anyway?", default=False
|
||||
f"That doesn't start with {ANTIGRAVITY_API_KEY_PREFIX_HINT}. Store it anyway?",
|
||||
default=False,
|
||||
):
|
||||
return None
|
||||
secret_store.store_secret(ANTIGRAVITY_SECRET_NAME, pasted)
|
||||
|
||||
@@ -9,6 +9,14 @@ assistant text → :class:`TextChunk`, thinking → :class:`ReasoningChunk`,
|
||||
tool calls → :class:`ToolCallRequest` / :class:`ToolCallComplete`, completing
|
||||
on the run's terminal :class:`cursor_sdk.RunResult`.
|
||||
|
||||
Policy enforcement covers three phases: PHASE_LLM_REQUEST (pre-send),
|
||||
PHASE_LLM_RESPONSE (post-response), and PHASE_TOOL_CALL (native tools).
|
||||
Cursor's native tools execute inside the Cursor process so they cannot be
|
||||
pre-blocked, but when a non-bridged tool call is observed the executor
|
||||
evaluates PHASE_TOOL_CALL and cancels the run on DENY. Bridged Omnigent
|
||||
tools (MCP-wrapped) are already gated server-side via the dispatch bridge
|
||||
and are skipped to avoid double evaluation.
|
||||
|
||||
Crucially, Omnigent's spec-declared tools (``sys_session_send`` et al.) are
|
||||
bridged into Cursor **in-process** via the SDK's ``custom_tools``: each
|
||||
:class:`~omnigent.inner.executor.ToolSpec` becomes a ``cursor_sdk.CustomTool``
|
||||
@@ -34,6 +42,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import concurrent.futures
|
||||
import contextlib
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
@@ -123,9 +132,22 @@ def _normalize_cursor_usage(raw: dict[str, Any], model: str) -> dict[str, Any]:
|
||||
"model": model,
|
||||
}
|
||||
# Carry cache breakdown if the backend reports it.
|
||||
# The Cursor backend sends cacheReadTokens / cacheWriteTokens;
|
||||
# map to the Omnigent-standard cache_read_input_tokens /
|
||||
# cache_creation_input_tokens names.
|
||||
for dst, *sources in (
|
||||
("cache_read_input_tokens", "cacheReadInputTokens", "cache_read_input_tokens"),
|
||||
("cache_creation_input_tokens", "cacheCreationInputTokens", "cache_creation_input_tokens"),
|
||||
(
|
||||
"cache_read_input_tokens",
|
||||
"cacheReadTokens",
|
||||
"cacheReadInputTokens",
|
||||
"cache_read_input_tokens",
|
||||
),
|
||||
(
|
||||
"cache_creation_input_tokens",
|
||||
"cacheWriteTokens",
|
||||
"cacheCreationInputTokens",
|
||||
"cache_creation_input_tokens",
|
||||
),
|
||||
):
|
||||
for src in sources:
|
||||
val = raw.get(src)
|
||||
@@ -260,9 +282,16 @@ def _sdk_message_to_events(message: Any) -> list[ExecutorEvent]: # type: ignore
|
||||
name = str(args.get("toolName") or name)
|
||||
inner = args.get("args")
|
||||
args = inner if isinstance(inner, dict) else {}
|
||||
is_bridged = True
|
||||
else:
|
||||
is_bridged = False
|
||||
call_id = getattr(message, "call_id", None)
|
||||
if status == "running":
|
||||
events.append(ToolCallRequest(name=name, args=args, metadata={"call_id": call_id}))
|
||||
events.append(
|
||||
ToolCallRequest(
|
||||
name=name, args=args, metadata={"call_id": call_id, "is_bridged": is_bridged}
|
||||
)
|
||||
)
|
||||
elif status in ("completed", "error"):
|
||||
result = getattr(message, "result", None)
|
||||
classification = classify_tool_result(result)
|
||||
@@ -277,7 +306,7 @@ def _sdk_message_to_events(message: Any) -> list[ExecutorEvent]: # type: ignore
|
||||
status=tool_status,
|
||||
result=result,
|
||||
error=error,
|
||||
metadata={"call_id": call_id},
|
||||
metadata={"call_id": call_id, "is_bridged": is_bridged},
|
||||
)
|
||||
)
|
||||
return events
|
||||
@@ -377,9 +406,10 @@ class CursorExecutor(Executor):
|
||||
# Installed by the runtime adapter; routes a bridged-tool call back into
|
||||
# Omnigent's session (policy gating, sub-agent dispatch, logging).
|
||||
self._tool_executor: ToolExecutor | None = None
|
||||
# Installed by the runtime adapter; evaluates PHASE_LLM_REQUEST /
|
||||
# PHASE_LLM_RESPONSE policies (the same round-trip pi / claude-sdk use).
|
||||
# ``None`` on single-process / pre-turn paths (then policy is a no-op).
|
||||
# Installed by the runtime adapter; evaluates PHASE_LLM_REQUEST,
|
||||
# PHASE_LLM_RESPONSE, and PHASE_TOOL_CALL policies (the same round-trip
|
||||
# pi / claude-sdk use). ``None`` on single-process / pre-turn paths
|
||||
# (then policy is a no-op).
|
||||
self._policy_evaluator: Callable[[str, dict[str, Any]], Awaitable[Any]] | None = None
|
||||
|
||||
def supports_streaming(self) -> bool:
|
||||
@@ -409,6 +439,24 @@ class CursorExecutor(Executor):
|
||||
return str(meta["session_id"])
|
||||
return "__default__"
|
||||
|
||||
# -- native-tool policy gate --------------------------------------------
|
||||
|
||||
async def _evaluate_native_tool_policy(
|
||||
self, name: str, args: dict[str, Any]
|
||||
) -> dict[str, Any]:
|
||||
"""Evaluate PHASE_TOOL_CALL policy for a Cursor native tool.
|
||||
|
||||
Returns ``{"block": bool, "reason": str}``.
|
||||
"""
|
||||
evaluator = self._policy_evaluator
|
||||
if evaluator is None:
|
||||
return {"block": False, "reason": ""}
|
||||
verdict = await evaluator("PHASE_TOOL_CALL", {"name": name, "arguments": args})
|
||||
action = getattr(verdict, "action", None)
|
||||
if action == "POLICY_ACTION_DENY":
|
||||
return {"block": True, "reason": getattr(verdict, "reason", "") or "blocked by policy"}
|
||||
return {"block": False, "reason": ""}
|
||||
|
||||
# -- custom-tool bridge -------------------------------------------------
|
||||
|
||||
def _make_custom_tools(
|
||||
@@ -589,7 +637,19 @@ class CursorExecutor(Executor):
|
||||
separate_next_text = False
|
||||
turn_usage: dict[str, Any] | None = None
|
||||
try:
|
||||
run = await state.agent.send(prompt)
|
||||
# Pass SendOptions with on_delta so the backend sends
|
||||
# interaction updates (including TurnEndedUpdate with usage).
|
||||
# Without enableDeltas the backend omits them entirely.
|
||||
from cursor_sdk import SendOptions as _SendOptions # lazy: optional dep
|
||||
|
||||
def _capture_delta(update: Any) -> None: # type: ignore[explicit-any]
|
||||
nonlocal turn_usage
|
||||
if getattr(update, "type", None) == "turn-ended":
|
||||
raw = getattr(update, "usage", None)
|
||||
if isinstance(raw, dict) and raw:
|
||||
turn_usage = _normalize_cursor_usage(raw, model)
|
||||
|
||||
run = await state.agent.send(prompt, options=_SendOptions(on_delta=_capture_delta))
|
||||
async for stream_event in run.events():
|
||||
if stream_event.sdk_message is not None:
|
||||
for event in _sdk_message_to_events(stream_event.sdk_message):
|
||||
@@ -609,6 +669,23 @@ class CursorExecutor(Executor):
|
||||
elif isinstance(event, ToolCallRequest):
|
||||
tool_calls += 1
|
||||
separate_next_text = True
|
||||
# Evaluate PHASE_TOOL_CALL for native tools.
|
||||
# Bridged tools are already gated by the
|
||||
# dispatch bridge — skip to avoid double eval.
|
||||
if not event.metadata.get("is_bridged") and policy_eval is not None:
|
||||
gate = await self._evaluate_native_tool_policy(
|
||||
event.name, event.args if isinstance(event.args, dict) else {}
|
||||
)
|
||||
if gate["block"]:
|
||||
# Cancel the run to prevent further
|
||||
# native tool use.
|
||||
with contextlib.suppress(Exception):
|
||||
await run.cancel()
|
||||
yield event # emit so observers see what was attempted
|
||||
reason = gate["reason"]
|
||||
msg = f"Native tool {event.name!r} denied by policy: {reason}"
|
||||
yield ExecutorError(message=msg)
|
||||
return
|
||||
elif isinstance(event, ToolCallComplete):
|
||||
separate_next_text = True
|
||||
yield event
|
||||
|
||||
@@ -438,10 +438,22 @@ def _get_openai_async_client(
|
||||
# Checked before profile and env-var lookups so the spec is self-contained.
|
||||
# base_url_override is populated from HARNESS_OPENAI_AGENTS_GATEWAY_BASE_URL
|
||||
# when the spec also declares executor.auth.base_url.
|
||||
#
|
||||
# Fall back to the ambient OPENAI_BASE_URL when no override reached us.
|
||||
# The api_key is frequently a gateway credential (e.g. a Databricks AI
|
||||
# Gateway PAT detected from OPENAI_API_KEY), and the companion base_url
|
||||
# can be dropped anywhere on the daemon → runner → harness propagation
|
||||
# chain (the spec auth bake omits it when OPENAI_BASE_URL is absent at
|
||||
# materialization time; a reused local daemon may predate the env var).
|
||||
# Without this fallback, a missing base_url silently routes the gateway
|
||||
# token to api.openai.com and every request 401s; honoring the ambient
|
||||
# OPENAI_BASE_URL the runner inherits keeps the gateway target present on
|
||||
# every turn even when the override is lost. base_url=None still defaults
|
||||
# to api.openai.com for a genuine OpenAI key with no gateway configured.
|
||||
if api_key and api_key.strip():
|
||||
return AsyncOpenAI(
|
||||
api_key=api_key,
|
||||
base_url=base_url_override or None,
|
||||
base_url=base_url_override or os.environ.get("OPENAI_BASE_URL") or None,
|
||||
**retry_kwargs,
|
||||
)
|
||||
|
||||
|
||||
@@ -99,19 +99,24 @@ _API_KEY_FIELD = "api_key"
|
||||
# spawn-env builder falls back to them when no key is configured.
|
||||
ANTIGRAVITY_ENV_VARS: tuple[str, ...] = ("GEMINI_API_KEY", "ANTIGRAVITY_API_KEY")
|
||||
|
||||
# Gemini / Google API-key prefix (e.g. ``AIzaSy…``). Used for a *soft* paste
|
||||
# Gemini / Google API-key prefixes. Legacy keys start with ``AIza`` (e.g.
|
||||
# ``AIzaSy…``); newer Google API keys start with ``AQ``. Used for a *soft* paste
|
||||
# check — a non-matching key may be forced through, so a prefix change never
|
||||
# locks anyone out.
|
||||
ANTIGRAVITY_API_KEY_PREFIX = "AIza"
|
||||
ANTIGRAVITY_API_KEY_PREFIXES = ("AIza", "AQ")
|
||||
|
||||
# Human-readable form of the accepted prefixes, e.g. ``'AIza' or 'AQ'``.
|
||||
ANTIGRAVITY_API_KEY_PREFIX_HINT = " or ".join(f"'{p}'" for p in ANTIGRAVITY_API_KEY_PREFIXES)
|
||||
|
||||
|
||||
def looks_like_gemini_api_key(value: str) -> bool:
|
||||
"""Return whether *value* looks like a Gemini / Google API key.
|
||||
|
||||
:param value: A pasted candidate, e.g. ``"AIzaSyAbC123"``.
|
||||
:returns: ``True`` when it starts with :data:`ANTIGRAVITY_API_KEY_PREFIX`.
|
||||
:param value: A pasted candidate, e.g. ``"AIzaSyAbC123"`` or ``"AQ…"``.
|
||||
:returns: ``True`` when it starts with one of
|
||||
:data:`ANTIGRAVITY_API_KEY_PREFIXES`.
|
||||
"""
|
||||
return value.startswith(ANTIGRAVITY_API_KEY_PREFIX)
|
||||
return value.startswith(ANTIGRAVITY_API_KEY_PREFIXES)
|
||||
|
||||
|
||||
def antigravity_api_key_ref(config: dict[str, object] | None = None) -> str | None:
|
||||
|
||||
@@ -13,21 +13,20 @@ Unlike ``test_policies_e2e.py`` (polling API, background=True),
|
||||
this test drives the REPL through the actual streaming code
|
||||
path — the code path a human types into at the terminal.
|
||||
|
||||
All 14 tests run against the mock LLM server: ``OPENAI_BASE_URL``
|
||||
is injected into the REPL subprocess's environment so the inner
|
||||
OpenAI harness routes to the mock server. Each test pre-configures
|
||||
the mock's keyed response queue before spawning the subprocess.
|
||||
|
||||
Prerequisites:
|
||||
- ``pexpect`` installed (4.9+).
|
||||
- ``--llm-api-key`` pytest option set to a valid key for
|
||||
``openai/gpt-4o``.
|
||||
- ``ap`` on ``PATH`` resolving to this worktree's entry
|
||||
point (set ``PYTHONPATH`` so the editable install from
|
||||
a sibling worktree doesn't shadow it).
|
||||
|
||||
Usage::
|
||||
|
||||
PYTHONPATH=/home/ubuntu/omnigent-policies:\\
|
||||
/home/ubuntu/omnigent-policies/sdks/python-client:\\
|
||||
/home/ubuntu/omnigent-policies/sdks/frontend \\
|
||||
python -m pytest tests/e2e/test_repl_approval_e2e.py \\
|
||||
--llm-api-key $(cat /tmp/mykey) -v
|
||||
python -m pytest tests/e2e/test_repl_approval_e2e.py -v --timeout=180 --no-skip-known
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -42,6 +41,8 @@ from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from tests.e2e.conftest import configure_mock_llm, reset_mock_llm
|
||||
|
||||
pexpect = pytest.importorskip("pexpect")
|
||||
|
||||
_ASK_DEMO_DIR = Path(__file__).resolve().parents[1] / "resources" / "agents" / "ask-demo"
|
||||
@@ -71,7 +72,11 @@ def _strip_ansi(text: str) -> str:
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def repl_env(llm_api_key: str, tmp_path_factory: pytest.TempPathFactory) -> dict[str, str]:
|
||||
def repl_env(
|
||||
llm_api_key: str,
|
||||
mock_llm_server_url: str,
|
||||
tmp_path_factory: pytest.TempPathFactory,
|
||||
) -> dict[str, str]:
|
||||
"""
|
||||
Build the env dict for ``omnigent chat`` — OPENAI_API_KEY plus
|
||||
whatever PYTHONPATH the outer shell already provides (so
|
||||
@@ -99,7 +104,14 @@ def repl_env(llm_api_key: str, tmp_path_factory: pytest.TempPathFactory) -> dict
|
||||
resolve, and ``OMNIGENT_SKIP_ONBOARD`` guards against any other
|
||||
first-run prompt (these tests exercise REPL approval, not onboarding).
|
||||
|
||||
:param llm_api_key: The API key for the LLM.
|
||||
``OPENAI_BASE_URL`` is pointed at the session-scoped mock LLM
|
||||
server so the REPL subprocess's inner OpenAI harness routes all
|
||||
completions through the mock instead of hitting ``api.openai.com``.
|
||||
|
||||
:param llm_api_key: The API key for the LLM (``"mock-key"`` in
|
||||
mock mode).
|
||||
:param mock_llm_server_url: Base URL of the mock LLM server,
|
||||
e.g. ``"http://127.0.0.1:12345"``.
|
||||
:param tmp_path_factory: Pytest temp-path factory for the fake HOME.
|
||||
:returns: Env mapping for ``pexpect.spawn``.
|
||||
"""
|
||||
@@ -113,6 +125,9 @@ def repl_env(llm_api_key: str, tmp_path_factory: pytest.TempPathFactory) -> dict
|
||||
env: dict[str, str] = {
|
||||
**os.environ,
|
||||
"OPENAI_API_KEY": llm_api_key,
|
||||
# Point the inner OpenAI harness at the mock LLM server.
|
||||
# The SDK appends /responses to the base URL, so include /v1.
|
||||
"OPENAI_BASE_URL": f"{mock_llm_server_url}/v1",
|
||||
"HOME": str(fake_home),
|
||||
"OMNIGENT_CONFIG_HOME": str(config_home),
|
||||
"DATABRICKS_CONFIG_FILE": str(real_databrickscfg),
|
||||
@@ -128,6 +143,53 @@ def repl_env(llm_api_key: str, tmp_path_factory: pytest.TempPathFactory) -> dict
|
||||
return env
|
||||
|
||||
|
||||
def _configure_mock_text(mock_llm_server_url: str, texts: list[str]) -> None:
|
||||
"""
|
||||
Pre-load the mock LLM server with simple text responses.
|
||||
|
||||
Resets all queues first, then configures a ``"default"`` queue
|
||||
with one ``QueuedResponse`` per string in *texts*. All agent
|
||||
fixtures use ``model: gpt-4o``, which falls through to the
|
||||
``"default"`` queue on the mock server.
|
||||
|
||||
:param mock_llm_server_url: Mock server base URL.
|
||||
:param texts: Ordered list of response texts the mock should
|
||||
return, one per LLM call.
|
||||
"""
|
||||
reset_mock_llm(mock_llm_server_url)
|
||||
configure_mock_llm(
|
||||
mock_llm_server_url,
|
||||
[{"text": t} for t in texts],
|
||||
)
|
||||
|
||||
|
||||
def _configure_mock_tool_then_text(
|
||||
mock_llm_server_url: str,
|
||||
tool_calls: list[dict[str, str]],
|
||||
follow_up_text: str,
|
||||
) -> None:
|
||||
"""
|
||||
Configure a tool-call response followed by a text response.
|
||||
|
||||
The first LLM call returns a function_call; after the tool
|
||||
executes and the result is sent back, the second LLM call
|
||||
returns a plain text reply.
|
||||
|
||||
:param mock_llm_server_url: Mock server base URL.
|
||||
:param tool_calls: Tool call dicts (``call_id``, ``name``,
|
||||
``arguments``).
|
||||
:param follow_up_text: Text for the second LLM call.
|
||||
"""
|
||||
reset_mock_llm(mock_llm_server_url)
|
||||
configure_mock_llm(
|
||||
mock_llm_server_url,
|
||||
[
|
||||
{"tool_calls": tool_calls},
|
||||
{"text": follow_up_text},
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def _require_omnigent_cli() -> str:
|
||||
"""
|
||||
Resolve the CLI path. Prefers the framework's own
|
||||
@@ -239,6 +301,7 @@ def _read_pending(child: Any, seconds: float = 0.2) -> str:
|
||||
def test_repl_single_approval_allows_llm_response(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
mock_llm_server_url: str,
|
||||
) -> None:
|
||||
"""
|
||||
Drive the full approval → LLM → response loop through the
|
||||
@@ -264,6 +327,7 @@ def test_repl_single_approval_allows_llm_response(
|
||||
multiple ``⚠ approval required`` banners. Counting on
|
||||
the ANSI-stripped buffer is the regression guard.
|
||||
"""
|
||||
_configure_mock_text(mock_llm_server_url, ["Hi there! How can I help you today?"])
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_ASK_DEMO_DIR)],
|
||||
@@ -343,6 +407,7 @@ def test_repl_single_approval_allows_llm_response(
|
||||
def test_repl_refusal_shows_deny_sentinel(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
mock_llm_server_url: str,
|
||||
) -> None:
|
||||
"""
|
||||
Same flow, user refuses → server substitutes the DENY
|
||||
@@ -356,6 +421,9 @@ def test_repl_refusal_shows_deny_sentinel(
|
||||
``_persist_input_deny_sentinel`` surfaces it as the
|
||||
assistant message the REPL renders.
|
||||
"""
|
||||
# No LLM call expected on refuse — configure a dummy response
|
||||
# so the mock doesn't 500 if the server unexpectedly calls it.
|
||||
_configure_mock_text(mock_llm_server_url, ["should not appear"])
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_ASK_DEMO_DIR)],
|
||||
@@ -395,6 +463,7 @@ def test_repl_refusal_shows_deny_sentinel(
|
||||
def test_repl_two_turns_fires_one_approval_per_turn(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
mock_llm_server_url: str,
|
||||
) -> None:
|
||||
"""
|
||||
Regression guard for the multi-turn duplicate-ASK bug.
|
||||
@@ -410,6 +479,14 @@ def test_repl_two_turns_fires_one_approval_per_turn(
|
||||
turn 2 proves the prior user message from turn 1 is NOT
|
||||
being re-enforced.
|
||||
"""
|
||||
# Two turns, each approved — two LLM responses needed.
|
||||
_configure_mock_text(
|
||||
mock_llm_server_url,
|
||||
[
|
||||
"Hello! Nice to meet you.",
|
||||
"Sure thing, got it!",
|
||||
],
|
||||
)
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_ASK_DEMO_DIR)],
|
||||
@@ -491,6 +568,7 @@ def test_repl_two_turns_fires_one_approval_per_turn(
|
||||
def test_repl_approve_always_caches_for_later_turns(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
mock_llm_server_url: str,
|
||||
) -> None:
|
||||
"""
|
||||
End-to-end coverage for the "approve always" cache.
|
||||
@@ -515,6 +593,14 @@ def test_repl_approve_always_caches_for_later_turns(
|
||||
expects no more prompting for this policy in this
|
||||
session.
|
||||
"""
|
||||
# Turn 1 approved-always, turn 2 auto-approved — two LLM calls.
|
||||
_configure_mock_text(
|
||||
mock_llm_server_url,
|
||||
[
|
||||
"Hello there!",
|
||||
"Following up as requested.",
|
||||
],
|
||||
)
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_ASK_DEMO_DIR)],
|
||||
@@ -586,6 +672,7 @@ def test_repl_approve_always_caches_for_later_turns(
|
||||
def test_repl_tool_call_approval_allows_tool_to_run(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
TOOL_CALL ASK → approve → tool runs → LLM responds.
|
||||
@@ -602,6 +689,8 @@ def test_repl_tool_call_approval_allows_tool_to_run(
|
||||
INPUT-phase tests above. Proves the TOOL_CALL site is
|
||||
wired and end-to-end correct.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_TOOL_GATE_DIR)],
|
||||
@@ -654,6 +743,7 @@ def test_repl_tool_call_approval_allows_tool_to_run(
|
||||
def test_repl_tool_call_refusal_blocks_tool(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
TOOL_CALL ASK → refuse → tool NEVER runs → sentinel
|
||||
@@ -666,6 +756,8 @@ def test_repl_tool_call_refusal_blocks_tool(
|
||||
proof that the pre-persistence ordering holds under real
|
||||
streaming + DBOS parking.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_TOOL_GATE_DIR)],
|
||||
@@ -722,6 +814,7 @@ def test_repl_tool_call_refusal_blocks_tool(
|
||||
def test_repl_subagent_ask_tunnels_approval_to_root(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
Sub-agent INPUT ASK → approval on ROOT SSE stream →
|
||||
@@ -744,6 +837,8 @@ def test_repl_subagent_ask_tunnels_approval_to_root(
|
||||
runs, the result flows to the parent, and the parent
|
||||
composes the final response.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_SUBAGENT_GATE_DIR)],
|
||||
@@ -823,6 +918,7 @@ def test_repl_subagent_ask_tunnels_approval_to_root(
|
||||
def test_repl_label_driven_ask_approves(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
mock_llm_server_url: str,
|
||||
) -> None:
|
||||
"""
|
||||
Two-turn label-ASK composition, approve path.
|
||||
@@ -843,6 +939,15 @@ def test_repl_label_driven_ask_approves(
|
||||
write in the chain doesn't leak the write on refuse
|
||||
(that's a separate refuse test below).
|
||||
"""
|
||||
# Turn 1: LLM responds normally (no ASK). Turn 2: ASK fires,
|
||||
# approved, then LLM responds.
|
||||
_configure_mock_text(
|
||||
mock_llm_server_url,
|
||||
[
|
||||
"Got it, banana trigger noted.",
|
||||
"Continuing as requested.",
|
||||
],
|
||||
)
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_LABEL_ASK_GATE_DIR)],
|
||||
@@ -904,6 +1009,7 @@ def test_repl_label_driven_ask_approves(
|
||||
def test_repl_label_driven_ask_refuse_shows_sentinel(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
mock_llm_server_url: str,
|
||||
) -> None:
|
||||
"""
|
||||
Same composition, refuse path.
|
||||
@@ -913,6 +1019,15 @@ def test_repl_label_driven_ask_refuse_shows_sentinel(
|
||||
label-gated ASK's refuse branch goes through the same
|
||||
pre-persist sentinel path as INPUT DENY.
|
||||
"""
|
||||
# Turn 1: LLM responds normally. Turn 2: refused — DENY sentinel,
|
||||
# no second LLM call. Extra dummy response as fail-safe.
|
||||
_configure_mock_text(
|
||||
mock_llm_server_url,
|
||||
[
|
||||
"Banana trigger received.",
|
||||
"should not appear",
|
||||
],
|
||||
)
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_LABEL_ASK_GATE_DIR)],
|
||||
@@ -969,6 +1084,7 @@ def test_repl_label_driven_ask_refuse_shows_sentinel(
|
||||
def test_repl_output_ask_approve_surfaces_llm_reply(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
OUTPUT ASK → approve → LLM reply appears verbatim.
|
||||
@@ -978,6 +1094,8 @@ def test_repl_output_ask_approve_surfaces_llm_reply(
|
||||
approve — the original ``text`` passes through the
|
||||
helper unchanged and lands in the assistant message.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_OUTPUT_GATE_DIR)],
|
||||
@@ -1036,6 +1154,7 @@ def test_repl_output_ask_approve_surfaces_llm_reply(
|
||||
def test_repl_output_ask_refuse_replaces_reply_with_sentinel(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
OUTPUT ASK → refuse → assistant message = sentinel.
|
||||
@@ -1046,6 +1165,8 @@ def test_repl_output_ask_refuse_replaces_reply_with_sentinel(
|
||||
invariant from POLICIES.md §11.4. A follow-up turn only
|
||||
sees the sentinel in history.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_OUTPUT_GATE_DIR)],
|
||||
@@ -1095,6 +1216,7 @@ def test_repl_output_ask_refuse_replaces_reply_with_sentinel(
|
||||
def test_repl_tool_result_ask_approve_surfaces_tool_output(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
TOOL_RESULT ASK → approve → tool output reaches the LLM.
|
||||
@@ -1104,6 +1226,8 @@ def test_repl_tool_result_ask_approve_surfaces_tool_output(
|
||||
original tool output (``echo: <input>``) flows back to
|
||||
the LLM which includes it in the final reply.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_TOOL_RESULT_GATE_DIR)],
|
||||
@@ -1159,6 +1283,7 @@ def test_repl_tool_result_ask_approve_surfaces_tool_output(
|
||||
def test_repl_tool_result_ask_refuse_replaces_output(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
TOOL_RESULT ASK → refuse → tool output replaced by DENY
|
||||
@@ -1169,6 +1294,8 @@ def test_repl_tool_result_ask_refuse_replaces_output(
|
||||
NOT the real output. Regression guard for the pre-
|
||||
persistence substitution in ``_execute_tools``.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_TOOL_RESULT_GATE_DIR)],
|
||||
@@ -1218,6 +1345,7 @@ def test_repl_tool_result_ask_refuse_replaces_output(
|
||||
def test_repl_subagent_tool_call_ask_tunnels_to_root(
|
||||
ap_cli: str,
|
||||
repl_env: dict[str, str],
|
||||
using_mock_llm: bool,
|
||||
) -> None:
|
||||
"""
|
||||
Sub-agent TOOL_CALL ASK → banner on root REPL → approve
|
||||
@@ -1234,6 +1362,8 @@ def test_repl_subagent_tool_call_ask_tunnels_to_root(
|
||||
- Root REPL sees the banner through the same SSE stream
|
||||
it was already consuming.
|
||||
"""
|
||||
if using_mock_llm:
|
||||
pytest.skip("requires real LLM (tool/subagent mock not supported in REPL)")
|
||||
child = pexpect.spawn(
|
||||
ap_cli,
|
||||
["run", str(_SUBAGENT_TOOL_GATE_DIR)],
|
||||
|
||||
@@ -82,6 +82,9 @@ def _install_fake_sdk(
|
||||
for iu in self._script.get("interaction_updates", []):
|
||||
yield SimpleNamespace(sdk_message=None, interaction_update=iu)
|
||||
|
||||
async def cancel(self) -> None:
|
||||
pass # no-op for tests
|
||||
|
||||
async def wait(self) -> Any:
|
||||
return SimpleNamespace(
|
||||
status=self._script.get("status", "finished"),
|
||||
@@ -89,9 +92,17 @@ def _install_fake_sdk(
|
||||
)
|
||||
|
||||
class _FakeAgent:
|
||||
async def send(self, prompt: str) -> _FakeRun:
|
||||
async def send(self, prompt: str, **kwargs: Any) -> _FakeRun:
|
||||
state["sent"].append(prompt)
|
||||
return _FakeRun(scripts.pop(0))
|
||||
script = scripts.pop(0)
|
||||
# Invoke on_delta for interaction_updates (mirrors real SDK
|
||||
# which dispatches TurnEndedUpdate via on_delta, not events).
|
||||
options = kwargs.get("options")
|
||||
on_delta = getattr(options, "on_delta", None) if options else None
|
||||
if on_delta and "interaction_updates" in script:
|
||||
for iu in script["interaction_updates"]:
|
||||
on_delta(iu)
|
||||
return _FakeRun(script)
|
||||
|
||||
# AsyncAgent exposes close() (a CloseAgent RPC + tool unregister).
|
||||
async def close(self) -> None:
|
||||
@@ -137,11 +148,16 @@ def _install_fake_sdk(
|
||||
self.cwd = cwd
|
||||
self.custom_tools = custom_tools
|
||||
|
||||
class _FakeSendOptions:
|
||||
def __init__(self, on_delta: Any = None, **_kw: Any) -> None:
|
||||
self.on_delta = on_delta
|
||||
|
||||
fake = types.ModuleType("cursor_sdk")
|
||||
fake.AsyncClient = _FakeClient # type: ignore[attr-defined]
|
||||
fake.AsyncAgent = _FakeAsyncAgent # type: ignore[attr-defined]
|
||||
fake.CustomTool = _FakeCustomTool # type: ignore[attr-defined]
|
||||
fake.LocalAgentOptions = _FakeLocalAgentOptions # type: ignore[attr-defined]
|
||||
fake.SendOptions = _FakeSendOptions # type: ignore[attr-defined]
|
||||
monkeypatch.setitem(sys.modules, "cursor_sdk", fake)
|
||||
return state
|
||||
|
||||
@@ -262,7 +278,7 @@ def test_sdk_message_to_events_unwraps_envelope_on_completion_and_error() -> Non
|
||||
done = _sdk_message_to_events(_envelope("completed", [{"type": "text", "text": "ok"}]))
|
||||
assert isinstance(done[0], ToolCallComplete)
|
||||
assert done[0].name == "sys_session_send" # unwrapped, not "mcp"
|
||||
assert done[0].metadata == {"call_id": "c1"}
|
||||
assert done[0].metadata == {"call_id": "c1", "is_bridged": True}
|
||||
|
||||
err = _sdk_message_to_events(_envelope("error", "boom"))
|
||||
assert isinstance(err[0], ToolCallComplete)
|
||||
@@ -861,3 +877,180 @@ def test_normalize_cursor_usage_zero_tokens_preserved() -> None:
|
||||
assert result["input_tokens"] == 0
|
||||
assert result["output_tokens"] == 0
|
||||
assert result["total_tokens"] == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# PHASE_TOOL_CALL policy for native tools
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_sdk_message_to_events_marks_native_tool_not_bridged() -> None:
|
||||
"""A plain (non-MCP-wrapped) tool call has ``is_bridged=False`` in metadata."""
|
||||
events = _sdk_message_to_events(_tool("bash", "t1", "running", args={"cmd": "ls"}))
|
||||
assert len(events) == 1
|
||||
assert isinstance(events[0], ToolCallRequest)
|
||||
assert events[0].metadata["is_bridged"] is False
|
||||
|
||||
# Completed status too.
|
||||
done = _sdk_message_to_events(
|
||||
_tool("bash", "t1", "completed", args={"cmd": "ls"}, result="ok")
|
||||
)
|
||||
assert isinstance(done[0], ToolCallComplete)
|
||||
assert done[0].metadata["is_bridged"] is False
|
||||
|
||||
|
||||
def test_sdk_message_to_events_marks_mcp_tool_bridged() -> None:
|
||||
"""An MCP-wrapped tool call has ``is_bridged=True`` in metadata."""
|
||||
envelope = SimpleNamespace(
|
||||
type="tool_call",
|
||||
name="mcp",
|
||||
call_id="c1",
|
||||
status="running",
|
||||
args={
|
||||
"providerIdentifier": "custom-user-tools",
|
||||
"toolName": "sys_session_send",
|
||||
"args": {"session": "s1"},
|
||||
},
|
||||
result=None,
|
||||
)
|
||||
events = _sdk_message_to_events(envelope)
|
||||
assert isinstance(events[0], ToolCallRequest)
|
||||
assert events[0].metadata["is_bridged"] is True
|
||||
|
||||
# Completed too.
|
||||
envelope_done = SimpleNamespace(
|
||||
type="tool_call",
|
||||
name="mcp",
|
||||
call_id="c1",
|
||||
status="completed",
|
||||
args={
|
||||
"providerIdentifier": "custom-user-tools",
|
||||
"toolName": "sys_session_send",
|
||||
"args": {"session": "s1"},
|
||||
},
|
||||
result="ok",
|
||||
)
|
||||
done = _sdk_message_to_events(envelope_done)
|
||||
assert isinstance(done[0], ToolCallComplete)
|
||||
assert done[0].metadata["is_bridged"] is True
|
||||
|
||||
|
||||
async def test_run_turn_native_tool_denied_by_policy(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""A native tool call triggers PHASE_TOOL_CALL. On DENY the run emits
|
||||
ToolCallRequest then ExecutorError and the turn ends."""
|
||||
script = {
|
||||
"messages": [
|
||||
_assistant("Let me run that."),
|
||||
_tool("bash", "t1", "running", args={"cmd": "rm -rf /"}),
|
||||
],
|
||||
"status": "finished",
|
||||
"result": "",
|
||||
}
|
||||
_install_fake_sdk(monkeypatch, [script])
|
||||
executor = CursorExecutor(api_key="crsr_x")
|
||||
executor._policy_evaluator = _policy("PHASE_TOOL_CALL")
|
||||
try:
|
||||
events = [e async for e in executor.run_turn([_user("hi")], [], "SYS")]
|
||||
finally:
|
||||
await executor.close()
|
||||
|
||||
# The ToolCallRequest is emitted so observers see what was attempted.
|
||||
reqs = [e for e in events if isinstance(e, ToolCallRequest)]
|
||||
assert len(reqs) == 1
|
||||
assert reqs[0].name == "bash"
|
||||
|
||||
# Then an ExecutorError with the denial reason.
|
||||
errors = [e for e in events if isinstance(e, ExecutorError)]
|
||||
assert len(errors) == 1
|
||||
assert "denied by policy" in errors[0].message
|
||||
assert "bash" in errors[0].message
|
||||
|
||||
# ToolCallRequest appears before ExecutorError, and nothing follows the error.
|
||||
req_idx = next(i for i, e in enumerate(events) if isinstance(e, ToolCallRequest))
|
||||
err_idx = next(i for i, e in enumerate(events) if isinstance(e, ExecutorError))
|
||||
assert req_idx < err_idx
|
||||
assert err_idx == len(events) - 1 # error is the last event
|
||||
|
||||
# No TurnComplete — the turn was aborted.
|
||||
assert not any(isinstance(e, TurnComplete) for e in events)
|
||||
|
||||
|
||||
async def test_run_turn_bridged_tool_skips_tool_call_policy(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A bridged (MCP-wrapped) tool does NOT trigger PHASE_TOOL_CALL — it's
|
||||
already gated server-side via the dispatch bridge."""
|
||||
# Build an MCP-envelope tool call (bridged).
|
||||
mcp_running = SimpleNamespace(
|
||||
type="tool_call",
|
||||
name="mcp",
|
||||
call_id="c1",
|
||||
status="running",
|
||||
args={
|
||||
"providerIdentifier": "custom-user-tools",
|
||||
"toolName": "sys_session_send",
|
||||
"args": {"session": "s1", "message": "go"},
|
||||
},
|
||||
result=None,
|
||||
)
|
||||
mcp_done = SimpleNamespace(
|
||||
type="tool_call",
|
||||
name="mcp",
|
||||
call_id="c1",
|
||||
status="completed",
|
||||
args={
|
||||
"providerIdentifier": "custom-user-tools",
|
||||
"toolName": "sys_session_send",
|
||||
"args": {"session": "s1", "message": "go"},
|
||||
},
|
||||
result="ok",
|
||||
)
|
||||
script = {
|
||||
"messages": [_assistant("Dispatching."), mcp_running, mcp_done, _assistant("Done.")],
|
||||
"status": "finished",
|
||||
"result": "Done.",
|
||||
}
|
||||
_install_fake_sdk(monkeypatch, [script])
|
||||
|
||||
# Wire a policy that denies PHASE_TOOL_CALL — if it fires, the turn would abort.
|
||||
executor = CursorExecutor(api_key="crsr_x")
|
||||
executor._policy_evaluator = _policy("PHASE_TOOL_CALL")
|
||||
try:
|
||||
events = [e async for e in executor.run_turn([_user("hi")], [], "SYS")]
|
||||
finally:
|
||||
await executor.close()
|
||||
|
||||
# Verify the bridged tool call was actually observed (not silently dropped).
|
||||
reqs = [e for e in events if isinstance(e, ToolCallRequest)]
|
||||
assert len(reqs) == 1 and reqs[0].name == "sys_session_send"
|
||||
|
||||
# The turn completes normally — the bridged tool was NOT policy-gated here.
|
||||
assert any(isinstance(e, TurnComplete) for e in events)
|
||||
assert not any(isinstance(e, ExecutorError) for e in events)
|
||||
|
||||
|
||||
async def test_run_turn_native_tool_allowed_by_policy(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""When PHASE_TOOL_CALL returns ALLOW, the turn proceeds normally."""
|
||||
script = {
|
||||
"messages": [
|
||||
_assistant("Running."),
|
||||
_tool("bash", "t1", "running", args={"cmd": "echo hi"}),
|
||||
_tool("bash", "t1", "completed", result="hi"),
|
||||
_assistant("Done."),
|
||||
],
|
||||
"status": "finished",
|
||||
"result": "Done.",
|
||||
}
|
||||
_install_fake_sdk(monkeypatch, [script])
|
||||
executor = CursorExecutor(api_key="crsr_x")
|
||||
executor._policy_evaluator = _policy(None) # never denies
|
||||
try:
|
||||
events = [e async for e in executor.run_turn([_user("hi")], [], "SYS")]
|
||||
finally:
|
||||
await executor.close()
|
||||
|
||||
assert any(isinstance(e, TurnComplete) for e in events)
|
||||
assert not any(isinstance(e, ExecutorError) for e in events)
|
||||
# The tool call went through.
|
||||
reqs = [e for e in events if isinstance(e, ToolCallRequest)]
|
||||
assert len(reqs) == 1 and reqs[0].name == "bash"
|
||||
|
||||
@@ -1625,6 +1625,102 @@ def test_get_openai_client_host_override_requires_auth_command(monkeypatch):
|
||||
)
|
||||
|
||||
|
||||
def test_get_openai_client_api_key_falls_back_to_env_base_url(monkeypatch):
|
||||
"""A spec-level api_key with NO override honors ambient ``OPENAI_BASE_URL``.
|
||||
|
||||
Regression for the residual gateway 401 (continuation-turn daemon
|
||||
spawns): the api_key is frequently a gateway credential (e.g. a
|
||||
Databricks AI Gateway PAT detected from ``OPENAI_API_KEY``), and the
|
||||
companion base_url can be dropped on the daemon → runner → harness
|
||||
propagation chain (the spec-auth bake omits it when ``OPENAI_BASE_URL``
|
||||
is absent at materialization time; a reused local daemon may predate the
|
||||
env var). Without the ambient fallback, the gateway PAT is sent to
|
||||
``api.openai.com`` and 401s. The runner inherits ``OPENAI_BASE_URL``, so
|
||||
honoring it here keeps the gateway target present on every turn.
|
||||
|
||||
:param monkeypatch: Pytest monkeypatch fixture.
|
||||
"""
|
||||
from omnigent.inner.openai_agents_sdk_executor import _get_openai_async_client
|
||||
|
||||
monkeypatch.setenv("OPENAI_BASE_URL", "https://gateway.example.com/ai-gateway/openai/v1")
|
||||
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
class _StubAsyncOpenAI:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
import openai as _openai_mod
|
||||
|
||||
with patch.object(_openai_mod, "AsyncOpenAI", _StubAsyncOpenAI, create=True):
|
||||
_get_openai_async_client(api_key="dapi-gateway-token", base_url_override=None)
|
||||
|
||||
assert captured["api_key"] == "dapi-gateway-token"
|
||||
assert captured["base_url"] == "https://gateway.example.com/ai-gateway/openai/v1", (
|
||||
"A spec-level api_key with no base_url override must fall back to the "
|
||||
"ambient OPENAI_BASE_URL the runner inherits; routing to api.openai.com "
|
||||
"(base_url=None) sends a gateway PAT to OpenAI and 401s."
|
||||
)
|
||||
|
||||
|
||||
def test_get_openai_client_api_key_override_wins_over_env_base_url(monkeypatch):
|
||||
"""An explicit base_url override wins over the ambient ``OPENAI_BASE_URL``.
|
||||
|
||||
The baked ``executor.auth.base_url`` (threaded via
|
||||
``HARNESS_OPENAI_AGENTS_GATEWAY_BASE_URL``) is the authoritative
|
||||
per-spec target and must not be shadowed by a differing env var.
|
||||
|
||||
:param monkeypatch: Pytest monkeypatch fixture.
|
||||
"""
|
||||
from omnigent.inner.openai_agents_sdk_executor import _get_openai_async_client
|
||||
|
||||
monkeypatch.setenv("OPENAI_BASE_URL", "https://wrong-env.example.com/v1")
|
||||
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
class _StubAsyncOpenAI:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
import openai as _openai_mod
|
||||
|
||||
with patch.object(_openai_mod, "AsyncOpenAI", _StubAsyncOpenAI, create=True):
|
||||
_get_openai_async_client(
|
||||
api_key="dapi-gateway-token",
|
||||
base_url_override="https://spec-gateway.example.com/openai/v1",
|
||||
)
|
||||
|
||||
assert captured["base_url"] == "https://spec-gateway.example.com/openai/v1"
|
||||
|
||||
|
||||
def test_get_openai_client_api_key_no_env_defaults_to_openai(monkeypatch):
|
||||
"""A genuine OpenAI key with no gateway anywhere still defaults to OpenAI.
|
||||
|
||||
With no override and no ambient ``OPENAI_BASE_URL``, ``base_url`` stays
|
||||
``None`` so the AsyncOpenAI client targets ``api.openai.com`` — the
|
||||
correct behavior for a real ``sk-...`` key.
|
||||
|
||||
:param monkeypatch: Pytest monkeypatch fixture.
|
||||
"""
|
||||
from omnigent.inner.openai_agents_sdk_executor import _get_openai_async_client
|
||||
|
||||
monkeypatch.delenv("OPENAI_BASE_URL", raising=False)
|
||||
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
class _StubAsyncOpenAI:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
import openai as _openai_mod
|
||||
|
||||
with patch.object(_openai_mod, "AsyncOpenAI", _StubAsyncOpenAI, create=True):
|
||||
_get_openai_async_client(api_key="sk-real-openai-key", base_url_override=None)
|
||||
|
||||
assert captured["api_key"] == "sk-real-openai-key"
|
||||
assert captured["base_url"] is None
|
||||
|
||||
|
||||
def test_get_openai_client_no_profile_honors_env_vars(monkeypatch):
|
||||
"""Without an explicit profile, the env-var branch still works.
|
||||
|
||||
|
||||
@@ -46,8 +46,9 @@ def _write_config(tmp_path: Path, block: dict[str, object]) -> None:
|
||||
|
||||
|
||||
def test_looks_like_gemini_api_key() -> None:
|
||||
"""The soft prefix check accepts ``AIza`` keys and rejects others."""
|
||||
"""The soft prefix check accepts ``AIza`` / ``AQ`` keys and rejects others."""
|
||||
assert looks_like_gemini_api_key("AIzaSyAbC123")
|
||||
assert looks_like_gemini_api_key("AQ.AbC123")
|
||||
assert not looks_like_gemini_api_key("sk-ant-123")
|
||||
assert not looks_like_gemini_api_key("")
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ SCRIPT = REPO_ROOT / ".github/scripts/fork-e2e/should-mirror.sh"
|
||||
def _run(
|
||||
tmp_path: Path,
|
||||
*,
|
||||
approvers: str = "",
|
||||
labels: str = "",
|
||||
labeler: str = "",
|
||||
maintainers: str = "alice bob",
|
||||
@@ -18,32 +19,31 @@ def _run(
|
||||
"""
|
||||
Run should-mirror.sh against a mocked ``gh`` and return its outputs.
|
||||
|
||||
The label-only gate makes exactly two ``gh`` calls, which the mock answers:
|
||||
The gate checks two paths (approval OR label), making up to three
|
||||
``gh`` calls which the mock answers:
|
||||
|
||||
- ``pr view {pr} ... --json labels --jq '.labels[].name'`` -> *labels*, a
|
||||
space-separated label list printed one per line (the post-``--jq`` shape
|
||||
the script greps).
|
||||
- ``api repos/{repo}/issues/{pr}/events ...`` -> *labeler*, the login of the
|
||||
account that last applied the gate label (empty if none).
|
||||
- ``api repos/{repo}/pulls/{pr}/reviews ...`` -> *approvers*
|
||||
- ``pr view {pr} ... --json labels --jq '.labels[].name'`` -> *labels*
|
||||
- ``api repos/{repo}/issues/{pr}/events ...`` -> *labeler*
|
||||
|
||||
:param tmp_path: Pytest tmp dir for the mock + output file.
|
||||
:param approvers: Space-separated logins the reviews mock returns as
|
||||
approvers; empty means no approving reviews.
|
||||
:param labels: Space-separated labels currently on the PR; empty means none.
|
||||
:param labeler: Login the issue-events mock attributes the gate label to.
|
||||
:param maintainers: Space-separated maintainer logins (as
|
||||
load-maintainers.sh would emit); empty means none.
|
||||
:returns: Parsed ``key=value`` GITHUB_OUTPUT lines, e.g.
|
||||
``{"mirror": "true", "reason": "..."}``.
|
||||
:param maintainers: Space-separated maintainer logins.
|
||||
:returns: Parsed ``key=value`` GITHUB_OUTPUT lines.
|
||||
"""
|
||||
gh = tmp_path / "gh"
|
||||
gh.write_text(
|
||||
"#!/usr/bin/env bash\n"
|
||||
"set -uo pipefail\n"
|
||||
# gh pr view <pr> --repo <repo> --json labels --jq '.labels[].name'
|
||||
# shellcheck-style: unquoted expansion intentionally splits labels.
|
||||
'if [[ "$1" == "pr" ]]; then [[ -n "$MOCK_LABELS" ]]'
|
||||
' && printf "%s\\n" $MOCK_LABELS; exit 0; fi\n'
|
||||
'if [[ "$1" == "api" ]]; then\n'
|
||||
' case "$2" in\n'
|
||||
' *pulls/*reviews*) [[ -n "$MOCK_APPROVERS" ]]'
|
||||
' && printf "%s\\n" $MOCK_APPROVERS; exit 0 ;;\n'
|
||||
' *issues/*events*) [[ -n "$MOCK_LABELER" ]]'
|
||||
' && printf "%s\\n" "$MOCK_LABELER"; exit 0 ;;\n'
|
||||
" esac\n"
|
||||
@@ -63,10 +63,9 @@ def _run(
|
||||
"GH_TOKEN": "unused",
|
||||
"REPO": "test/repo",
|
||||
"PR": "7",
|
||||
"LABEL": "e2e-approved",
|
||||
"MIRROR_BRANCH": "fork-e2e/pr-7",
|
||||
"MAINTAINERS": maintainers,
|
||||
"GITHUB_OUTPUT": str(out_file),
|
||||
"MOCK_APPROVERS": approvers,
|
||||
"MOCK_LABELS": labels,
|
||||
"MOCK_LABELER": labeler,
|
||||
}
|
||||
@@ -87,78 +86,80 @@ def _run(
|
||||
return outputs
|
||||
|
||||
|
||||
def test_label_applied_by_maintainer_mirrors(tmp_path: Path) -> None:
|
||||
"""The gate label, applied by a maintainer, opens the gate.
|
||||
# --- Path 1: maintainer approval ---
|
||||
|
||||
The whole contract: secret-bearing e2e runs only after a maintainer applies
|
||||
``e2e-approved``. Asserts ``mirror=true`` and that the reason names the
|
||||
maintainer who applied it.
|
||||
"""
|
||||
out = _run(tmp_path, labels="e2e-approved", labeler="bob")
|
||||
|
||||
def test_approved_by_maintainer_mirrors(tmp_path: Path) -> None:
|
||||
"""A maintainer's approving review opens the gate."""
|
||||
out = _run(tmp_path, approvers="bob")
|
||||
assert out["mirror"] == "true"
|
||||
assert "applied by maintainer" in out["reason"]
|
||||
assert "approved by maintainer" in out["reason"]
|
||||
|
||||
|
||||
def test_maintainer_match_is_case_insensitive(tmp_path: Path) -> None:
|
||||
"""Labeler vs MAINTAINER comparison is case-insensitive.
|
||||
|
||||
GitHub logins are compared lowercased, so a maintainer labelled as ``Bob``
|
||||
still opens the gate against a ``bob`` MAINTAINER entry.
|
||||
"""
|
||||
out = _run(tmp_path, labels="e2e-approved", labeler="Bob", maintainers="alice bob")
|
||||
"""Approver vs MAINTAINER comparison is case-insensitive."""
|
||||
out = _run(tmp_path, approvers="Bob", maintainers="alice bob")
|
||||
assert out["mirror"] == "true"
|
||||
|
||||
|
||||
def test_label_absent_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""Without the gate label the gate stays shut.
|
||||
|
||||
No ``e2e-approved`` means no secret-bearing e2e on a fork PR. Asserts
|
||||
``mirror=false`` and an awaiting-label reason.
|
||||
"""
|
||||
out = _run(tmp_path, labels="", labeler="bob")
|
||||
def test_approved_by_non_maintainer_no_label_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""An approval from a non-maintainer (and no label) doesn't open the gate."""
|
||||
out = _run(tmp_path, approvers="eve", maintainers="alice bob")
|
||||
assert out["mirror"] == "false"
|
||||
assert "awaiting" in out["reason"]
|
||||
|
||||
|
||||
def test_other_labels_are_ignored(tmp_path: Path) -> None:
|
||||
"""Unrelated labels never open the gate.
|
||||
def test_multiple_approvers_first_maintainer_wins(tmp_path: Path) -> None:
|
||||
"""When multiple users approve, the first matching maintainer opens the gate."""
|
||||
out = _run(tmp_path, approvers="eve alice", maintainers="alice bob")
|
||||
assert out["mirror"] == "true"
|
||||
assert "@alice" in out["reason"]
|
||||
|
||||
Only the exact gate label counts; ``bug``/``enhancement`` leave it shut.
|
||||
"""
|
||||
out = _run(tmp_path, labels="bug enhancement", labeler="bob")
|
||||
assert out["mirror"] == "false"
|
||||
assert "awaiting" in out["reason"]
|
||||
|
||||
# --- Path 2: e2e-approved label ---
|
||||
|
||||
|
||||
def test_label_applied_by_maintainer_mirrors(tmp_path: Path) -> None:
|
||||
"""The e2e-approved label applied by a maintainer opens the gate."""
|
||||
out = _run(tmp_path, labels="e2e-approved", labeler="bob")
|
||||
assert out["mirror"] == "true"
|
||||
assert "e2e-approved" in out["reason"]
|
||||
assert "maintainer" in out["reason"]
|
||||
|
||||
|
||||
def test_label_applied_by_non_maintainer_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""The gate label applied by a NON-maintainer must not open the gate.
|
||||
|
||||
Triage+ access lets non-maintainers apply labels too, so label presence
|
||||
alone is insufficient: the labeler must be in MAINTAINER. ``eve`` applies it
|
||||
but isn't a maintainer, so ``mirror=false``.
|
||||
"""
|
||||
"""The e2e-approved label applied by a non-maintainer doesn't open the gate."""
|
||||
out = _run(tmp_path, labels="e2e-approved", labeler="eve", maintainers="alice bob")
|
||||
assert out["mirror"] == "false"
|
||||
assert "non-maintainer" in out["reason"]
|
||||
|
||||
|
||||
def test_label_present_but_unattributable_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""A present label with no labeled-event actor stays shut (fail closed).
|
||||
|
||||
If the label can't be attributed to anyone (e.g. seeded outside the events
|
||||
timeline), we can't confirm a maintainer applied it, so ``mirror=false``.
|
||||
"""
|
||||
def test_label_without_labeler_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""A label with no attributable labeler doesn't open the gate."""
|
||||
out = _run(tmp_path, labels="e2e-approved", labeler="")
|
||||
assert out["mirror"] == "false"
|
||||
assert "no attributable labeler" in out["reason"]
|
||||
|
||||
|
||||
# --- Either path ---
|
||||
|
||||
|
||||
def test_approval_takes_precedence_over_label(tmp_path: Path) -> None:
|
||||
"""When both approval and label are present, approval wins (checked first)."""
|
||||
out = _run(tmp_path, approvers="alice", labels="e2e-approved", labeler="bob")
|
||||
assert out["mirror"] == "true"
|
||||
assert "approved by maintainer" in out["reason"]
|
||||
|
||||
|
||||
# --- Neither path ---
|
||||
|
||||
|
||||
def test_no_approval_no_label_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""Without approval or label, the gate stays shut."""
|
||||
out = _run(tmp_path, approvers="", labels="")
|
||||
assert out["mirror"] == "false"
|
||||
assert "awaiting" in out["reason"]
|
||||
|
||||
|
||||
def test_no_maintainers_loaded_does_not_mirror(tmp_path: Path) -> None:
|
||||
"""An empty MAINTAINER list fails closed.
|
||||
|
||||
With no maintainers to verify against, even a present label can't be
|
||||
trusted, so the gate stays shut regardless of the labeler.
|
||||
"""
|
||||
out = _run(tmp_path, labels="e2e-approved", labeler="bob", maintainers="")
|
||||
"""An empty MAINTAINER list fails closed."""
|
||||
out = _run(tmp_path, approvers="bob", maintainers="")
|
||||
assert out["mirror"] == "false"
|
||||
assert "no maintainers" in out["reason"]
|
||||
|
||||
@@ -10,24 +10,21 @@ SCRIPT = REPO_ROOT / ".github/scripts/merge-ready/compute-gate.sh"
|
||||
# A representative FAILED bullet list, the shape evaluate-checks.sh emits.
|
||||
FAILED = "- `E2E Tests (shard 0/4)` (still pending or cancelled)\n"
|
||||
|
||||
# The hint sentinel -- the maintainer-only label name appears only in the nudge.
|
||||
HINT_MARKER = "e2e-approved"
|
||||
|
||||
|
||||
def _run(
|
||||
tmp_path: Path,
|
||||
*,
|
||||
eval_outcome: str = "failure",
|
||||
failed: str = FAILED,
|
||||
fork_needs_e2e_label: str | None = None,
|
||||
fork_needs_e2e_approval: str | None = None,
|
||||
) -> dict[str, str]:
|
||||
"""Run compute-gate.sh with the given env and parse its GITHUB_OUTPUT.
|
||||
|
||||
The script makes no ``gh`` calls -- it is a pure function of its env -- so we
|
||||
just set the inputs and read back ``state`` / ``short_desc`` / ``long_desc``.
|
||||
|
||||
:param fork_needs_e2e_label: when ``None`` the var is left unset entirely, to
|
||||
exercise the ``${FORK_NEEDS_E2E_LABEL:-false}`` default (back-compat).
|
||||
:param fork_needs_e2e_approval: when ``None`` the var is left unset entirely,
|
||||
to exercise the ``${FORK_NEEDS_E2E_APPROVAL:-false}`` default (back-compat).
|
||||
"""
|
||||
out_file = tmp_path / "gh_output"
|
||||
out_file.touch()
|
||||
@@ -40,12 +37,12 @@ def _run(
|
||||
"GITHUB_OUTPUT": str(out_file),
|
||||
}
|
||||
)
|
||||
if fork_needs_e2e_label is not None:
|
||||
env["FORK_NEEDS_E2E_LABEL"] = fork_needs_e2e_label
|
||||
if fork_needs_e2e_approval is not None:
|
||||
env["FORK_NEEDS_E2E_APPROVAL"] = fork_needs_e2e_approval
|
||||
else:
|
||||
# Drop any ambient value so the None case deterministically exercises
|
||||
# the script's `:-false` default (os.environ.copy() could inherit it).
|
||||
env.pop("FORK_NEEDS_E2E_LABEL", None)
|
||||
env.pop("FORK_NEEDS_E2E_APPROVAL", None)
|
||||
|
||||
proc = subprocess.run(
|
||||
["bash", str(SCRIPT)],
|
||||
@@ -80,54 +77,58 @@ def _parse_github_output(text: str) -> dict[str, str]:
|
||||
return out
|
||||
|
||||
|
||||
def test_green_gate_has_no_hint_for_same_repo(tmp_path: Path) -> None:
|
||||
"""A green same-repo gate is unchanged: success, no e2e-label nudge."""
|
||||
out = _run(tmp_path, eval_outcome="success", fork_needs_e2e_label="false")
|
||||
def test_green_gate_has_no_blocker_for_same_repo(tmp_path: Path) -> None:
|
||||
"""A green same-repo gate is unchanged: success, no fork-approval blocker."""
|
||||
out = _run(tmp_path, eval_outcome="success", fork_needs_e2e_approval="false")
|
||||
assert out["state"] == "success"
|
||||
assert "merging now" in out["long_desc"]
|
||||
assert HINT_MARKER not in out["long_desc"]
|
||||
assert "maintainer must approve" not in out["long_desc"]
|
||||
|
||||
|
||||
def test_red_gate_has_no_hint_for_same_repo(tmp_path: Path) -> None:
|
||||
"""A red same-repo gate lists failures but adds no e2e-label nudge."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_label="false")
|
||||
def test_red_gate_has_no_blocker_for_same_repo(tmp_path: Path) -> None:
|
||||
"""A red same-repo gate lists failures but adds no fork-approval blocker."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_approval="false")
|
||||
assert out["state"] == "failure"
|
||||
assert "gate not green yet" in out["long_desc"]
|
||||
assert HINT_MARKER not in out["long_desc"]
|
||||
assert "maintainer must approve" not in out["long_desc"]
|
||||
|
||||
|
||||
def test_red_gate_on_fork_without_label_adds_hint(tmp_path: Path) -> None:
|
||||
"""A fork PR missing the label gets the apply-`e2e-approved` nudge appended.
|
||||
def test_fork_without_approval_is_blocking(tmp_path: Path) -> None:
|
||||
"""A fork PR without maintainer approval must be blocked (state=failure).
|
||||
|
||||
The failure prose still comes first; the hint is an extra paragraph so the
|
||||
contributor/maintainer sees both what's blocking and how to run e2e.
|
||||
Even if all other checks are green (eval=success), the gate stays red
|
||||
until a maintainer approves the PR to trigger e2e.
|
||||
"""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_label="true")
|
||||
out = _run(tmp_path, eval_outcome="success", fork_needs_e2e_approval="true")
|
||||
assert out["state"] == "failure"
|
||||
assert "Awaiting maintainer approval" in out["short_desc"]
|
||||
assert "maintainer must approve" in out["long_desc"]
|
||||
|
||||
|
||||
def test_red_gate_on_fork_without_approval_is_still_blocking(tmp_path: Path) -> None:
|
||||
"""A fork PR with red checks AND no approval is doubly blocked."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_approval="true")
|
||||
assert out["state"] == "failure"
|
||||
assert "maintainer must approve" in out["long_desc"]
|
||||
assert "gate not green yet" in out["long_desc"]
|
||||
assert HINT_MARKER in out["long_desc"]
|
||||
assert "do not run automatically on fork PRs" in out["long_desc"]
|
||||
|
||||
|
||||
def test_green_gate_on_fork_without_label_adds_hint(tmp_path: Path) -> None:
|
||||
"""Even a green fork gate carries the hint: green means e2e was skipped,
|
||||
not that it ran, so the caveat still matters before merge."""
|
||||
out = _run(tmp_path, eval_outcome="success", fork_needs_e2e_label="true")
|
||||
def test_fork_with_approval_uses_normal_gate(tmp_path: Path) -> None:
|
||||
"""A fork PR that HAS maintainer approval uses the normal CI gate."""
|
||||
out = _run(tmp_path, eval_outcome="success", fork_needs_e2e_approval="false")
|
||||
assert out["state"] == "success"
|
||||
assert "merging now" in out["long_desc"]
|
||||
assert HINT_MARKER in out["long_desc"]
|
||||
|
||||
|
||||
def test_short_desc_never_carries_hint(tmp_path: Path) -> None:
|
||||
"""The hint is comment-only; the 140-char commit status stays clean."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_label="true")
|
||||
assert HINT_MARKER not in out["short_desc"]
|
||||
def test_short_desc_never_exceeds_140_chars(tmp_path: Path) -> None:
|
||||
"""The 140-char commit status limit is respected."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_approval="true")
|
||||
assert len(out["short_desc"]) <= 140
|
||||
|
||||
|
||||
def test_fork_label_var_unset_defaults_to_no_hint(tmp_path: Path) -> None:
|
||||
"""With FORK_NEEDS_E2E_LABEL unset, the script defaults to no hint (the
|
||||
``:-false`` fallback keeps it safe under ``set -u``)."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_label=None)
|
||||
def test_fork_approval_var_unset_defaults_to_no_blocker(tmp_path: Path) -> None:
|
||||
"""With FORK_NEEDS_E2E_APPROVAL unset, the script defaults to no blocker
|
||||
(the ``:-false`` fallback keeps it safe under ``set -u``)."""
|
||||
out = _run(tmp_path, eval_outcome="failure", fork_needs_e2e_approval=None)
|
||||
assert out["state"] == "failure"
|
||||
assert HINT_MARKER not in out["long_desc"]
|
||||
assert "maintainer must approve" not in out["long_desc"]
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
SCRIPT = REPO_ROOT / ".github/scripts/merge-ready/required.sh"
|
||||
|
||||
|
||||
def _is_allow_skip(
|
||||
check_name: str,
|
||||
*,
|
||||
is_fork: str = "false",
|
||||
) -> bool:
|
||||
"""Source required.sh and test is_allow_skip for *check_name*.
|
||||
|
||||
:param is_fork: passed as IS_FORK env var ("true" or "false").
|
||||
:returns: True if the check is allowed to skip.
|
||||
"""
|
||||
env = os.environ.copy()
|
||||
env["IS_FORK"] = is_fork
|
||||
# Source required.sh then call is_allow_skip; exit code is the result.
|
||||
proc = subprocess.run(
|
||||
[
|
||||
"bash",
|
||||
"-c",
|
||||
f'source "{SCRIPT}" && is_allow_skip "{check_name}"',
|
||||
],
|
||||
env=env,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
return proc.returncode == 0
|
||||
|
||||
|
||||
# --- Same-repo (IS_FORK=false): ALLOW_SKIP works normally ---
|
||||
|
||||
|
||||
def test_e2e_shard_skippable_for_same_repo() -> None:
|
||||
"""Same-repo PRs can skip e2e shards (path-filtered, e.g. ap-web-only)."""
|
||||
assert _is_allow_skip("E2E Tests (shard 0/4)", is_fork="false") is True
|
||||
|
||||
|
||||
def test_integration_skippable_for_same_repo() -> None:
|
||||
"""Same-repo PRs can skip integration checks."""
|
||||
assert _is_allow_skip("Integration (codex)", is_fork="false") is True
|
||||
|
||||
|
||||
def test_pytest_skippable_for_same_repo() -> None:
|
||||
"""Same-repo PRs can skip Pytest shards."""
|
||||
assert _is_allow_skip("Pytest (runtime-core)", is_fork="false") is True
|
||||
|
||||
|
||||
def test_precommit_not_skippable() -> None:
|
||||
"""Pre-commit checks are never skippable (not in ALLOW_SKIP)."""
|
||||
assert _is_allow_skip("Pre-commit checks", is_fork="false") is False
|
||||
|
||||
|
||||
# --- Fork PRs (IS_FORK=true): e2e/integration NOT skippable ---
|
||||
|
||||
|
||||
def test_e2e_shard_not_skippable_for_fork() -> None:
|
||||
"""Fork PRs must NOT skip e2e shards -- FORK_NEVER_SKIP overrides."""
|
||||
assert _is_allow_skip("E2E Tests (shard 0/4)", is_fork="true") is False
|
||||
assert _is_allow_skip("E2E Tests (shard 3/4)", is_fork="true") is False
|
||||
|
||||
|
||||
def test_e2e_ui_shard_not_skippable_for_fork() -> None:
|
||||
"""Fork PRs must NOT skip e2e UI shards."""
|
||||
assert _is_allow_skip("E2E UI Tests (shard 0/3)", is_fork="true") is False
|
||||
|
||||
|
||||
def test_integration_not_skippable_for_fork() -> None:
|
||||
"""Fork PRs must NOT skip integration checks."""
|
||||
assert _is_allow_skip("Integration (codex)", is_fork="true") is False
|
||||
assert _is_allow_skip("Integration (claude-sdk)", is_fork="true") is False
|
||||
assert _is_allow_skip("Integration (openai-agents)", is_fork="true") is False
|
||||
|
||||
|
||||
def test_pytest_still_skippable_for_fork() -> None:
|
||||
"""Fork PRs CAN still skip Pytest shards (not in FORK_NEVER_SKIP)."""
|
||||
assert _is_allow_skip("Pytest (runtime-core)", is_fork="true") is True
|
||||
|
||||
|
||||
def test_is_fork_unset_defaults_to_skippable() -> None:
|
||||
"""When IS_FORK is unset, the :-false default keeps ALLOW_SKIP intact."""
|
||||
env = os.environ.copy()
|
||||
env.pop("IS_FORK", None)
|
||||
proc = subprocess.run(
|
||||
[
|
||||
"bash",
|
||||
"-c",
|
||||
f'source "{SCRIPT}" && is_allow_skip "E2E Tests (shard 0/4)"',
|
||||
],
|
||||
env=env,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
assert proc.returncode == 0, "E2E should be skippable when IS_FORK is unset"
|
||||
Reference in New Issue
Block a user