diff --git a/.github/CODEX.md b/.github/CODEX.md index aa1c6b6d614f19..7e1e2232f24618 100644 --- a/.github/CODEX.md +++ b/.github/CODEX.md @@ -12,11 +12,11 @@ two ordered desired-state files: - `codex.plan` for production; - `codex-unstable.plan` for preview. -Each plan row names a topic ref, its exact reviewed source SHA, the source +Each plan row names a topic ref, its exact admitted source SHA, the source boundary used to replay it, and one prerequisite. The controller retains each -reviewed source object at `refs/heads/codex-pins/`. A mutable topic ref +admitted source object at `refs/heads/codex-pins/`. A mutable topic ref may advance later; that does not change a published generation until another -reviewed plan transition pins the new head. +approved plan transition pins the new head. `codex.config` is the realized ledger. Version 3 records the plan blob, source pins, generated topic tips, and lane output tips produced by the last @@ -31,31 +31,41 @@ active topics belong in `codex`. | Change | Human review | Bot action | Plan effect | | --- | --- | --- | --- | -| Add | Review the topic PR against its lane | Pin the approved head and open a plan PR | Append one topic with its uniquely inferred prerequisite | -| Alter | Review the updated topic PR against its lane | Pin the approved head and open a plan PR | Replace only that topic's source SHA and source boundary | +| Add | Review the topic PR against its lane | Pin its current head and open a plan PR | Append one topic with its uniquely inferred prerequisite | +| Alter | Keep the topic PR approved against its lane | Pin its current head and open a plan PR | Replace only that topic's source SHA and source boundary | | Remove | Review the generated plan PR | Open a plan PR from the explicit workflow dispatch | Delete one plan row | | Reorder | Review the generated plan PR | Open a plan PR from the explicit workflow dispatch | Move one existing row; keep its SHA, boundary, and prerequisite | An add or alter has one human decision: the topic PR. The pull request is review-only because the output-lane rulesets reject ordinary updates. When an approval is submitted, a scheduled scanner on the trusted default branch -notices the exact approved head and runs the plan producer. Operators can -dispatch the same scan immediately when waiting five minutes is undesirable. -That trusted run: +notices the approved topic and freezes its current head before running the +plan producer. Operators can dispatch the same scan immediately when waiting +five minutes is undesirable. That trusted run: 1. rechecks that the PR is open, same-repository, non-draft, aimed at the matching lane, and overall `APPROVED`; -2. requires an effective approval at the exact current head SHA; +2. requires an effective approval from a different repository writer; 3. asks the dedicated plan App to create immutable pins and a `codex-plan/*` branch; 4. opens a one-row plan PR against `meta`; and 5. runs the trusted admission check, which rechecks the topic approval and gives the mechanical plan approval. -The plan PR auto-merges with rebase after its required check passes. If the -topic head changes before that happens, the exact-head check fails. A new -approval produces a new pin and a new plan transition. Once the plan merges, -the immutable pin remains authoritative even if the source branch moves. +An approval remains effective across later topic updates unless it is +dismissed or the reviewer supersedes it with a request for changes. The plan PR +auto-merges with rebase after its required check passes. If the topic head +changes before that happens, the pinned-head check fails; the next scan can +reuse the effective approval and create a new pin and plan transition. Once +the plan merges, the immutable pin remains authoritative even if the source +branch moves. + +After publication, the publisher closes a review-only topic PR only when its +head still matches both the approved plan and the published source pin. A +rebased topic may not appear verbatim in the generated lane, so GitHub shows +that PR as closed rather than merged. Staging alone never closes a PR, and a +closure failure cannot undo an otherwise successful publication. A later +change to the same topic needs another topic PR. Remove and reorder are policy decisions rather than projections of a reviewed topic head. Run **Actions > Refresh codex > Run workflow** with @@ -100,7 +110,7 @@ For a local diagnostic of the same projection: Meta/codex propose-plan --remote origin \ --lane codex-unstable \ --topic tb/codex/my-topic-unstable \ - --source-tip \ + --source-tip \ --review-pr \ --action auto --no-push ``` @@ -137,8 +147,8 @@ then run a fresh rebuild. For a pinned merge-shaped source, the controller uses its reviewed `source-base` as the exact old root and preserves the DAG across a moved generated base only when the two changed-path sets are disjoint. An overlapping base move or a graph with different reviewed roots -fails closed; restack and obtain a new exact-head review instead of flattening -or guessing. +fails closed; restack the approved topic and pin its new head instead of +flattening or guessing. ## Required automation topic diff --git a/.github/workflows/codex-branch.sh b/.github/workflows/codex-branch.sh index 12743516da525c..bf9c7b59b68d91 100755 --- a/.github/workflows/codex-branch.sh +++ b/.github/workflows/codex-branch.sh @@ -5905,7 +5905,9 @@ has_qualifying_current_review () { repository=$4 pull_number=$5 candidates=$6 - awk -F '\t' -v author="$author" -v head="$head" ' + require_exact_head=${7:-true} + awk -F '\t' -v author="$author" -v head="$head" \ + -v exact="$require_exact_head" ' NF == 4 && $2 ~ /^(APPROVED|CHANGES_REQUESTED|DISMISSED)$/ { state[$1] = $2 commit[$1] = $3 @@ -5915,7 +5917,7 @@ has_qualifying_current_review () { for (reviewer in state) if (reviewer != author && state[reviewer] == "APPROVED" && - commit[reviewer] == head) + (exact == "false" || commit[reviewer] == head)) print reviewer "\t" association[reviewer] } ' "$reviews" >"$candidates" || return 1 @@ -5937,9 +5939,10 @@ has_qualifying_current_review () { [(.author.login // "-"), .state, (.commit.oid // "-")] | @tsv' \ >"$candidates-writers" 2>/dev/null || return 1 - awk -F '\t' -v author="$author" -v head="$head" ' + awk -F '\t' -v author="$author" -v head="$head" \ + -v exact="$require_exact_head" ' NF == 3 && $1 != author && $2 == "APPROVED" && - $3 == head { approved++ } + (exact == "false" || $3 == head) { approved++ } END { exit !approved } ' "$candidates-writers" } @@ -6013,8 +6016,8 @@ validate_topic_review () { die "could not inspect reviews for topic pull request #$pull_number" has_qualifying_current_review "$tmp_dir/topic-reviews" "$author" \ "$source_tip" "$repository" "$pull_number" \ - "$tmp_dir/topic-review-candidates" || - die "topic pull request #$pull_number has no current approval for $source_tip" + "$tmp_dir/topic-review-candidates" false || + die "topic pull request #$pull_number has no qualifying approval" say "validated reviewed topic pull request #$pull_number at $source_tip" } @@ -9566,6 +9569,101 @@ stage_and_wait_for_ci () { fi } +close_published_topic_review () ( + controller=$1 + updates=$2 + plan_commit=$3 + seen=$4 + review=$(plan_trailer_optional "$plan_commit" Codex-Plan-Review \ + "Codex-Plan-Review") || return 1 + test -n "$review" || return 0 + case "$review" in + *[!0-9]*) return 1 ;; + esac + action=$(plan_trailer_one "$plan_commit" Codex-Plan-Action \ + "Codex-Plan-Action") || return 1 + case "$action" in + add|alter) ;; + *) return 0 ;; + esac + lane=$(plan_trailer_one "$plan_commit" Codex-Plan-Lane \ + "Codex-Plan-Lane") || return 1 + case "$lane" in + codex|codex-unstable) ;; + *) return 1 ;; + esac + topic=$(plan_trailer_one "$plan_commit" Codex-Plan-Topic \ + "Codex-Plan-Topic") || return 1 + topic=${topic#refs/heads/} + source_tip=$(plan_trailer_one "$plan_commit" Codex-Plan-Source-Tip \ + "Codex-Plan-Source-Tip") || return 1 + published_meta=$(awk -F '\t' \ + '$1 == "refs/heads/meta" { print $3 }' "$updates") || return 1 + published_output=$(awk -F '\t' -v ref="refs/heads/$lane" \ + '$1 == ref { print $3 }' "$updates") || return 1 + test -n "$published_meta" && test -n "$published_output" || return 1 + make_tmp_dir + review_state=$tmp_dir/published-topic-review + mkdir -p "$review_state" || return 1 + git show "$published_meta:$lane.plan" >"$review_state/plan" || return 1 + git show "$published_meta:$meta_config_path" \ + >"$review_state/config" || return 1 + planned_tip=$(git config --no-includes --file "$review_state/plan" \ + --get "branch.$topic.source-tip" || :) + test "$planned_tip" = "$source_tip" || return 0 + ledger_tip=$(git config --no-includes --file "$review_state/config" \ + --get "branch.$topic.source-tip" || :) + ledger_output=$(git config --no-includes --file "$review_state/config" \ + --get "$lane.output-tip" || :) + generated_tip=$(git config --no-includes --file "$review_state/config" \ + --get "branch.$topic.codex-tip" || :) + test "$ledger_tip" = "$source_tip" && + test "$ledger_output" = "$published_output" && + test -n "$generated_tip" || return 1 + git merge-base --is-ancestor "$generated_tip" \ + "$published_output" || return 1 + test "$(remote_head_oid origin refs/heads/meta)" = \ + "$published_meta" || return 1 + test "$(remote_head_oid origin "refs/heads/$lane")" = \ + "$published_output" || return 1 + test "$(remote_head_oid origin "refs/heads/$topic")" = \ + "$source_tip" || return 0 + if grep -F -x "$review" "$seen" >/dev/null + then + return 0 + fi + printf '%s\n' "$review" >>"$seen" || return 1 + gh api --hostname github.com "repos/openai/git/pulls/$review" \ + --jq '[.state, (.draft | tostring), .base.ref, + (.head.repo.full_name // "-"), .head.ref, .head.sha] | @tsv' \ + >"$review_state/pull-request" || return 1 + IFS="$tab" read -r pull_state draft base head_repository \ + head_ref head_sha <"$review_state/pull-request" || return 1 + test "$pull_state" != closed || return 0 + test "$pull_state" = open && test "$draft" = false && + test "$base" = "$lane" && test "$head_repository" = openai/git && + test "$head_ref" = "$topic" && test "$head_sha" = "$source_tip" || + return 0 + gh pr close "$review" --repo github.com/openai/git || return 1 + say "Closed reviewed topic pull request #$review after publishing $lane." +) + +close_published_topic_reviews () ( + controller=$1 + updates=$2 + make_tmp_dir + review_history=$tmp_dir/published-topic-review-history + reviewed=$tmp_dir/published-topic-review-seen + : >"$reviewed" || return 1 + git rev-list --first-parent --max-count=64 "$controller" -- \ + codex.plan codex-unstable.plan >"$review_history" || return 1 + while IFS= read -r plan_commit + do + close_published_topic_review "$controller" "$updates" \ + "$plan_commit" "$reviewed" || return 1 + done <"$review_history" +) + rebuild_codex_locally () { prepare_local_candidate verify_local_candidate "$local_candidate_dir" @@ -9580,6 +9678,11 @@ rebuild_codex_locally () { --updates "$local_candidate_dir/codex-updates" \ --require-automation say "Published codex candidate $candidate from local preparation session $session." + if ! close_published_topic_reviews "$controller_oid" \ + "$local_candidate_dir/codex-updates" + then + say "warning: publication succeeded, but its reviewed topic pull request could not be closed." + fi say "Generated commits identify $bot_name <$bot_email>; the push uses your configured origin credentials." } @@ -9699,6 +9802,11 @@ publish_run () { --inputs "$metadata/codex-inputs" \ --updates "$metadata/codex-updates" --require-automation say "Published codex candidate $artifact_candidate from Actions run $run_id." + if ! close_published_topic_reviews "$run_controller" \ + "$metadata/codex-updates" + then + say "warning: publication succeeded, but its reviewed topic pull request could not be closed." + fi say "Generated commits identify $bot_name <$bot_email>; the push uses your configured origin credentials." } diff --git a/t/t9905-codex-branch.sh b/t/t9905-codex-branch.sh index b9f9cb20c553c0..c50907f444b0a0 100755 --- a/t/t9905-codex-branch.sh +++ b/t/t9905-codex-branch.sh @@ -1549,7 +1549,7 @@ test_expect_success 'generated lanes are output-only and meta gates review in th " "$codex_root/.github/rulesets/codex-meta.json" ' -test_expect_success 'plan admission checks exact reviewed heads from trusted meta' ' +test_expect_success 'plan admission checks pinned topic heads from trusted meta' ' test_path_is_file "$codex_plan_admission_workflow" && test_grep "name: Codex plan admission" \ "$codex_plan_admission_workflow" && @@ -8935,7 +8935,7 @@ test_expect_success 'explicit reorder and remove stay plan-only policy changes' ) ' -test_expect_success 'topic review requires the exact approved head' ' +test_expect_success 'topic approval survives updates to its pinned head' ' mkdir topic-review-bin && cat >topic-review-bin/gh <<-\EOF && #!/bin/sh @@ -8951,6 +8951,12 @@ test_expect_success 'topic review requires the exact approved head' ' *"pulls/42/reviews?"*) printf "%s\t%s\t%s\t%s\n" reviewer APPROVED \ "$FAKE_REVIEW_HEAD" "${FAKE_ASSOCIATION:-MEMBER}" + if test -n "${FAKE_LATEST_REVIEW_STATE:-}" + then + printf "%s\t%s\t%s\t%s\n" reviewer \ + "$FAKE_LATEST_REVIEW_STATE" "$FAKE_REVIEW_HEAD" \ + "${FAKE_ASSOCIATION:-MEMBER}" + fi exit 0 ;; *"pulls/42 "*) @@ -8972,8 +8978,6 @@ test_expect_success 'topic review requires the exact approved head' ' source=$(git rev-parse HEAD) && test_commit review-moved && moved=$(git rev-parse HEAD) && - test_commit review-old && - old=$(git rev-parse HEAD) && test_expect_code 1 env PATH="$PWD/topic-review-bin:$PATH" \ FAKE_HEAD="$moved" FAKE_REVIEW_HEAD="$source" \ FAKE_DECISION=APPROVED sh "$codex_branch" \ @@ -8988,22 +8992,33 @@ test_expect_success 'topic review requires the exact approved head' ' --topic aa/codex/reviewed --source-tip "$source" \ >decision.out 2>decision.err && test_grep "is not approved" decision.err && - test_expect_code 1 env PATH="$PWD/topic-review-bin:$PATH" \ - FAKE_HEAD="$source" FAKE_REVIEW_HEAD="$old" \ + env PATH="$PWD/topic-review-bin:$PATH" \ + FAKE_HEAD="$moved" FAKE_REVIEW_HEAD="$source" \ FAKE_DECISION=APPROVED sh "$codex_branch" \ validate-topic-review --pull-request 42 --lane codex \ - --topic aa/codex/reviewed --source-tip "$source" \ - >stale.out 2>stale.err && - test_grep "has no current approval for $source" stale.err && + --topic aa/codex/reviewed --source-tip "$moved" \ + >updated.out && + test_grep "validated reviewed topic" updated.out && env PATH="$PWD/topic-review-bin:$PATH" \ - FAKE_HEAD="$source" FAKE_REVIEW_HEAD="$source" \ + FAKE_HEAD="$moved" FAKE_REVIEW_HEAD="$source" \ 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" \ + --topic aa/codex/reviewed --source-tip "$moved" \ >writer.out && test_grep "validated reviewed topic" writer.out && + for state in CHANGES_REQUESTED DISMISSED + do + test_expect_code 1 env PATH="$PWD/topic-review-bin:$PATH" \ + FAKE_HEAD="$moved" FAKE_REVIEW_HEAD="$source" \ + FAKE_LATEST_REVIEW_STATE="$state" FAKE_WRITER=no \ + FAKE_DECISION=APPROVED sh "$codex_branch" \ + validate-topic-review --pull-request 42 --lane codex \ + --topic aa/codex/reviewed --source-tip "$moved" \ + >"$state.out" 2>"$state.err" || return 1 + test_grep "has no qualifying approval" "$state.err" || return 1 + done && test_expect_code 1 env PATH="$PWD/topic-review-bin:$PATH" \ FAKE_HEAD="$source" FAKE_REVIEW_HEAD="$source" \ FAKE_ASSOCIATION=NONE FAKE_WRITER=no \ @@ -9011,7 +9026,116 @@ test_expect_success 'topic review requires the exact approved head' ' validate-topic-review --pull-request 42 --lane codex \ --topic aa/codex/reviewed --source-tip "$source" \ >nonwriter.out 2>nonwriter.err && - test_grep "has no current approval for $source" nonwriter.err + test_grep "has no qualifying approval" nonwriter.err +' + +test_expect_success 'publication closes only its exact integrated topic review' ' + git init --bare published-review.git && + test_create_repo published-review-source && + ( + cd published-review-source && + git remote add origin ../published-review.git && + test_commit published-review-base && + base=$(git rev-parse HEAD) && + git branch codex && + git switch -c aa/codex/reviewed-unstable && + test_commit published-review-topic && + source=$(git rev-parse HEAD) && + git switch -c codex-unstable "$base" && + test_commit published-review-generated && + generated=$(git rev-parse HEAD) && + git switch -c meta "$base" && + git config --file codex.config codex-unstable.output-tip "$base" && + git config --file codex.config \ + branch.aa/codex/reviewed-unstable.source-tip "$base" && + git config --file codex.config \ + branch.aa/codex/reviewed-unstable.codex-tip "$base" && + git add codex.config && + git commit -m "previous published preview" && + printf "%s\t%s\t%s\t%s\n" aa/codex/reviewed-unstable \ + "$source" "$generated" codex >rows && + write_pinned_plan codex-unstable codex rows \ + codex-unstable.plan && + git add codex-unstable.plan && + { + printf "Codex plan: alter reviewed preview\n\n" && + printf "Codex-Plan-Lane: codex-unstable\n" && + printf "Codex-Plan-Action: alter\n" && + printf "Codex-Plan-Topic: aa/codex/reviewed-unstable\n" && + printf "Codex-Plan-Source-Tip: %s\n" "$source" && + printf "Codex-Plan-Review: 42\n" + } >message && + git commit -F message && + write policy controller-policy && + git add controller-policy && + git commit -m "unrelated controller update" && + controller=$(git rev-parse HEAD) && + git config --file codex.config \ + codex-unstable.output-tip "$generated" && + git config --file codex.config \ + branch.aa/codex/reviewed-unstable.source-tip "$source" && + git config --file codex.config \ + branch.aa/codex/reviewed-unstable.codex-tip "$generated" && + git add codex.config && + git commit -m "record published preview" && + published_meta=$(git rev-parse HEAD) && + git push origin meta codex codex-unstable \ + aa/codex/reviewed-unstable && + { + printf "refs/heads/meta\t%s\t%s\n" \ + "$controller" "$published_meta" && + printf "refs/heads/codex\t%s\t%s\n" "$base" "$base" && + printf "refs/heads/codex-unstable\t%s\t%s\n" \ + "$base" "$generated" + } >updates && + mkdir close-bin && + cat >close-bin/gh <<-\EOF && + #!/bin/sh + if test "$1" = pr && test "$2" = close + then + printf "%s\n" "$*" >>"$FAKE_CLOSE_LOG" + test "${FAKE_CLOSE_MODE:-}" != failure || exit 93 + printf "%s\n" closed >"$FAKE_CLOSE_STATE" + exit 0 + fi + test "$1" = api && test "$2" = --hostname && + test "$3" = github.com && + test "$4" = repos/openai/git/pulls/42 || exit 92 + state=$(cat "$FAKE_CLOSE_STATE" 2>/dev/null || printf open) + head=$FAKE_HEAD + test "${FAKE_CLOSE_MODE:-}" != moved || head=$FAKE_OTHER + printf "%s\tfalse\tcodex-unstable\topenai/git\taa/codex/reviewed-unstable\t%s\n" \ + "$state" "$head" + EOF + chmod +x close-bin/gh && + cat >close-review <<-\EOF && + #!/bin/sh + set -- --help + . "$CODEX_BRANCH" >/dev/null + close_published_topic_reviews "$FAKE_CONTROLLER" "$FAKE_UPDATES" + EOF + close_review () { + env PATH="$PWD/close-bin:$PATH" CODEX_BRANCH="$codex_branch" \ + FAKE_CONTROLLER="$controller" FAKE_UPDATES="$PWD/updates" \ + FAKE_HEAD="$source" FAKE_OTHER="$base" \ + FAKE_CLOSE_LOG="$PWD/closed" \ + FAKE_CLOSE_STATE="$PWD/close-state" \ + FAKE_CLOSE_MODE="${1:-}" sh close-review + } && + : >closed && + close_review moved && + test_must_be_empty closed && + close_review >close.out && + test_grep "Closed reviewed topic pull request #42" close.out && + test_grep "pr close 42 --repo github.com/openai/git" closed && + close_review && + test_line_count = 1 closed && + rm -f close-state && + test_expect_code 1 close_review failure && + test_line_count = 2 closed && + test "$source" = "$(git --git-dir=../published-review.git \ + rev-parse refs/heads/aa/codex/reviewed-unstable)" + ) ' test_expect_success 'checked-in release recovery manifest is the bound incident' '