Skip to content

codex: use writer reviews for hidden members #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ttaylorr-oai merged 1 commit into from
Aug 9, 2026
Merged

codex: use writer reviews for hidden members #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 22 additions & 13 deletions .github/workflows/codex-branch.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,7 @@ authenticate_pending_codex_merge () (
die "could not authenticate the merged Codex pull request"
fi
has_qualifying_current_review "$state/reviews" "$author" "$head" \
openai/git "$state/review-candidates" ||
openai/git "$number" "$state/review-candidates" ||
die "pending Codex pull request has no qualifying approval"
printf '%s\t%s\t%s\t%s\n' "$name" "$head" "$number" "$merge" \
>"$output" || die "could not record the reviewed Codex admission"
Expand Down Expand Up @@ -5903,7 +5903,8 @@ has_qualifying_current_review () {
author=$2
head=$3
repository=$4
candidates=$5
pull_number=$5
candidates=$6
awk -F '\t' -v author="$author" -v head="$head" '
NF == 4 && $2 ~ /^(APPROVED|CHANGES_REQUESTED|DISMISSED)$/ {
state[$1] = $2
Expand All @@ -5923,17 +5924,24 @@ has_qualifying_current_review () {
case "$association" in
OWNER|MEMBER|COLLABORATOR) return 0 ;;
esac
# author_association is relative to the API caller. The
# Actions token can see an organization member as NONE, so
# fall back to the repository-scoped collaborator check.
if gh api --hostname github.com \
"repos/$repository/collaborators/$reviewer" \
--silent >/dev/null 2>&1
then
return 0
fi
done <"$candidates"
return 1
# author_association is relative to the API caller. The Actions
# token can see an organization member as NONE, so ask GitHub for
# the latest reviews from writers instead of trusting that field.
owner=${repository%%/*}
name=${repository#*/}
gh api --hostname github.com graphql \
-f 'query=query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){latestOpinionatedReviews(first:100,writersOnly:true){nodes{author{login}state commit{oid}}}}}}' \
-F owner="$owner" -F name="$name" -F number="$pull_number" \
--jq '.data.repository.pullRequest.latestOpinionatedReviews.nodes[] |
[(.author.login // "-"), .state,
(.commit.oid // "-")] | @tsv' \
>"$candidates-writers" 2>/dev/null || return 1
awk -F '\t' -v author="$author" -v head="$head" '
NF == 3 && $1 != author && $2 == "APPROVED" &&
$3 == head { approved++ }
END { exit !approved }
' "$candidates-writers"
}

validate_topic_review () {
Expand Down Expand Up @@ -6004,7 +6012,8 @@ validate_topic_review () {
.author_association] | @tsv' >"$tmp_dir/topic-reviews" ||
die "could not inspect reviews for topic pull request #$pull_number"
has_qualifying_current_review "$tmp_dir/topic-reviews" "$author" \
"$source_tip" "$repository" "$tmp_dir/topic-review-candidates" ||
"$source_tip" "$repository" "$pull_number" \
"$tmp_dir/topic-review-candidates" ||
die "topic pull request #$pull_number has no current approval for $source_tip"
say "validated reviewed topic pull request #$pull_number at $source_tip"
}
Expand Down
23 changes: 14 additions & 9 deletions t/t9905-codex-branch.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8940,9 +8940,13 @@ test_expect_success 'topic review requires the exact approved head' '
cat >topic-review-bin/gh <<-\EOF &&
#!/bin/sh
case " $* " in
*"repos/openai/git/collaborators/reviewer "*)
test "$FAKE_COLLABORATOR" = yes
exit
*" graphql "*)
if test "$FAKE_WRITER" = yes
then
printf "%s\t%s\t%s\n" reviewer APPROVED \
"$FAKE_WRITER_HEAD"
fi
exit 0
;;
*"pulls/42/reviews?"*)
printf "%s\t%s\t%s\t%s\n" reviewer APPROVED \
Expand Down Expand Up @@ -8993,20 +8997,21 @@ test_expect_success 'topic review requires the exact approved head' '
test_grep "has no current approval for $source" stale.err &&
env PATH="$PWD/topic-review-bin:$PATH" \
FAKE_HEAD="$source" FAKE_REVIEW_HEAD="$source" \
FAKE_ASSOCIATION=NONE FAKE_COLLABORATOR=yes \
FAKE_ASSOCIATION=NONE FAKE_WRITER=yes \
FAKE_WRITER_HEAD="$source" \
FAKE_DECISION=APPROVED sh "$codex_branch" \
validate-topic-review --pull-request 42 --lane codex \
--topic aa/codex/reviewed --source-tip "$source" \
>collaborator.out &&
test_grep "validated reviewed topic" collaborator.out &&
>writer.out &&
test_grep "validated reviewed topic" writer.out &&
test_expect_code 1 env PATH="$PWD/topic-review-bin:$PATH" \
FAKE_HEAD="$source" FAKE_REVIEW_HEAD="$source" \
FAKE_ASSOCIATION=NONE FAKE_COLLABORATOR=no \
FAKE_ASSOCIATION=NONE FAKE_WRITER=no \
FAKE_DECISION=APPROVED sh "$codex_branch" \
validate-topic-review --pull-request 42 --lane codex \
--topic aa/codex/reviewed --source-tip "$source" \
>outsider.out 2>outsider.err &&
test_grep "has no current approval for $source" outsider.err
>nonwriter.out 2>nonwriter.err &&
test_grep "has no current approval for $source" nonwriter.err
'

test_expect_success 'checked-in release recovery manifest is the bound incident' '
Expand Down