Skip to content

fix(test): force sync before snapshot digest check (#731) - #732

Merged
proerror77 merged 9 commits into
mainfrom
codex/snapshot-test-fix-731
Aug 7, 2026
Merged

fix(test): force sync before snapshot digest check (#731)#732
proerror77 merged 9 commits into
mainfrom
codex/snapshot-test-fix-731

Conversation

@proerror77

@proerror77 proerror77 commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Change contract

Fix intermittent snapshot-test flake (#731): force a real sync after
snapshot_legacy copies the bootstrap binary, before the digest check, so the
check does not read a partially-flushed file under CI IO pressure.

Issue relationship

Closes #731

Focused validation

Rust Workspace passes; bounded preflight scan unchanged.

Summary by CodeRabbit

  • Bug Fixes
    • Improved bootstrap snapshot verification by ensuring rollback data is fully flushed before checksum validation.
    • Prevented intermittent verification failures caused by pending file system writes.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@proerror77, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5fa2fb3f-b47b-4530-afbe-0ab3d6c3f8a0

📥 Commits

Reviewing files that changed from the base of the PR and between 1c347dd and dd6b5f4.

📒 Files selected for processing (1)
  • deployment/aliyun/test-polymarket-raw-ops-control-plane.sh
📝 Walkthrough

Walkthrough

The bootstrap snapshot test now runs command sync after copying rollback data and before checksum verification. Sync failures do not abort the test.

Changes

Snapshot checksum validation

Layer / File(s) Summary
Flush rollback data before validation
deployment/aliyun/test-polymarket-raw-ops-control-plane.sh
The test synchronizes copied rollback data before validating its checksum manifest. Sync failures are ignored.

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

Possibly related PRs

  • proerror77/monday#520: Both changes modify legacy rollback verification in the same deployment test script.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the change, issue relationship, and validation but omits four required template sections. Add Out of scope, Dependencies and merge order, Rollout and rollback, and Scope exception sections, using None where applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the test fix: forcing synchronization before the snapshot digest check.
Linked Issues check ✅ Passed The change directly addresses issue #731 by syncing copied snapshot data before digest verification.
Out of Scope Changes check ✅ Passed The four added lines are limited to the snapshot-test fix described in issue #731.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/snapshot-test-fix-731

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Sonic Shih added 4 commits August 7, 2026 13:50
snapshot_legacy copies ACTIVE_BINARY into the rollback dir then immediately
sha256sum --check --strict's it. Under CI cache/IO pressure the copy may not be
flushed when read, causing an intermittent digest mismatch. Force a real sync
(bypassing the test's sync() no-op override) before the check.
@proerror77 proerror77 changed the title debug: diagnose snapshot test digest mismatch (#731) fix(test): force sync before snapshot digest check (#731) Aug 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Line 2671: Remove the silent fallback from the command sync step in the
rollback validation flow so sync failures propagate and stop execution. Ensure
sha256sum --check runs only after command sync succeeds, preserving the existing
checksum validation otherwise.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b84de5c-ddc0-4750-8051-95ce33659c24

📥 Commits

Reviewing files that changed from the base of the PR and between f9d8647 and 1c347dd.

📒 Files selected for processing (1)
  • deployment/aliyun/test-polymarket-raw-ops-control-plane.sh

Comment thread deployment/aliyun/test-polymarket-raw-ops-control-plane.sh Outdated
Sonic Shih added 2 commits August 7, 2026 14:08
Address CodeRabbit review: the sync must not silently fall back (|| true),
else the digest check can still run on an unflushed file. die on sync failure.
@proerror77
proerror77 merged commit a74ee6f into main Aug 7, 2026
45 checks passed
@proerror77
proerror77 deleted the codex/snapshot-test-fix-731 branch August 7, 2026 07:00
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.

fix(test): polymorpharket raw-ops control-plane snapshot test fails on CI when Rust Workspace job runs

1 participant