Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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
90 changes: 49 additions & 41 deletions .github/workflows/codex-review-gate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ on:
issue_comment:
types:
- created
pull_request_review:
types:
- submitted

permissions:
contents: read
Expand Down Expand Up @@ -74,29 +71,6 @@ jobs:
--raw-field description="Codex review in progress" \
--raw-field target_url="${pr_url}"

complete:
name: Release the gate after Codex reviews the current commit
if: github.event_name == 'pull_request_review' && github.event.review.user.login == 'chatgpt-codex-connector[bot]'
runs-on: ubuntu-24.04
steps:
- name: Mark the reviewed PR commit successful
env:
GH_TOKEN: ${{ github.token }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
PR_URL: ${{ github.event.pull_request.html_url }}
REVIEW_SHA: ${{ github.event.review.commit_id }}
run: |
if [[ "${REVIEW_SHA}" != "${HEAD_SHA}" ]]; then
echo "Ignoring a Codex review for a stale PR commit."
exit 0
fi

gh api --method POST "repos/${GITHUB_REPOSITORY}/statuses/${HEAD_SHA}" \
--raw-field state=success \
--raw-field context="${STATUS_CONTEXT}" \
--raw-field description="Codex review complete" \
--raw-field target_url="${PR_URL}"

complete-comment:
name: Release the gate after a clean Codex review comment
if: >-
Expand All @@ -108,30 +82,49 @@ jobs:
COMMENT_BODY: ${{ github.event.comment.body }}
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.issue.number }}
CLEAN_REVIEW_MARKER: '^Codex Review: Didn''t find any major issues\.([[:space:]]+Keep it up!|[[:space:]]+:\+1:)?[[:space:]]*$'
CLEAN_REVIEW_MARKER: '^Codex Review: Didn''t find any major issues\.([^[:cntrl:]]*)$'
Comment thread
myagentdojo marked this conversation as resolved.
Resolved
CONCERNING_REVIEW_SUFFIX: '(^|[^[:alnum:]_])(but|concerns?|defects?|findings?|found|however|issues?|problems?|regressions?|risks?|suggestions?|p[0-3])([^[:alnum:]_]|$)'
Comment thread
myagentdojo marked this conversation as resolved.
Resolved
REVIEW_MARKER: '^\*\*Reviewed commit:\*\* `([0-9a-f]{10,40})`[[:space:]]*$'
steps:
- name: Mark the reviewed PR commit successful
shell: bash
run: |
clean_marker_count=0
while IFS= read -r line; do
if [[ "${line}" =~ ${CLEAN_REVIEW_MARKER} ]]; then
clean_marker_count=$((clean_marker_count + 1))
fi
done <<< "${COMMENT_BODY}"
if (( clean_marker_count != 1 )); then
echo "Ignoring a Codex comment without exactly one clean-review marker."
details_marker=$'\n<details> <summary>ℹ️ About Codex in GitHub</summary>'
if [[ "${COMMENT_BODY}" != *"${details_marker}"* ]] ||
[[ ! "${COMMENT_BODY}" =~ \</details\>[[:space:]]*$ ]]; then
echo "Ignoring a Codex comment without the expected informational-details boundary."
exit 0
fi

reviewed_prefix=""
review_payload="${COMMENT_BODY%%"${details_marker}"*}"
payload_line_count=0
clean_line=""
review_line=""
while IFS= read -r line; do
if [[ "${line}" =~ ${REVIEW_MARKER} ]]; then
reviewed_prefix="${BASH_REMATCH[1]}"
break
fi
done <<< "${COMMENT_BODY}"
[[ "${line}" =~ ^[[:space:]]*$ ]] && continue
payload_line_count=$((payload_line_count + 1))
case "${payload_line_count}" in
1) clean_line="${line}" ;;
2) review_line="${line}" ;;
esac
done <<< "${review_payload}"
if (( payload_line_count != 2 )) || [[ ! "${clean_line}" =~ ${CLEAN_REVIEW_MARKER} ]]; then
echo "Ignoring a Codex comment that is not an exact clean-review payload."
exit 0
fi
review_suffix="${BASH_REMATCH[1]}"
shopt -s nocasematch
if [[ "${review_suffix}" =~ ${CONCERNING_REVIEW_SUFFIX} ]]; then
echo "Ignoring a Codex clean marker with a contradictory suffix."
exit 0
fi
shopt -u nocasematch

if [[ ! "${review_line}" =~ ${REVIEW_MARKER} ]]; then
echo "Ignoring a Codex comment without an exact reviewed-commit marker."
exit 0
fi
reviewed_prefix="${BASH_REMATCH[1]}"
if [[ ! "${reviewed_prefix}" =~ ^[0-9a-f]{10,40}$ ]]; then
echo "Ignoring a Codex comment without an exact reviewed-commit marker."
exit 0
Expand All @@ -143,6 +136,21 @@ jobs:
exit 0
fi

codex_review_count="$(
gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" |
jq -s --arg head "${HEAD_SHA}" \
'[.[][] | select(.user.login == "chatgpt-codex-connector[bot]" and .commit_id == $head)] | length'
)"
codex_inline_count="$(
gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/comments" |
jq -s --arg head "${HEAD_SHA}" \
'[.[][] | select(.user.login == "chatgpt-codex-connector[bot]" and (.commit_id == $head or .original_commit_id == $head))] | length'
)"
if (( codex_review_count != 0 || codex_inline_count != 0 )); then
echo "Ignoring a Codex clean marker because the reviewed commit also has finding objects."
exit 0
fi

PR_URL="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/pull/${PR_NUMBER}"
gh api --method POST "repos/${GITHUB_REPOSITORY}/statuses/${HEAD_SHA}" \
--raw-field state=success \
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ jobs:
| jq -sc 'add | map({filename,status,sha}) | sort_by(.filename)')
printf '%s\n' "$projection" > repair-projection.json
base_parent=$(gh api "repos/${GITHUB_REPOSITORY}/commits/${candidate_sha}" --jq '.parents[0].sha')
# Re-run the trusted base parent's admission policy. Current projection
# rules may legitimately remove generated paths after an older release.
git checkout --detach "$base_parent"
projection_result=$(bun run scripts/release-projection.ts \
--base "$base_parent" \
--head "$candidate_sha" \
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ Hosted CI builds one candidate, then on Linux x64, Linux arm64, macOS arm64, and

### Optional Codex review gate

Require the `Codex review gate` status on `main` to make review opt-in without leaving every PR blocked. New PR commits start green. A maintainer with write access can comment `@codex review`; the status becomes pending until the ChatGPT Codex Connector submits a review for that exact commit. Push a new commit to reset the optional gate, then request another review when needed.
Require the `Codex review gate` status on `main` to make review opt-in without leaving every PR blocked. New PR commits start green. A maintainer with write access can comment `@codex review`; the status becomes pending until the ChatGPT Codex Connector reports a clean review for that exact commit. When Codex reports findings, resolve them, push the fixes, and request another review of the new commit.

Enable Codex code review for the repository before activating the required status. The gate proves completion only. Resolve any Codex findings through the normal review conversation rules.
Enable Codex code review for the repository before activating the required status. The gate proves an authenticated clean marker for the current commit and refuses completion when that commit also has Codex review or inline-finding objects. The review conversation remains the source of finding details.

## Release

Expand Down
86 changes: 67 additions & 19 deletions scripts/codex-review-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ type WorkflowJob = {
steps: Array<{ name?: string; run?: string }>
}

async function runCleanReviewComment(commentBody: string): Promise<string> {
async function runCleanReviewComment(
commentBody: string,
findingSurface: "review" | "inline" | null = null,
): Promise<string> {
const workflow = Bun.YAML.parse(await Bun.file(workflowUrl).text()) as {
jobs: Record<string, WorkflowJob>
}
Expand All @@ -29,7 +32,19 @@ async function runCleanReviewComment(commentBody: string): Promise<string> {
"bash",
"-c",
`gh() {
if [[ "$*" == *"pulls/13"* ]]; then
if [[ "$*" == *"pulls/13/reviews"* ]]; then
if [[ "$CODEX_FINDING_SURFACE" == "review" ]]; then
printf '%s\\n' '[{"user":{"login":"chatgpt-codex-connector[bot]"},"commit_id":"38d88841ee82cf88b52b9d27f08dd29347869581"}]'
else
printf '%s\\n' '[]'
fi
elif [[ "$*" == *"pulls/13/comments"* ]]; then
if [[ "$CODEX_FINDING_SURFACE" == "inline" ]]; then
printf '%s\\n' '[{"user":{"login":"chatgpt-codex-connector[bot]"},"commit_id":"38d88841ee82cf88b52b9d27f08dd29347869581"}]'
else
printf '%s\\n' '[]'
fi
elif [[ "$*" == *"pulls/13"* ]]; then
printf '%s\\n' '38d88841ee82cf88b52b9d27f08dd29347869581'
else
printf '%s\\n' "$*" >> "$GH_CALLS"
Expand All @@ -38,10 +53,12 @@ async function runCleanReviewComment(commentBody: string): Promise<string> {
export -f gh
${script}`,
],
env: {
...process.env,
COMMENT_BODY: commentBody,
CLEAN_REVIEW_MARKER: job.env.CLEAN_REVIEW_MARKER ?? "",
env: {
...process.env,
CODEX_FINDING_SURFACE: findingSurface ?? "",
COMMENT_BODY: commentBody,
CLEAN_REVIEW_MARKER: job.env.CLEAN_REVIEW_MARKER ?? "",
CONCERNING_REVIEW_SUFFIX: job.env.CONCERNING_REVIEW_SUFFIX ?? "",
GH_CALLS: callsPath,
GH_TOKEN: "test-token",
GITHUB_REPOSITORY: "myagentdojo/agent-plugin-template",
Expand All @@ -60,18 +77,23 @@ ${script}`,
}
}

const aboutCodexDetails = `

<details> <summary>ℹ️ About Codex in GitHub</summary>
Informational boilerplate may mention reviews and suggestions.
</details>`

test("Codex review gate is opt-in and fail-closed after a request", async () => {
const source = await Bun.file(workflowUrl).text()
const workflow = Bun.YAML.parse(source)

expect(workflow).toMatchObject({
on: {
pull_request_target: {
types: ["opened", "reopened", "synchronize"],
on: {
pull_request_target: {
types: ["opened", "reopened", "synchronize"],
},
issue_comment: { types: ["created"] },
},
issue_comment: { types: ["created"] },
pull_request_review: { types: ["submitted"] },
},
permissions: {
contents: "read",
"pull-requests": "read",
Expand All @@ -80,37 +102,63 @@ test("Codex review gate is opt-in and fail-closed after a request", async () =>
env: { STATUS_CONTEXT: "Codex review gate" },
})

expect(source.match(/--raw-field state=success/g)).toHaveLength(3)
expect(source.match(/--raw-field state=success/g)).toHaveLength(2)
expect(source.match(/--raw-field state=pending/g)).toHaveLength(1)
expect(source).toContain("collaborators/${COMMENTER}/permission")
expect(source).toContain("admin|maintain|write")
expect(source).toContain("chatgpt-codex-connector[bot]")
expect(source).toContain("github.event.comment.user.login == 'chatgpt-codex-connector[bot]'")
expect(source).toContain("reviewed_prefix=")
expect(source).toContain('"${HEAD_SHA}" != "${reviewed_prefix}"*')
expect(source).toContain('"${REVIEW_SHA}" != "${HEAD_SHA}"')
expect(source).not.toContain("github.event.review")
expect(source).not.toContain("actions/checkout")
})

test("clean Codex comment releases the gate only with the exact clean marker", async () => {
test("clean Codex comment releases the gate from its stable verdict sentence", async () => {
const calls = await runCleanReviewComment(
"Codex Review: Didn't find any major issues. Keep it up!\n\n**Reviewed commit:** `38d88841ee8`",
`Codex Review: Didn't find any major issues. Breezy!\n\n**Reviewed commit:** \`38d88841ee8\`${aboutCodexDetails}`,
)

expect(calls).toContain("--raw-field state=success")
})

test("Codex comment without a clean marker leaves the gate unchanged", async () => {
const calls = await runCleanReviewComment(
"Codex Review: Found a major issue.\n\n**Reviewed commit:** `38d88841ee8`",
`Codex Review: Found a major issue.\n\n**Reviewed commit:** \`38d88841ee8\`${aboutCodexDetails}`,
)

expect(calls).toBe("")
})

test("Codex comment with findings after the clean marker leaves the gate unchanged", async () => {
const calls = await runCleanReviewComment(
`Codex Review: Didn't find any major issues. Findings follow.\n\n**Reviewed commit:** \`38d88841ee8\`${aboutCodexDetails}`,
)

expect(calls).toBe("")
})

test("Codex comment with a separate finding paragraph leaves the gate unchanged", async () => {
const calls = await runCleanReviewComment(
`Codex Review: Didn't find any major issues. Bravo.\n\nFinding: a major defect remains.\n\n**Reviewed commit:** \`38d88841ee8\`${aboutCodexDetails}`,
)

expect(calls).toBe("")
})

test("Codex review finding object on the current commit leaves the gate unchanged", async () => {
const calls = await runCleanReviewComment(
`Codex Review: Didn't find any major issues. Breezy!\n\n**Reviewed commit:** \`38d88841ee8\`${aboutCodexDetails}`,
"review",
)

expect(calls).toBe("")
})

test("Codex comment with an invalid clean marker leaves the gate unchanged", async () => {
test("Codex inline finding on the current commit leaves the gate unchanged", async () => {
const calls = await runCleanReviewComment(
"Codex Review: Didn't find any major issues. Findings follow.\n\n**Reviewed commit:** `38d88841ee8`",
`Codex Review: Didn't find any major issues. Breezy!\n\n**Reviewed commit:** \`38d88841ee8\`${aboutCodexDetails}`,
"inline",
)

expect(calls).toBe("")
Expand Down
11 changes: 11 additions & 0 deletions scripts/release-validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,17 @@ test("release workflow is pinned and publishes proven assets after validation",
expect(workflow).toContain("admitted_automation_identity")
expect(workflow).toContain('--expected-automation-login "$admitted_automation_identity"')
expect(workflow).toContain("scripts/release-projection.ts")
const historicalPolicyCheckout = repairValidationStep.indexOf('git checkout --detach "$base_parent"')
const historicalPolicyExecution = repairValidationStep.indexOf("bun run scripts/release-projection.ts")
const provenanceGuard = repairValidationStep.indexOf('if [[ -z "$merged_at"')
const parentCountGuard = repairValidationStep.indexOf('if [[ "$parent_count" != "2" ]]')
expect(historicalPolicyCheckout).toBeGreaterThanOrEqual(0)
expect(historicalPolicyExecution).toBeGreaterThanOrEqual(0)
expect(provenanceGuard).toBeGreaterThanOrEqual(0)
expect(parentCountGuard).toBeGreaterThanOrEqual(0)
expect(historicalPolicyCheckout).toBeGreaterThan(provenanceGuard)
expect(historicalPolicyCheckout).toBeGreaterThan(parentCountGuard)
expect(historicalPolicyExecution).toBeGreaterThan(historicalPolicyCheckout)
expect(workflow).not.toContain("ALLOWED_RELEASE_PATHS")
expect(workflow).not.toContain("missing_paths")
expect(workflow).toContain("parent_count")
Expand Down