Compare commits

...

12 Commits

Author SHA1 Message Date
Tomu Hirata d3a57a12f4 refactor(cursor_executor): streamline agent.send call for improved readability
Consolidated the parameters of the agent.send method into a single line for better clarity and maintainability. This change enhances the readability of the code without altering its functionality.

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
2026-06-18 19:26:40 +09:00
Tomu Hirata 2f10a86195 fix(cursor): enable delta stream so TurnEndedUpdate usage arrives
The Cursor backend only sends interaction updates (including
TurnEndedUpdate with token usage) when the request includes
enableDeltas: true — set by passing SendOptions(on_delta=...) to
agent.send(). Without it, no interaction_update events arrive in the
stream and cost tracking silently produces nothing.

Also adds cacheReadTokens / cacheWriteTokens (the actual field names
the Cursor backend sends) to the normalization lookup.

Co-authored-by: Isaac
2026-06-18 19:17:21 +09:00
Tomu Hirata f45209e44a feat(cursor): evaluate PHASE_TOOL_CALL policy for native tools (#643)
* feat(cursor): evaluate PHASE_TOOL_CALL policy for native tools

Cursor's native tools (bash, file editing, etc.) previously bypassed all
tool-call policies. Now when a non-bridged tool call is observed in the
stream, the executor evaluates PHASE_TOOL_CALL and cancels the run on
DENY. Bridged (MCP-wrapped) tools are skipped since they're already
gated server-side via the dispatch bridge.

Co-authored-by: Isaac

* fix(cursor): fix lint formatting and strengthen policy test assertions

Address Polly review: fix any test fixture typos, assert ToolCallRequest
is observed in the bridged-skip test, assert event ordering in the DENY
test, and fix line-length formatting.

Co-authored-by: Isaac
2026-06-18 09:56:29 +00:00
Tomu Hirata 42a6ce5815 fix(ci): strip sub-agent preamble from Polly review comments (#646)
* fix(ci): strip sub-agent preamble from Polly review comments

Sub-agents (e.g. Codex) sometimes leak coordination narration
("I've dispatched the codex reviewer…") before the structured
review output. Post-process the output to trim everything before
the first markdown heading or horizontal rule.

Co-authored-by: Isaac

* fix(ci): use sentinel + robust fallback for preamble stripping

Address Polly review feedback:
- Prompt now asks the model to emit <!-- POLLY_REVIEW_START -->
  sentinel; stripping anchors on it deterministically
- Fallback heuristic covers #{1,6} headings (not just #{1,3})
- Anchors `---` to standalone lines to avoid matching table separators

Co-authored-by: Isaac
2026-06-18 18:55:27 +09:00
Serena Ruan cd91a621a2 fix(antigravity): accept new 'AQ' Google API key prefix in setup (#640)
* fix(antigravity): accept new 'AQ' Google API key prefix in setup

New Google API keys start with 'AQ' instead of the legacy 'AIza',
which triggered a spurious "doesn't start with 'AIza'. Store it
anyway?" prompt during `omni setup`. Broaden the soft prefix check
to accept both prefixes.

Co-authored-by: Isaac

* style: ruff format antigravity key prefix hint

Co-authored-by: Isaac

* chore: revert accidental uv.lock / package-lock.json drift

Co-authored-by: Isaac
2026-06-18 17:52:54 +08:00
Tomu Hirata ad5e9cc534 test: migrate REPL approval e2e tests to mock LLM (#641)
* test: migrate 6 REPL approval tests to mock LLM, skip 8 complex ones

6 tests (single approval, refusal, two-turn, approve-always,
label-driven approve/refuse) now run fully against the mock LLM
server. 8 tests that require tool-call/subagent/output-phase mock
support not yet available in REPL pexpect mode are guarded with
`if using_mock_llm: pytest.skip(...)` so they only run with a real
LLM key.

Co-authored-by: Isaac

* test: remove dead mock setup code from 8 skipped REPL approval tests

These tests skip under mock LLM, so the _configure_mock_* calls after
pytest.skip() were unreachable dead code. Remove those calls and the
now-unused mock_llm_server_url parameter from each test signature.

Co-authored-by: Isaac
2026-06-18 09:51:10 +00:00
Pat Sukprasert dcce5caa39 fix(openai-agents): honor ambient OPENAI_BASE_URL on spec api_key path (#645)
A baked executor.auth api_key is frequently a gateway PAT (detected from
OPENAI_API_KEY). When its companion base_url is dropped on the
daemon -> runner -> harness propagation chain (the spec-auth bake omits
base_url when OPENAI_BASE_URL is absent at materialization time; a reused
local daemon may predate the env var), the executor's api_key branch set
base_url=None and routed the gateway token to api.openai.com -> 401.

Fall back to the ambient OPENAI_BASE_URL (which the runner/harness inherit)
when no base_url override reached us, so the gateway target is present on
every turn. A genuine OpenAI key with no gateway anywhere still defaults to
api.openai.com (base_url=None).

Co-authored-by: Isaac
2026-06-18 17:42:40 +08:00
Tomu Hirata a7ae6bb7f7 ci: gate fork e2e on maintainer approval, make blocking (#636)
* ci: gate fork e2e on maintainer approval instead of label, make blocking

Replace the `e2e-approved` label gate with maintainer PR approval for
triggering e2e on fork PRs. The merge gate now blocks until e2e passes
after approval, instead of allowing fork PRs to merge with skipped e2e.

Co-authored-by: Isaac

* ci: make e2e/integration checks non-skippable for fork PRs

Add FORK_NEVER_SKIP list to required.sh so that is_allow_skip returns
false for e2e/integration checks when IS_FORK=true. This closes the
edge case where a fork PR could merge with e2e never having run (e.g.
if the mirror failed after approval). Pytest shards remain skippable
for fork PRs since they don't require secrets.

Co-authored-by: Isaac

* ci: address Polly review — cleanup on revocation, fork guard, relay scope

B1: Delete the stale mirror branch when should-mirror returns false on
workflow_dispatch (approval revoked / changes requested). Extend the
review relay to fire on all non-COMMENTED review states so dismissals
and changes-requested also trigger re-evaluation.

B2: The relay now fires on all decisive review states (not just
approved). The mirror workflow re-evaluates via should-mirror.sh and
either mirrors (approved) or cleans up (revoked).

B3: Add fork guard for workflow_dispatch in the mirror job — resolve
the PR and skip early for same-repo PRs.

Co-authored-by: Isaac

* ci: keep e2e-approved label as alternative gate alongside approval

The fork e2e mirror gate now accepts either condition:
  1. Maintainer PR approval (primary flow), OR
  2. e2e-approved label applied by a maintainer (escape hatch for
     running e2e without approving for merge)

Co-authored-by: Isaac
2026-06-18 18:39:19 +09:00
Pat Sukprasert 65058d3fba feat(ci): post Polly AI review as omnigent-ci[bot] (#642) 2026-06-18 09:31:18 +00:00
Pat Sukprasert faf67f4e34 ci(merge-ready): pin gate scripts to main, never the PR head (#639)
* ci(merge-ready): pin gate scripts to main, never the PR head

The "Check out scripts" step had no `ref:`, so on the `pull_request`
(automerge) event it checked out `refs/pull/N/merge` and on `check_suite`
the suite head SHA -- i.e. the PR's own copy of
`.github/scripts/merge-ready/required.sh` and `evaluate-checks.sh`.

`required.sh` is a generated file replaced wholesale on each sync, so a PR
branched before E2E was added to REQUIRED carried a stale list: labeling it
`automerge` evaluated the gate from the PR's old script and merged it
without E2E required. It is also a privilege escalation -- a same-repo PR
could edit its own gate scripts and self-merge under the job's
contents:write + auto-merge permissions.

Pin the checkout to `ref: main` so Merge Ready always evaluates with main's
gate logic regardless of trigger, matching fork-e2e-mirror.yml's
"trusted; never the PR head" pattern.

Co-authored-by: Isaac

* ci(merge-ready): trim comment to one line
2026-06-18 16:28:09 +07:00
Serena Ruan 8f21bdd5fd fix(ci): make skip-security-scan waiver label-only and fix rerun race (#637)
* fix(ci): make skip-security-scan waiver label-only and fix rerun race

The skip-security-scan waiver required BOTH the label AND a maintainer
approval (should-scan.sh). When those two events arrived apart (as on
#556, 8 min apart), the approval fired a premature relay while the scan
still failed, leaving gate runs in-progress; the decisive label-triggered
relay then hit `gh run rerun` on those in-flight runs, which GitHub
rejects ("could not re-run"), stranding stale failing checks (Lint,
Integration, E2E UI).

The approval half added no real authority: applying the label already
requires Triage permission, held only by write/admin collaborators, so a
fork author can never self-waive. Make the waiver label-only.

- should-scan.sh: replace skip_label_effective() (label + maintainer
  approval/author) with has_skip_label() (label presence only). Still
  fails closed on missing token/repo/PR. author_is_maintainer (private-
  membership author trust) is unchanged.
- security-scan.yml: drop the pull_request_review trigger; re-run on
  labeled/unlabeled only. Update the on-failure waiver message.
- rerun-security-gate.yml: drop the pull_request_review trigger; gate the
  record job on the skip label only.
- rerun-security-gate-run.yml: add a race guard -- wait for the head
  SHA's Security Scan check to complete and only re-run gate workflows
  once it has passed, so the relay never churns in-progress runs.

Co-authored-by: Isaac

* fix(ci): raise rerun-gate job timeout above the race-guard wait budget

The race guard can wait up to ~6 min for the Security Scan to settle, but
the job timeout was 5 min, so a slow scan could cancel the job before it
reached the rerun loop -- stranding the very gate re-runs the guard exists
to issue. Bump timeout-minutes to 10 to cover the wait plus download/rerun.

Co-authored-by: Isaac

* fix(ci): address PR review — single-call race guard, accurate triage wording

- rerun-security-gate-run.yml: fetch scan status+conclusion in ONE check-runs
  call (was two, a TOCTOU on which run is 'latest'); sort by monotonic id
  instead of started_at; document the >6-min scan timeout as a known gap.
- should-scan.sh: reword 'write/admin' to 'Triage (or higher)' and frame the
  'can already push' claim as an accepted repo-policy risk, not a GitHub
  guarantee; fix the waiver reason string accordingly.

Co-authored-by: Isaac
2026-06-18 17:23:01 +08:00
Tomu Hirata 612e6db792 ci: use pull_request_target in merge-ready so it always runs from main
A PR cannot modify the gate logic by editing merge-ready.yml since
pull_request_target always runs the workflow file from the base branch.

Co-authored-by: Isaac
2026-06-18 18:17:22 +09:00
26 changed files with 1174 additions and 337 deletions
+46 -40
View File
@@ -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"
+19 -16
View File
@@ -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.
+31 -5
View File
@@ -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
+33 -47
View File
@@ -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
+3 -2
View File
@@ -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).
+97 -31
View File
@@ -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:
+52 -16
View File
@@ -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.
#
+29 -2
View File
@@ -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 }}
+52 -2
View File
@@ -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
+12 -21
View File
@@ -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:
+2 -2
View File
@@ -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'
+12 -18
View File
@@ -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
View File
@@ -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)
+85 -8
View File
@@ -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
+13 -1
View File
@@ -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,
)
+10 -5
View File
@@ -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:
+139 -9
View File
@@ -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)],
+196 -3
View File
@@ -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.
+2 -1
View File
@@ -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("")
+64 -63
View File
@@ -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"]
+39 -38
View File
@@ -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"]
+102
View File
@@ -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"