Skip to content

ci: cancel superseded Full CI runs per PR - #294

Merged
jasonqinzhou merged 6 commits into
mainfrom
codex/full-ci-pr-concurrency
Sep 19, 2026
Merged

jasonqinzhou merged 6 commits into
mainfrom
codex/full-ci-pr-concurrency

Conversation

@jasonqinzhou

@jasonqinzhou jasonqinzhou commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Why and what changed

Full CI currently lets superseded PR revisions keep consuming runners. A second push to pull-request/159, for example, can leave both revisions' build/test matrices active.

Add workflow-level concurrency that replaces older queued and running validation for the same trusted PR copy. Automatic and manual runs on pull-request/N share a group. Manual source-branch runs replace only that branch's runs. Different PRs remain independent, and every main/release/tag run keeps a unique group.

Review map

  • Risk level: Medium — changes cancellation of active CI work.
  • Start with: .github/workflows/ci.yml (group selection and cancellation); tests/test_ci_workflow_contracts.py (group identity and protected refs); docs/ci.md (current-SHA PR admission, manual-run limitations, and retry guidance).
  • Public or serialized contract changed: No product API/schema changes. Full CI now cancels earlier runs in the same eligible ref group. Required check names, permissions, selection, and coverage are unchanged.
  • Compatibility or rollback concern: Existing manual callers remain valid. Trusted-copy manual runs share the automatic cancellation group, but GitHub excludes manual job checks from required PR rulesets. Preserve eligible push-triggered validation; manual source-branch runs provide diagnostic evidence. Revert the concurrency block to restore independent runs. The policy applies only after branches incorporate this workflow; existing runs are not retroactively canceled. An old-revision rerun can replace a newer run, so replacement validation should target the current PR head.

Evidence

Validated head: 72c3e703ae01c3b69f4a625afcdc972d916cf4db, incorporating main 7ae483e942b0a2593d55e4e30957f94640580441. Squash-merged into main as c8ba25e8ff9d7c17dbb3d980e4c2311071e0c2bd on 2026-09-19 at 03:19:09 UTC. The three-file feature diff is unchanged by the integration merge.

  • Local tests: 316 passed on this head with Python 3.12 and the bundled Node runtime. Command: python -m pytest -c /dev/null tests/test_ci_workflow_contracts.py tests/test_ci_qualification.py tests/test_release_fpe.py -q -p no:cacheprovider --override-ini=filterwarnings=error. The environment includes the CI-pinned pip-licenses==5.5.5 dependency.
  • Current-head checks: Ruff lint and format, copyright headers, packaged legal files, local documentation links, and git diff --check pass.
  • Codex review: fresh complete-diff and surrounding-workflow review at 72c3e703ae01c3b69f4a625afcdc972d916cf4db found no actionable issue. Both previous findings remain fixed and their threads are resolved.
  • CodeRabbit: current-head review found no actionable issue. The bot disclosed its local history limitation; the full feature diff and main integration were independently verified locally.
  • Fast CI: PR run 35417501128 passed on this head; codeowners and DCO also pass.
  • Trusted merge-gate CI: Fast CI 35417629984 and Full CI 35417630105 both passed after exact-head admission. Full CI completed 54 successful jobs with one expected PR release-staging skip. pull-request/294 was verified to equal 72c3e703ae01c3b69f4a625afcdc972d916cf4db. The earlier green runs are historical evidence only.
  • Merge verification: applicable human approval was retained, both review threads are resolved, and required Fast CI, Full CI and codeowners checks passed. The resulting squash commit is on main, has parent 7ae483e942b0a2593d55e4e30957f94640580441, and exactly matches the validated tree 8c80e62912b7a3c5ceb30c088c84b8e19f66d12c; git diff between validated head and merge commit is empty.
  • Negative or boundary cases: same-PR push/manual identity; different PR isolation; same versus different manual source branches; unique main, release, and tag groups for both push and manual events.
  • Expected-value derivation (for exact numerical assertions): N/A — no prediction/numerical assertions changed.
  • Before/after output, trace, benchmark, or golden diff: Before, both PR revisions could remain active. After, native Actions concurrency gives the same PR ref one shared cancellation group. Tests evaluate the checked-in expression; scheduler cancellation itself has not been exercised with a deliberate live duplicate run.

Modeling or data provenance

N/A — no modeling, performance data, or prediction changes.

Tracking

  • Closes: N/A.
  • Related PRs or issues: N/A; follow-up to the observed AISim AMD64 CI queue and concurrent superseded PR runs.

Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: ai-dynamo/aisimulate/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c41173ff-fcd1-487b-bed7-6c6fefb2e3dd

📥 Commits

Reviewing files that changed from the base of the PR and between 56aeb08 and ddb63d5.

📒 Files selected for processing (1)
  • docs/ci.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ai-dynamo/dynamo (manual)
  • ai-dynamo/aiconfigurator (manual)

Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
Check commands, defaults, supported runtimes, public names, and claims against executable behavior.

⚙️ CodeRabbit configuration file

Files:

  • docs/ci.md
Read REVIEW.md before commenting.

⚙️ CodeRabbit configuration file

Files:

  • docs/ci.md
Before making any change under: `python/aisimulate/src/aiconfigurator/generator/**` MUST read: `python/aisimulate/.claude/rules/generator-development.md` Before making any change under `python/aisimulate/collector/**` MUST read: `python/ais...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • docs/ci.md
🔀 Multi-repo context ai-dynamo/dynamo, ai-dynamo/aiconfigurator

Linked repositories findings

ai-dynamo/dynamo

  • Full CI approval creates pull-request/N branches, which matches the PR’s trusted-copy concurrency grouping. [::ai-dynamo/dynamo::]
  • Existing pre-merge.yml already uses ref/PR-scoped concurrency with cancellation, providing a repository precedent. [::ai-dynamo/dynamo::]

ai-dynamo/aiconfigurator

  • pull-request/* is also the established privileged CI boundary in build-platform-wheels-copied-pr.yml. [::ai-dynamo/aiconfigurator::]
  • Existing workflows use both ref-scoped cancellation and unique run_id groups for manual runs, consistent with the PR’s documented behavior. [::ai-dynamo/aiconfigurator::]
  • No shared API, schema, runtime binding, or dependency consumer was identified.

📝 Summary

Risk level: Medium. Human attention should focus on concurrency-group boundaries, trusted-copy SHA verification, and exact-head Full CI validation.

Changed behavior

  • Full CI cancels older queued or running runs in the same concurrency group.
  • Trusted pull-request/N runs share a group across push and manual events.
  • Manual source-branch runs share a group only with the same source branch.
  • main, release/*, and tag runs use unique github.run_id groups.
  • Existing runs and older branches are not covered retroactively.
  • Documentation defines manual dispatch and trusted-copy SHA verification.

Public contracts

  • Full CI can cancel active validation runs.
  • Manual trusted-copy validation requires SHA equality with the originating PR head.
  • The replacement run must pass checks for its exact SHA.
  • The updated workflow must exist on the branch before the policy applies.

Technical quality

  • Contract tests cover group evaluation, cancellation, PR isolation, source-branch isolation, and lifecycle/tag behavior.
  • Supplied validation reports 316 tests and actionlint, Ruff, copyright, legal-file, documentation-link, and diff checks.
  • Standalone Fast CI and exact-head Full CI reportedly passed, with 54 successful jobs.
  • Bot reviews reported no actionable findings. This is not human approval.
  • Current review-finding counts are unavailable.

Merge readiness

  • The supplied evidence supports technical validation completion.
  • Human review remains required for merge readiness.
  • The available inspection evidence does not independently reproduce the reported test results.

Walkthrough

Full CI now uses workflow-level concurrency. Trusted pull-request copies and eligible manual branch runs share groups, while main, release, and other lifecycle runs use unique run IDs. Tests and CI documentation cover these rules.

Changes

Full CI concurrency control

Layer / File(s) Summary
Concurrency contract and validation
.github/workflows/ci.yml, tests/test_ci_workflow_contracts.py
Full CI cancels active runs in the same group. Tests verify shared groups for matching pull-request copies and source branches, and distinct groups for different branches, main, release, and tag runs.
Rerun and replacement guidance
docs/ci.md
The guide documents canceled-run replacement, trusted PR-copy reruns, commit-SHA verification, and concurrency boundaries.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Modeling And Data Evidence ⚠️ Warning The PR changes selection logic in .github/workflows/ci.yml by assigning concurrency groups and enabling cancellation. The base workflow had no workflow-level concurrency. The added tests only re-eva… Add reproducible evidence for the concurrency policy. Use a machine-readable fixture with event, ref, run ID, expected group, and expected cancellation/isolation outcome for trusted PR copies, manual source branches, main, release branches,…
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cross-Layer Contract ✅ Passed PASS — The PR changes only Full CI workflow concurrency, its documentation, and contract tests. The existing expected_sha input, triggers, job outputs, reusable-workflow inputs, and Full CI required…
Compatibility Boundaries ✅ Passed The reviewed range changes only Full CI concurrency, CI documentation, and CI contract tests. It does not modify Python or Rust manifests, public bindings, schemas, versions, defaults, or artifact-bui…
Review Evidence ✅ Passed The PR description names the pytest command and its 316-pass result, plus actionlint, Ruff, copyright, legal-file, documentation-link, and diff checks. It lists boundary cases for same/different PRs, …
Title check ✅ Passed The title precisely states the behavioral change: superseded Full CI runs are canceled per pull request. It is specific and avoids vague wording.
Description check ✅ Passed The description includes all required sections and explains the problem, behavior change, review risks, affected files, compatibility considerations, evidence, boundary cases, and tracking status.
Full details: Modeling And Data Evidence

Explanation

The PR changes selection logic in .github/workflows/ci.yml by assigning concurrency groups and enabling cancellation. The base workflow had no workflow-level concurrency. The added tests only re-evaluate the configured expression and assert equality/inequality for hard-coded cases; they provide no independent oracle, before/after comparison, explained golden diff, or machine-readable case summary with anomaly checks. The PR description also marks before/after evidence as N/A. Reported CI success is binary validation and does not supply the required evidence for this changed selection logic.

Resolution

Add reproducible evidence for the concurrency policy. Use a machine-readable fixture with event, ref, run ID, expected group, and expected cancellation/isolation outcome for trusted PR copies, manual source branches, main, release branches, and tags. Validate the fixture with an independent policy evaluator, include anomaly checks for PR isolation and lifecycle uniqueness, and record the pre-change versus post-change results with provenance (workflow revision and case source).

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

Comment thread .github/workflows/ci.yml Outdated
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
@jasonqinzhou
jasonqinzhou marked this pull request as ready for review September 19, 2026 00:11
@jasonqinzhou
jasonqinzhou requested review from a team as code owners September 19, 2026 00:11
…urrency

Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
@jasonqinzhou

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

Please review current head 56aeb08a48d4c0276c3724acffb8fa8382f3a076 after synchronizing with main. The net change remains the same three files. Local CI contract tests (316), actionlint, Ruff, and source/documentation checks pass; the fresh Codex review found no remaining actionable issue. Standalone Fast CI has passed on this commit.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

@jasonqinzhou I will review the complete pull request at the specified head revision.

✅ Action performed

Full review finished.

@jasonqinzhou

Copy link
Copy Markdown
Contributor Author

/ok to test 56aeb08

Comment thread docs/ci.md Outdated
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
@jasonqinzhou

Copy link
Copy Markdown
Contributor Author

/ok to test ddb63d5

@jasonqinzhou
jasonqinzhou enabled auto-merge (squash) September 19, 2026 01:45

@jasonqinzhou jasonqinzhou left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh Codex review of 72c3e70 after integrating main 7ae483e: no remaining actionable finding.

Reviewed the full three-file feature diff and surrounding workflow dispatch, trusted-copy SHA verification, scope selection, Fast CI prerequisite, and release staging paths. The feature diff is byte-identical to the prior revision. Cancellation identity uses only the workflow ref; trusted copies group across push/manual events, source branches are isolated by ref, and lifecycle/tag runs retain unique run IDs. Both prior inline findings are fixed and resolved.

Validation on this head: 316 CI/workflow/release tests passed with Python 3.12, working Node, and CI-pinned pip-licenses; Ruff lint/format, copyright headers, packaged legal files, documentation links, and diff hygiene passed.

The CodeRabbit modeling-data warning does not identify a product-model change: this PR changes GitHub scheduling policy only. The tests provide explicit cancellation/isolation expectations for eight boundary cases, while the description states that deliberate live duplicate-run cancellation is not exercised. Native scheduler semantics match GitHub's documented concurrency contract. No numerical oracle or modeling fixture is applicable.

Current-head CodeRabbit and required CI remain to be refreshed. Human approval is separate and will be rechecked before merge.

@jasonqinzhou

Copy link
Copy Markdown
Contributor Author

@coderabbitai review current head 72c3e70. Main is now integrated; the complete three-file feature diff is unchanged. Please refresh the current-head review before trusted Full CI admission.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== checked out revision ==='
git rev-parse HEAD
printf '%s\n' '=== base candidates ==='
git branch --all --no-color | sed -n '1,80p'
printf '%s\n' '=== merge-base diff stat (main...HEAD) ==='
git diff --stat main...HEAD
printf '%s\n' '=== changed paths (main...HEAD) ==='
git diff --name-status main...HEAD

printf '%s\n' '=== workflow outline ==='
ast-grep outline .github/workflows/ci.yml --items all || true
printf '%s\n' '=== workflow relevant content ==='
cat -n .github/workflows/ci.yml | sed -n '1,260p'

printf '%s\n' '=== CI contract test outline ==='
ast-grep outline tests/test_ci_workflow_contracts.py --items all --type function || true
printf '%s\n' '=== CI contract test relevant content ==='
cat -n tests/test_ci_workflow_contracts.py | sed -n '1,360p'

printf '%s\n' '=== CI documentation changed-region content ==='
cat -n docs/ci.md | sed -n '1,360p'

printf '%s\n' '=== exact patch ==='
git diff --no-ext-diff --unified=80 main...HEAD -- .github/workflows/ci.yml docs/ci.md tests/test_ci_workflow_contracts.py

printf '%s\n' '=== related workflow expressions and workflow dispatch usage ==='
rg -n -C 3 --glob '*.yml' --glob '*.yaml' \
  'concurrency:|workflow_dispatch:|pull-request/' .github/workflows || true

Length of output: 1889


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== checked out revision ==='
git rev-parse HEAD
printf '%s\n' '=== merge-base diff stat (main...HEAD) ==='
git diff --stat main...HEAD
printf '%s\n' '=== changed paths (main...HEAD) ==='
git diff --name-status main...HEAD

printf '%s\n' '=== workflow ==='
cat -n .github/workflows/ci.yml | sed -n '1,280p'
printf '%s\n' '=== CI contract tests ==='
cat -n tests/test_ci_workflow_contracts.py | sed -n '1,380p'
printf '%s\n' '=== CI documentation ==='
cat -n docs/ci.md | sed -n '1,380p'

printf '%s\n' '=== exact patch ==='
git diff --no-ext-diff --unified=80 main...HEAD -- .github/workflows/ci.yml docs/ci.md tests/test_ci_workflow_contracts.py

printf '%s\n' '=== related workflow expressions ==='
rg -n -C 3 --glob '*.yml' --glob '*.yaml' \
  'concurrency:|workflow_dispatch:|pull-request/' .github/workflows || true

Length of output: 1358


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== HEAD commit metadata and parents ==='
git show --no-patch --format='commit=%H%nparents=%P%nsubject=%s' HEAD
printf '%s\n' '=== HEAD changed paths ==='
git show --no-ext-diff --format='' --name-status HEAD
printf '%s\n' '=== HEAD patch for review files ==='
git show --no-ext-diff --format='' --unified=100 HEAD -- \
  .github/workflows/ci.yml docs/ci.md tests/test_ci_workflow_contracts.py

printf '%s\n' '=== current workflow ==='
cat -n .github/workflows/ci.yml | sed -n '1,300p'
printf '%s\n' '=== current CI workflow contract tests ==='
cat -n tests/test_ci_workflow_contracts.py | sed -n '1,420p'
printf '%s\n' '=== current CI documentation ==='
cat -n docs/ci.md | sed -n '1,420p'

printf '%s\n' '=== all Full CI workflow references ==='
rg -n -C 4 --glob '*.yml' --glob '*.yaml' \
  'workflow_dispatch:|concurrency:|github\.event_name|github\.ref_name|pull-request/' \
  .github/workflows || true

printf '%s\n' '=== test helpers that evaluate workflow expressions ==='
rg -n -C 5 --type py \
  'concurrency|workflow.*expression|github\.event_name|github\.ref_name|pull-request/' \
  tests || true

Length of output: 167401


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== feature commit availability ==='
git cat-file -e 56aeb08a48d4c0276c3724acffb8fa8382f3a076^{commit} && echo available || true
printf '%s\n' '=== changed paths since reviewed feature commit ==='
git diff --name-status 56aeb08a48d4c0276c3724acffb8fa8382f3a076..HEAD || true

printf '%s\n' '=== current Full CI concurrency declaration ==='
sed -n '1,45p' .github/workflows/ci.yml | cat -n
printf '%s\n' '=== current Full CI SHA-validation context ==='
sed -n '45,130p' .github/workflows/ci.yml | cat -n

printf '%s\n' '=== current concurrency contract helper and cases ==='
sed -n '1100,1160p' tests/test_ci_workflow_contracts.py | cat -n

printf '%s\n' '=== current concurrency documentation ==='
rg -n -C 6 -i 'cancel|concurren|manual.*Full CI|trusted.*copy|expected_sha' docs/ci.md

printf '%s\n' '=== exact feature-file diff from reviewed feature commit to current head ==='
git diff --no-ext-diff --unified=20 56aeb08a48d4c0276c3724acffb8fa8382f3a076..HEAD -- \
  .github/workflows/ci.yml docs/ci.md tests/test_ci_workflow_contracts.py || true

printf '%s\n' '=== parse workflow and report concurrency object ==='
python - <<'PY'
from pathlib import Path
import yaml
workflow = yaml.safe_load(Path(".github/workflows/ci.yml").read_text())
print(workflow["concurrency"])
PY

Length of output: 31324


@jasonqinzhou No actionable findings in the refreshed review of current head 72c3e703ae01c3b69f4a625afcdc972d916cf4db.

Verified:

  • Full CI cancels only superseded trusted-copy and manual source-branch runs.
  • Trusted-copy push and manual runs use the same pull-request/N group.
  • Main, release, and tag runs use unique github.run_id groups.
  • Contract tests cover each concurrency partition.
  • Documentation describes cancellation, exact-SHA validation, and manual-dispatch limits.

The local clone lacks the earlier commit and a merge base with main, so it could not produce a base-to-head diff. The current workflow, test contract, and documentation are internally consistent.

You are interacting with an AI system.

@jasonqinzhou

Copy link
Copy Markdown
Contributor Author

/ok to test 72c3e70

@jasonqinzhou
jasonqinzhou enabled auto-merge (squash) September 19, 2026 03:07
@jasonqinzhou
jasonqinzhou merged commit c8ba25e into main Sep 19, 2026
68 checks passed
@jasonqinzhou
jasonqinzhou deleted the codex/full-ci-pr-concurrency branch September 19, 2026 03:19
@jasonqinzhou

Copy link
Copy Markdown
Contributor Author

Verified squash merge into main: c8ba25e8ff9d7c17dbb3d980e4c2311071e0c2bd (2026-09-19 03:19:09 UTC).

  • Reviewed/tested head: 72c3e703ae01c3b69f4a625afcdc972d916cf4db.
  • Exact-head trusted Fast CI and trusted Full CI passed. Full CI: 54 successful jobs and one expected release-staging skip for the PR branch. Required codeowners and PR Fast CI also passed.
  • Applicable human approval remains recorded; all review threads are resolved. Current-head Codex and CodeRabbit reviews found no actionable issue.
  • Fetched main and verified the merge commit is on it. Its parent is 7ae483e942b0a2593d55e4e30957f94640580441; its tree exactly equals the validated head's 8c80e62912b7a3c5ceb30c088c84b8e19f66d12c, with an empty head-to-merge diff.

PR description and validation notes now reflect the completed merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants