Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions .github/workflows/codex_review_gate.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
name: Codex Review Gate

# Two-mode gate for the Codex GitHub App (chatgpt-codex-connector):
# Static gate plus advisory observer for the Codex GitHub App
# (chatgpt-codex-connector):
#
# 1. WAIT mode (PR opened/synchronized):
# Creates a pending check → polls for Codex review up to N minutes.
# If Codex responds → check reflects review state.
# If Codex times out → check passes (don't block when App is broken).
# 1. PR mode (PR opened/synchronized):
# Runs the blocking static security guard, then observes any current-head
# connector review without polling.
#
# 2. REACT mode (Codex submits review):
# Updates check instantly — CHANGES_REQUESTED → fail, APPROVED → pass.
# Reports the advisory review immediately. Connector verdicts never block;
# the repository-owned Codex PR Review workflow is the AI merge gate.
#
# No API keys needed — reads the App's review via GitHub API.

Expand Down Expand Up @@ -39,8 +40,6 @@ jobs:
timeout-minutes: 15
env:
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
CODEX_GATE_POLL_SECONDS: ${{ vars.CODEX_GATE_POLL_SECONDS || '30' }}
CODEX_GATE_MAX_WAIT_MINUTES: ${{ vars.CODEX_GATE_MAX_WAIT_MINUTES || '5' }}

steps:
- name: Checkout
Expand Down
6 changes: 5 additions & 1 deletion docs/ai_autonomy_architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,11 @@ AIAuditBridge 是 QuantStrategyLab 的 AI 审计控制面,负责:

Contract Oscillation Guard 是 `AIAuditBridge` 的中央 PR review gate 语义,不是要求每个消费者仓库新增一套 branch rule。消费者仍使用原有 required check、branch protection 和 merge queue;guard 不提供 label、admin 或人工确认绕过。

trusted review comment 只保存最近固定轮数、固定字节上限且脱敏后的 blocking finding 摘要,包括 head SHA、file、category、severity、description 和 suggestion。历史只能由已验证的 review bot comment 恢复;legacy comment 没有 history marker 时保持兼容,但既有 blocker 会被迁移为 `invalid_history` 并继续 fail closed,不能因一次 clean review 自动清除。畸形或超限 history 同样 fail closed。
仓库自有的 `Codex PR Review` 是唯一具有 finding fingerprint、contract arbitration、remediation/freeze 语义的 AI 阻断源。GitHub 官方 `chatgpt-codex-connector` review 仅作为 advisory:`Codex Review Gate` 仍执行可阻断的静态 secret/path/metadata 检查,但 connector 的 `CHANGES_REQUESTED` 不得进入自有 finding history,也不得触发 closure、freeze、merge 或 reslice。connector review 通过 `pull_request_review: submitted` 事件即时报告,不再轮询等待。

blocking finding 必须给出当前 diff 到真实 repository caller 或已声明 public untrusted boundary 的可达证据。私有 typed value 不因假设存在 raw JSON caller、`object.__new__`/`object.__setattr__`、自定义 stateful mapping、损坏私有文件或未来 consumer 而升级成阻断项;JSON/wire 审计只覆盖当前变更实际暴露的边界与契约。缺少具体 `evidence` 的 finding 可保留为 advisory,但不能阻断合并。

trusted review comment 只保存最近固定轮数、固定字节上限且脱敏后的 blocking finding 摘要,包括 head SHA、file、category、severity、evidence、description 和 suggestion。历史只能由已验证的 review bot comment 恢复;legacy comment 没有 history marker 时保持兼容,但既有 blocker 会被迁移为 `invalid_history` 并继续 fail closed,不能因一次 clean review 自动清除。畸形或超限 history 同样 fail closed。

若 `overflow` / `invalid_history` 状态中没有可供仲裁的 trusted prior finding,系统不得用空上下文自动 `clear`。此时需要人工确认 source-of-truth 后修复或删除损坏的 trusted bot state,再重新运行普通 required review check;这只恢复可审计状态,不直接放行 merge,也不绕过 branch protection。

Expand Down
11 changes: 8 additions & 3 deletions prompts/pr_review.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,23 @@ You are reviewing a pull request for a **production quantitative trading and dat
- Documentation quality
- Minor refactoring opportunities
- Test coverage suggestions
- Hypothetical hardening for unsupported inputs or callers that do not exist in the repository
- Object-forging or mutation escape hatches unless a repository-backed caller actually uses them

## Review completeness

- Review the entire diff holistically and report all independent actionable findings in one response. Do not stop after the first blocking issue.
- Review the entire diff holistically and report all independent reachable findings in one response. Do not stop after the first blocking issue.
- Do not invent backward-compatibility requirements that are absent from the repository and PR contract. If both explicitly define a clean-slate namespace, check for accidental legacy fallback instead of requesting dual-read or migration. This never overrides security or data-integrity findings.
- For public JSON/wire contracts, systematically check optional-key presence versus explicit null, recursive JSON-safe types, every identity-bearing integer range, one canonical timestamp representation, deterministic encode/decode round-trips and digests, deep immutability, and identifier/path safety.
- Emit a finding only when the current diff causes or exposes a defect on a supported input through a repository-backed caller or a declared public untrusted boundary. Encode evidence as `kind|path|line|symbol`, where `kind` is `repository_call` or `public_boundary`, `path` is repository-relative, and `symbol` appears on that exact line in the current checkout. Free-form evidence is advisory only.
- Do not invent a raw JSON/parser boundary for private typed values. Do not treat `object.__new__`, `object.__setattr__`, `dataclasses.replace`, custom stateful mappings, corrupted private files, or similar escape hatches as reachable unless the repository or PR contract explicitly exposes them.
- For JSON/wire code, check only requirements evidenced by the changed public boundary and its real callers. Do not expand the task into a generic canonicalization or adversarial-parser checklist.

## Severity definitions

| Severity | Definition | Example |
|----------|-----------|---------|
| critical | Causes data loss, security breach, or production crash | SQL injection, credential in plaintext, deletion without backup |
| high | Produces wrong results or breaks downstream systems | Wrong formula, API signature change, resource leak |
| high | A concrete supported call path produces wrong results or breaks downstream systems | Wrong formula, reachable API signature change, resource leak |
| medium | Degrades reliability or performance under load | Missing error handling, N+1 query, unbounded growth |
| low | Misleading or confusing but not dangerous | Stale comment, redundant code, unclear intent |

Expand All @@ -46,6 +50,7 @@ Return exactly one JSON object (do not wrap in markdown fences):
"category": "security",
"file": "path/to/file.py",
"line": 42,
"evidence": "repository_call|service/handler.py|42|review(request.body)",
"description": "What's wrong",
"suggestion": "How to fix it"
}
Expand Down
95 changes: 51 additions & 44 deletions scripts/gate_codex_app_review.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
#!/usr/bin/env python3
"""PR merge gate: static scan + Codex App review → job exit code = check status.
"""PR check: blocking static scan plus advisory Codex App review.

Two phases, zero API keys needed:
1. STATIC — scan diff for secrets, blocked files, metadata issues (<30s).
Fail job immediately on hard violations.
2. WAIT — poll for Codex GitHub App review up to N min.
Fail job on CHANGES_REQUESTED, pass on APPROVED/timeout.
3. REACT — on Codex bot review submitted: update instantly.
2. OBSERVE — read a current-head Codex GitHub App review without polling.
3. REACT — on Codex bot review submitted: report it immediately.

The workflow job IS the check — exit 0 = pass, exit 1 = fail.
Only the static guard can fail this check. The repository-owned Codex PR Review
workflow remains the authoritative AI review gate.
"""

from __future__ import annotations

import json
import os
import sys
import time
import urllib.error
import urllib.request
from pathlib import Path
Expand Down Expand Up @@ -66,13 +65,6 @@ def env(name: str, default: str = "") -> str:
return os.environ.get(name, default).strip()


def env_int(name: str, default: int) -> int:
try:
return int(env(name, str(default)))
except ValueError:
return default


def github_request(token: str, method: str, path: str,
payload: dict[str, Any] | None = None) -> Any:
url = f"{API_BASE}{path}" if not path.startswith("https://") else path
Expand Down Expand Up @@ -150,33 +142,53 @@ def run_static_guard(token: str, repo: str, pr_number: int) -> int:

# ─── app review ──────────────────────────────────────────────────────────────

def get_codex_review(token: str, repo: str, pr_number: int) -> dict[str, Any] | None:
def review_matches_head(review: dict[str, Any], head_sha: str) -> bool:
return type(review.get("commit_id")) is str and review["commit_id"] == head_sha


def get_codex_review(
token: str,
repo: str,
pr_number: int,
head_sha: str,
) -> dict[str, Any] | None:
reviews = github_request(token, "GET", f"/repos/{repo}/pulls/{pr_number}/reviews?per_page=100")
if not isinstance(reviews, list):
return None
for r in reversed(reviews):
if isinstance(r, dict) and (r.get("user") or {}).get("login") == BOT_LOGIN:
if (
isinstance(r, dict)
and (r.get("user") or {}).get("login") == BOT_LOGIN
and review_matches_head(r, head_sha)
):
return r
return None


def app_decision(review: dict[str, Any] | None) -> tuple[int, str, str]:
"""(exit_code, title, summary)"""
if review is None:
return (0, "Codex: no review — passed through",
"Codex did not respond in time. Merge allowed to avoid blocking development.")
return (
0,
"Codex advisory: no current-head review",
"No current-head connector review is available; this advisory does not block merge.",
)
state = (review.get("state") or "").strip().upper()
url = review.get("html_url", "")
body = (review.get("body") or "").strip()
at = review.get("submitted_at", "")

if state == "CHANGES_REQUESTED":
snippet = (body[:500] + "...") if len(body) > 500 else body
return (1, "Codex: changes requested — MERGE BLOCKED",
f"Codex **requested changes** at {at}.\n\n{snippet}\n\n[View review]({url})")
return (
0,
Comment on lines +183 to +184

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Re-check stale review events before passing the gate

In the pull_request_review path, main() skips run_static_guard() and returns this advisory decision directly; when a CHANGES_REQUESTED connector review for an older commit is delivered after a new push, this now creates a successful Codex Review Gate run without running the blocking static secret/path checks for the current head. In repositories that require this workflow check, that race can satisfy the required check for a head that has not passed the static guard, so the REACT branch should either verify review.commit_id == head_sha before returning success or run the static guard first.

Useful? React with 👍 / 👎.

"Codex advisory: changes requested",
f"Codex **requested changes** at {at}, but this advisory does not block merge.\n\n"
f"{snippet}\n\n[View review]({url})",
)
if state == "APPROVED":
return (0, "Codex: approved", f"Codex approved at {at}. [View review]({url})")
return (0, f"Codex: reviewed ({state.lower()})",
return (0, "Codex advisory: approved", f"Codex approved at {at}. [View review]({url})")
return (0, f"Codex advisory: reviewed ({state.lower()})",
f"Codex state `{state}` at {at}. Not blocking. [View review]({url})")


Expand Down Expand Up @@ -216,18 +228,28 @@ def main() -> int:
return 1
print("STATIC → clean")

# ── Phase 2: App review ───────────────────────────────────────────
# ── Phase 2: advisory App review ──────────────────────────────────
# REACT: Codex just submitted a review
review_event = event.get("review") or {}
if event_name == "pull_request_review" and (review_event.get("user") or {}).get("login") == BOT_LOGIN:
if (
event_name == "pull_request_review"
and (review_event.get("user") or {}).get("login") == BOT_LOGIN
):
if not review_matches_head(review_event, head_sha):
print("REACT → stale connector review ignored")
step_summary(
"## Codex advisory ignored\n\n"
"The submitted connector review does not match the current PR head."
)
return 0
rc, title, summary = app_decision(review_event)
print(f"REACT → exit={rc}: {title}")
step_summary(f"## {title}\n\n{summary}")
return rc

# WAIT: poll for existing or upcoming review
try:
existing = get_codex_review(token, repo, pr_number)
existing = get_codex_review(token, repo, pr_number, head_sha)
except RuntimeError:
existing = None

Expand All @@ -237,26 +259,11 @@ def main() -> int:
step_summary(f"## {title}\n\n{summary}")
return rc

poll_s = env_int("CODEX_GATE_POLL_SECONDS", 30)
max_w = env_int("CODEX_GATE_MAX_WAIT_MINUTES", 5)
deadline = time.time() + max_w * 60
print(f"WAIT → polling every {poll_s}s for up to {max_w}min")

while time.time() < deadline:
time.sleep(poll_s)
try:
review = get_codex_review(token, repo, pr_number)
except RuntimeError:
continue
if review is not None:
rc, title, summary = app_decision(review)
print(f"WAIT → found review → exit={rc}: {title}")
step_summary(f"## {title}\n\n{summary}")
return rc

# Timeout
print(f"TIMEOUT → Codex did not respond in {max_w}min; passing through")
step_summary(f"## Codex: timeout after {max_w}min\n\nPassed through to avoid blocking development.")
print("ADVISORY → no current-head connector review; not waiting")
step_summary(
"## Codex advisory pending\n\n"
"No current-head connector review is available. The event-driven review hook will report it when submitted."
)
return 0


Expand Down
Loading
Loading