Skip to content

feat(common): add QA-facing upgrade report#2157

Merged
mergify[bot] merged 2 commits intomainfrom
elias/upgrade-report
Mar 26, 2026
Merged

feat(common): add QA-facing upgrade report#2157
mergify[bot] merged 2 commits intomainfrom
elias/upgrade-report

Conversation

@Eikix
Copy link
Copy Markdown
Contributor

@Eikix Eikix commented Mar 23, 2026

Summary

  • extract the upgrade version check into a shared helper without changing the existing CI entrypoint
  • add a small ci/list-upgrades.ts command that prints which upgradeable contracts changed, their reinitializer signature, and whether upgrade args/defaults need attention
  • keep a tiny maintained hints file for rollout notes that cannot be inferred mechanically

Why

QA and release still have to reconstruct the existing CI plumbing by hand to answer a simple question: which contracts need upgrades for a given release delta, and do any of them need extra attention.

This keeps the existing checker as the source of truth and adds one human-facing wrapper on top.

Validation

  • A/B checked old vs new ci/check-upgrade-versions.ts locally on identical prepared inputs for both host-contracts and gateway-contracts; outputs matched exactly
  • ran:
    • bun ci/list-upgrades.ts --from v0.11.1 --to elias/prepare-upgrade-fhevm-executor --package host-contracts
    • bun ci/list-upgrades.ts --from v0.11.1 --to elias/prepare-upgrade-fhevm-executor --package gateway-contracts

@Eikix Eikix requested a review from a team as a code owner March 23, 2026 15:43
@cla-bot cla-bot bot added the cla-signed label Mar 23, 2026
@Eikix Eikix changed the title feat(ci): add QA-facing upgrade report feat(common): add QA-facing upgrade report Mar 23, 2026
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 24, 2026

@claude /pr-review

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026 •
edited
Loading

Claude finished @Eikix's request. — View run

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

[AUTOMATED]

Code Review

Summary

This PR cleanly extracts the core upgrade-version-check logic from ci/check-upgrade-versions.ts into a reusable library (ci/upgrade-version-check-lib.ts), and adds a new QA-facing ci/list-upgrades.ts CLI tool on top of it. A small upgrade-report-hints.ts data file is added for rollout notes that can't be inferred mechanically.

Files Reviewed

File Assessment
.github/workflows/contracts-upgrade-version-check.yml ✅ Correctly adds the new lib to the path filter for both packages
ci/check-upgrade-versions.ts ✅ Clean refactor — delegates to the shared library while preserving all CI behavior
ci/upgrade-version-check-lib.ts ✅ Well-structured extraction; logic faithfully preserves all original checks
ci/list-upgrades.ts ✅ Clean CLI tool with proper arg parsing, worktree management, and cleanup
ci/upgrade-report-hints.ts ✅ Simple, typed data file

Review Results

Checked for: bugs, security issues, error handling, and architectural consistency.

No blocking issues found. The refactoring is behavior-preserving and well-structured. Specific observations:

  1. Logic preservation — I traced through the old inline logic vs. the new collectUpgradeVersionResults library and confirmed that all error conditions (missing baseline, missing target, parse failures, compilation failures, version/bytecode mismatch checks, reinitializer function naming) are correctly preserved.

  2. The reinitializeV{N-1} convention — The -1 offset in the expected function name (REINITIALIZER_VERSION=N → reinitializeV{N-1}) is preserved from the original code and is the project's existing naming convention.

  3. Worktree cleanup — The finally block in list-upgrades.ts correctly removes the temp directory first (rmSync), then prunes stale worktree bookkeeping (git worktree prune). The empty catch {} around prune is acceptable since it's best-effort cleanup.

  4. Typed interfaces — Good use of ContractCheckResult, ReinitializerInfo, and ContractVersions interfaces to make the library's contract explicit and self-documenting.

Nit: The getReinitializer regex expects the visibility modifier (public|external|internal|private) immediately after the closing parenthesis. If a Solidity function has other modifiers (e.g., reinitializer(N)) between the params and visibility, the regex won't match. This mirrors the old code's approach so it's not a regression, but worth noting for future hardening.

No issues found. LGTM ✅

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

The review has been posted. Here's a summary of the review:

Review Complete ✅

PR #2157: feat(common): add QA-facing upgrade report

I ran 4 specialized review agents in parallel (2× Bug Hunter on opus, Error Handling Auditor, and Guideline Compliance on sonnet) to thoroughly analyze the 5 changed files (379 additions, 114 deletions).

Findings

  • No blocking issues found
  • The refactoring cleanly extracts logic into upgrade-version-check-lib.ts while preserving all original CI behavior
  • One potential concern about reinitializer function naming (reinitializeV{N-1}) was validated as the existing project convention preserved from the original code
  • Error handling patterns are consistent with the pre-existing codebase
  • One minor nit noted about the getReinitializer regex's sensitivity to Solidity modifier ordering (non-blocking, not a regression)

Review comment posted: #2157 (comment)

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 25, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 25, 2026 •
edited
Loading

Merge Queue Status

This pull request spent 14 hours 34 minutes 23 seconds in the queue, including 12 hours 1 minute 30 seconds running CI.

Reason

The merge conditions cannot be satisfied due to failing checks

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

mergify bot added a commit that referenced this pull request Mar 25, 2026
@mergify
Merge of #2157
638f677
This was referenced Mar 25, 2026
Closed
Merged
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 26, 2026

@Mergifyio dequeue

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 26, 2026

@Mergifyio refresh

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 26, 2026

@Mergifyio rebase

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026

dequeue

☑️ The pull request is not queued

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026

refresh

✅ Pull request refreshed

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026

rebase

✅ Branch has been successfully rebased

@Eikix Eikix force-pushed the elias/upgrade-report branch from ddd6ae0 to 1f72238 Compare March 26, 2026 08:28
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 26, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026 •
edited
Loading

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Required conditions to enter a queue
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule main]:
      • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • base = main
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=do-not-merge
      • any of [🛡 GitHub branch protection]:
        • check-success = common-pull-request/lint (bpr)
        • check-neutral = common-pull-request/lint (bpr)
        • check-skipped = common-pull-request/lint (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = coprocessor-cargo-test/cargo-tests (bpr)
        • check-neutral = coprocessor-cargo-test/cargo-tests (bpr)
        • check-success = coprocessor-cargo-test/cargo-tests (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)
        • check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)
        • check-success = coprocessor-dependency-analysis/dependencies-check (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)
        • check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)
        • check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = kms-connector-tests/test-connector (bpr)
        • check-neutral = kms-connector-tests/test-connector (bpr)
        • check-success = kms-connector-tests/test-connector (bpr)

@mergify mergify bot removed the dequeued label Mar 26, 2026
@mergify
Copy link
Copy Markdown

mergify bot commented Mar 26, 2026 •
edited
Loading

Merge Queue Status

This pull request spent 2 hours 40 minutes 23 seconds in the queue, including 1 hour 39 minutes 27 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • check-success = run-e2e-tests / fhevm-e2e-test
  • any of [🛡 GitHub branch protection]:
    • check-success = common-pull-request/lint (bpr)
    • check-neutral = common-pull-request/lint (bpr)
    • check-skipped = common-pull-request/lint (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-success = coprocessor-cargo-test/cargo-tests (bpr)
    • check-neutral = coprocessor-cargo-test/cargo-tests (bpr)
    • check-skipped = coprocessor-cargo-test/cargo-tests (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)
    • check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)
    • check-success = coprocessor-dependency-analysis/dependencies-check (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)
    • check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)
    • check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = kms-connector-tests/test-connector (bpr)
    • check-neutral = kms-connector-tests/test-connector (bpr)
    • check-success = kms-connector-tests/test-connector (bpr)

@mergify mergify bot removed the merge-queued label Mar 26, 2026
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2157
cb6c3f0
@mergify mergify bot mentioned this pull request Mar 26, 2026
Closed
51 tasks
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2157
21a8d16
@mergify mergify bot mentioned this pull request Mar 26, 2026
Merged
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2157
afd7ee3
@mergify mergify bot merged commit 7e528aa into main Mar 26, 2026
64 of 65 checks passed
@mergify mergify bot deleted the elias/upgrade-report branch March 26, 2026 11:10
@mergify mergify bot removed the queued label Mar 26, 2026
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2157
4b627f2
mergify bot added a commit that referenced this pull request Mar 26, 2026
@mergify
Merge of #2157
1a45090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PanGan21 PanGan21 PanGan21 approved these changes

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Eikix @PanGan21