Skip to content

fix(ci): omit null conclusion from check update#7064

Closed
ericksoa wants to merge 1 commit into
mainfrom
fix/e2e-in-progress-check-patch
Closed

fix(ci): omit null conclusion from check update#7064
ericksoa wants to merge 1 commit into
mainfrom
fix/e2e-in-progress-check-patch

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Omit conclusion when moving the PR E2E coordination check back to in_progress. GitHub rejects an explicit null conclusion with HTTP 422, so the controller currently stops before it can authorize or launch E2E.

Changes

  • Send only status: "in_progress" and the updated output in the check-run PATCH.
  • Continue requiring GitHub's returned in-progress check to have conclusion: null.
  • Assert both retry-history and fork-approval recovery paths omit the request field.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This corrects an internal GitHub check-run request and does not change user-facing behavior.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent controller review found no blocker and confirmed both markCheckInProgress call paths remain covered.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run test/pr-e2e-gate-retry-history.test.ts test/pr-e2e-gate-fork-skip.test.ts test/pr-e2e-gate-lifecycle.test.ts test/pr-e2e-gate.test.ts test/pr-e2e-required.test.ts (108/108 passed); npm run build:cli; npm run typecheck:cli; Biome and git diff --check passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Targeted controller coverage is proportionate to this one-field request correction.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

The failure is reproducible in controller run 29548952081 and independently in controller run 29548924425.


Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved check status updates by preserving existing conclusions when a check is marked as in progress.
    • Updated end-to-end validation to confirm that in-progress check updates omit unnecessary conclusion data.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa added area: ci CI workflows, checks, release automation, or GitHub Actions bug v0.0.85 Release target labels Jul 17, 2026
@ericksoa ericksoa self-assigned this Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: faa47d7b-7cf9-447a-9793-2faf2f930424

📥 Commits

Reviewing files that changed from the base of the PR and between 94b5e46 and 6e8eca0.

📒 Files selected for processing (3)
  • test/pr-e2e-gate-fork-skip.test.ts
  • test/pr-e2e-gate-retry-history.test.ts
  • tools/e2e/pr-e2e-gate.mts

📝 Walkthrough

Walkthrough

The PR stops sending conclusion: null when marking GitHub check runs in progress and updates two E2E tests to verify that the field is omitted.

Changes

Check-run payload handling

Layer / File(s) Summary
Update and validate in-progress check runs
tools/e2e/pr-e2e-gate.mts, test/pr-e2e-gate-fork-skip.test.ts, test/pr-e2e-gate-retry-history.test.ts
markCheckInProgress now sends only the in-progress status and output fields; both E2E tests assert that conclusion is absent.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested labels: area: e2e

Suggested reviewers: apurvvkumaria

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main fix: omitting the null conclusion from the check update request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/e2e-in-progress-check-patch

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

@github-code-quality

github-code-quality Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the branch.

TypeScript / code-coverage/cli

The overall coverage in the branch remains at 80%, unchanged from the branch.

Show a code coverage summary of the most impacted files.
File 94b5e46 6e8eca0 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/inferen...er-lifecycle.ts 65% 71% +6%
src/lib/inferen...lama/process.ts 50% 100% +50%

Updated July 17, 2026 02:21 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, credential-sanitization, security-posture

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Closing as superseded by #7065, which has now merged the same production fix and includes the additional initial-authorization regression assertion. Thank you for the fast independent repair.

@cv

cv commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Duplicate closure audit: #7065 has merged and fully supersedes this PR. The production tools/e2e/pr-e2e-gate.mts and retry-history test/pr-e2e-gate-retry-history.test.ts blobs were identical; #7065 was the strict test superset with the additional authorized-dispatch absence assertion. #7064 contains no unique behavior or test that needs cherry-picking.

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

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions bug v0.0.85 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants