Repository navigation
ci: preserve required checks after pull request edits - #1944
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks! Owner-level merge-safety review found no blocking issues. The metadata job with a fixed command and empty token permissions preserves source-check routing, names and cancellation isolation. All 325 focused workflow/release tests, 10 selector/gate tests, types, formatting and workflow lint passed. The planned same-head hosted acceptance check is still needed to establish the merge-eligibility effect; local routing tests cannot establish that, and the linked historical ruleset evaluation returned 404.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks — the shared timeout assertion fails the required TypeScript check locally and in this head’s hosted check; see the inline finding on #1945. Please apply the same type fix here. All 17 changed files and the prerequisite routing and forwarding changes were reviewed. The 442 focused Bun tests, 22 direct Node tests, SDK formatting and workflow lint passed; no additional substantive findings. Successful hosted source reuse remains unverified at this failing head.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks — the timeout-type issue is fixed. Reviewed all 17 changed files and the prerequisite context; 442 focused SDK tests, 21 Node tests, SDK/MCP types and formatting pass. No new confirmed source defect.
The remaining material gap is the planned hosted acceptance of the main behavior: a green source run followed by an ordinary edit that reuses it and preserves the required checks and merge eligibility. This head’s source run failed, so that path remains unverified. The new Windows report fixture returned a null exit status after about 10 seconds, while the identical fixture passed on the diagnostic’s Windows run; a macOS archive test also timed out. Those results do not establish a deterministic code bug. Please obtain clean current-head source qualification and complete the planned same-head edit check to finish the owner-level merge-safety pass.
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks — confirmed that this empty commit has the same source tree as the previous review. Rechecked all 17 changed files and the prerequisite context; the fresh 21 selector/source-verification tests pass. The prior 442 focused SDK tests, SDK/MCP typechecks and formatting results apply to this identical tree. No new confirmed source defect.
The owner-level merge-safety pass still needs the planned positive hosted acceptance: a green source followed by an ordinary same-head edit that reuses it and preserves the required checks and merge eligibility. The fresh source run is still in progress, with no edit run on this head yet. Please complete that acceptance sequence before merge.
982d1a7 to
0faaa3d
Compare
2dc84b6 to
4d8ddc7
Compare
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for preserving the required checks through PR edits. No blocking findings. Source reuse verifies the PR, head, base, tested merge and first-attempt history, then rechecks the evidence at the final gate; partial retries are guarded separately. Locally, 21 Node tests, 38 focused Bun tests (468 assertions), 14 CLI/gate probes, SDK/MCP types, formatting and workflow lint passed. The earlier-head hosted body and title edits successfully reused the source and reported every required check; the verifier and relevant workflow settings match this head. Current-head CI is still running, and current-head edit acceptance was not exercised.
Summary
Updating a pull request body can block an approved change after its required checks passed. In the first experiment on this PR, the source run passed and GitHub reported a clean merge state. A body edit made GitHub report it blocked, even though a separately named metadata job and the title check both succeeded on the same head. Original source run · Original metadata run
Changes
This eight-file change is stacked on the forwarding PR, which itself depends on the routing PR. Routing prerequisite · Forwarding prerequisite
Testing
2dc84b6epassed all 69 jobs in 18m19s. The classifier selected real checks and the three required OS checks passed. Protected source run92074d06completed with two fixture failures. The macOS archive check reached 30.422597 seconds on its second child invocation. The Windows Python report fixture returned no exit status on its first invocation after 10.123105 seconds. Both original jobs and their red required checks remain recorded. Four matching Windows controls completed all seven assertions in 0.32–1.18 seconds. Both fixtures have independent repairs. macOS failure · Windows failure · Archive fixture repair · Native report fixture repair2dc84b6estarted the new first-attempt qualification. Its source tree is byte-identical to92074d06(2881f98d). A rerun of a failed attempt cannot authorize the fast edit path.Risk and rollout
The conservative policy can re-run the full matrices for subsequent edits on an ambiguous head. A fresh source commit starts a new history. Land the stale-retarget guard before relying on retarget qualification. Removing the obsolete PR release-progress listener also avoids a housekeeping job when Actions displays a custom PR run name. Retarget guard · Release-progress cleanup
The closed diagnostic PR also exercised a fresh head with no completed source. After its source was cancelled, the edit selected full qualification and ran all 69 jobs. The deliberate static error and all three required OS checks failed. Full qualification after cancellation
Public disclosure review