Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the ALLOW_SKIP_VERSION_CHECK environment variable to conditionally control the version check skip mechanism in scripts/check-version-bumps.sh. A bug was identified in the git log command where the use of the symmetric difference operator (...) could cause version checks to be skipped incorrectly if the base branch contains the skip flag; it is recommended to use the double-dot syntax (..) instead to correctly isolate PR commits.
There was a problem hiding this comment.
Pull request overview
Phase 1 CI workflow redesign to prepare the repo for a future main-based GitHub merge queue, while keeping required check names stable and enabling reusable workflow composition for staging batching.
Changes:
- Make
test.ymlmerge-queue aware (merge_group) and reusable (workflow_call), with path/risk-based gating and a single protected roll-up check (Run Tests). - Update
code_style.ymlto run onmerge_groupwhile preserving the existing roll-up job name and adjusting clippy matrix behavior by event type. - Convert
replay-gate.ymlande2e.ymlinto reusable workflows, including E2Emode: smoke|fullselection and new CI wiring for expanded coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/check-version-bumps.sh |
Adds ALLOW_SKIP_VERSION_CHECK gating for skip behavior to support merge-queue-safe label handling. |
.github/workflows/test.yml |
Adds workflow_call + merge_group, expands change detection outputs, refactors job graph, and introduces new CI lanes (Slack + sharded Telegram + reusable dependencies). |
.github/workflows/replay-gate.yml |
Makes replay gate reusable via workflow_call and updates action pins/permissions. |
.github/workflows/e2e.yml |
Makes E2E reusable and adds mode-driven matrix configuration; removes overlapping PR trigger path. |
.github/workflows/code_style.yml |
Adds merge_group support, adjusts change detection, and updates clippy/no-panics gating while keeping the roll-up job name stable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bcff52b9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This PR implements Phase 1 of
#2719by redesigning CI onstagingto support the futuremainmerge-queue model.This PR only changes CI behavior and supporting workflow/script wiring. It does not change the default branch, branch protections, staging-promotion flow, or merge-policy configuration yet.
What Changed
test.ymlis now merge-queue aware, but direct PR triggering is limited tomain.github/workflows/test.ymlnow:merge_groupsupport for the future queue onmainworkflow_callfor the staging batch pathRun Testspull_requesttriggering tomainThis means:
stagingPRs keep the existing PR check surface for nowRun Testsbehavior is exercised throughworkflow_call, futuremainPRs, andmerge_groupCode style supports merge queue without changing the required check name
.github/workflows/code_style.ymlnow:merge_groupCode Style (fmt + gateway-js-syntax + clippy + deny)Replay and E2E are reusable workflow dependencies
.github/workflows/replay-gate.ymlis reusable soRun Testscan depend on replay safely.github/workflows/e2e.ymlis reusable withmode: smoke|fulle2e.ymlto avoid duplicate smoke + full PR runsVersion-bump skip behavior remains safe in merge queue
scripts/check-version-bumps.shnow supportsALLOW_SKIP_VERSION_CHECKtest.ymlresolvesskip-version-checklabels formerge_group, but only enables skip behavior when the queue candidate can be attributed to exactly one PR. Multi-PR queue candidates fail closed so skip labels cannot leak across queued PRs.Coverage Added In This PR
This branch now also closes a few deterministic coverage gaps that were already present in the repo but were not exercised by normal CI:
Slack Channel Teststests/slack_auth_integration.rstelegram_auth_integrationtarget instead of only one exact regression caseweb-regressionsgroup:test_auth_no_duplicate_response.pytest_message_persistence.pytest_pending_user_messages.pyv2-enginegroup:test_v2_engine_auth_flow.pytest_v2_engine_approval_flow.pytest_v2_engine_tool_lifecycle.pytest_v2_kernel_auth_preflight.pytest_v2_kernel_auth_gateway_flow.pytest_v2_thread_visibility.pyParallelization Changes
To avoid turning the broader deterministic coverage into one long serial lane:
e2e_thread_schedulingstays inHeavy Integration Teststelegram_auth_integrationnow runs in a separate two-way sharded job usingcargo nextest --partition hash:1/2andhash:2/2Important Warnings / Likely Failure Modes
These changes are expected to surface real failures that were previously untested in normal CI.
Most likely breakage points:
tests/slack_auth_integration.rswas not previously wired into CItelegram_auth_integrationtarget has much broader coverage than the previous single regression casev2-engineE2E group exercises auth, approval, tool lifecycle, and thread-visibility flows that were not part of the normal reusable E2E suiteENGINE_V2=truebehavior gapsweb-regressionsgroup covers history persistence, pending user messages, and duplicate auth-response suppressioncargo-nextestin CIOperational caveat:
staging, the new directRun TestsPR path does not execute on this PR itselfRun Testsgraph is primarily being prepared for:workflow_callfrom staging batch CImainPRsmerge_groupValidation
Validated locally:
git diff --checkpassesghsimulation:Not yet validated live on GitHub:
merge_grouppayload behaviormainRun Testsgraph on a realmainPRFollow-Up / Phase 2
After this lands on
staging:staging -> mainpromotion PR carrying the validated CI changes and remaining staged workmain:Stagingruleset first so~DEFAULT_BRANCHdoes not attach tomainmainmainmainstaging-promote/*PRsCloses the Phase 1 implementation work for
#2719.