deploy: recover Polymarket from an unregistered degraded Rust baseline - #629
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe control plane adds ChangesRust bootstrap baseline support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
deployment/aliyun/test-polymarket-raw-ops-control-plane.sh (3)
4941-4944: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePin the occurrence count alongside
tail -1.The bootstrap branch at Lines 1162-1166 of
deployment/aliyun/polymarket-raw-ops-cutover.shadded a secondactive_targetrelease-root guard and a secondrm -f "$ACTIVE_BINARY".tail -1now selects thelegacy_pythonblock at Lines 1189-1194, which is the intended site. If a future branch is appended after that block,tail -1moves to the new site and the ordering assertion still passes against the wrong code. Assert the expected count as well, as Line 4579 does for the invocation-ID check.♻️ Sketch
+[[ $(grep -c 'active_target == "\$RELEASE_ROOT"/\*/polymarket-raw-ops' "$CUTOVER") -eq 2 ]] active_target_guard_line=$(grep -n \ 'active_target == "\$RELEASE_ROOT"/\*/polymarket-raw-ops' "$CUTOVER" | tail -1 | cut -d: -f1) +[[ $(grep -c '^ rm -f "\$ACTIVE_BINARY"$' "$CUTOVER") -eq 2 ]] active_rm_line=$(grep -n '^ rm -f "\$ACTIVE_BINARY"$' "$CUTOVER" | tail -1 | cut -d: -f1)🤖 Prompt for 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. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 4941 - 4944, Update the occurrence lookups for active_target_guard_line and active_rm_line to assert the expected match count before selecting the final occurrence, following the existing invocation-ID check pattern at Line 4579. Preserve selecting the intended legacy_python block while making the ordering assertion fail if additional matching branches are introduced later.
1387-1400: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd drift cases for the bootstrap-specific guards.
The drift loop covers
active,proc,digest, andruntime. The bootstrap branch indeployment/aliyun/polymarket-raw-ops-shadow-gate.shLines 452-457 adds three guards that no case exercises:
baseline_release_path != RUST_ACTIVE_BINARYRUST_ACTIVE_BINARYis a symlinkbaseline_release_pathis not executableThese guards are what separate a bootstrap baseline from a release baseline. Add cases for them.
💚 Proposed additional cases
- for drift in active proc digest runtime; do + for drift in active proc digest runtime path symlink nonexec; do mock_active=$RUST_ACTIVE_BINARY mock_proc=$RUST_ACTIVE_BINARY mock_digest=true mock_runtime=true + baseline_release_path=$RUST_ACTIVE_BINARY + rm -f "$tmp_dir/bootstrap-link"; chmod +x "$RUST_ACTIVE_BINARY" case "$drift" in active) mock_active=/tmp/wrong ;; proc) mock_proc=/tmp/wrong ;; digest) mock_digest=false ;; runtime) mock_runtime=false ;; + path) baseline_release_path=/tmp/wrong ;; + symlink) + ln -s "$RUST_ACTIVE_BINARY" "$tmp_dir/bootstrap-link" + RUST_ACTIVE_BINARY=$tmp_dir/bootstrap-link + baseline_release_path=$RUST_ACTIVE_BINARY ;; + nonexec) chmod -x "$RUST_ACTIVE_BINARY" ;; esac🤖 Prompt for 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. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 1387 - 1400, Extend the drift loop in the test around verify_baseline_identity with cases that independently violate the bootstrap-specific guards: set baseline_release_path different from RUST_ACTIVE_BINARY, make RUST_ACTIVE_BINARY a symlink, and make baseline_release_path non-executable. Ensure each case causes verify_baseline_identity to reject the baseline while preserving the existing active, proc, digest, and runtime cases.
2331-2339: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe non-executable case tests the recorded mode, not the file permission.
make_bootstrap_lineage_evidencealways runschmod +xon the snapshot binary at Line 2270. Passing0644therefore only changes.active_direct.modeinstate.json. The rejection comes from the mode check at Line 1432 ofdeployment/aliyun/polymarket-raw-ops-cutover.sh, not from the-xcheck at Line 1437. The failure message at Line 2338 states "non-executable bootstrap rollback image", which describes the untested guard.Add a separate case that clears the executable bit on
$evidence/rollback/bin/polymarket-raw-opswhile keepingmodeat0755. Then rename the current case to reflect that it covers mode metadata.🤖 Prompt for 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. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 2331 - 2339, The non-executable bootstrap test currently validates only recorded mode metadata because make_bootstrap_lineage_evidence restores executable permissions. Rename the existing bootstrap-nonexec case and its failure message to identify the mode-metadata rejection, then add a separate case that keeps mode 0755 while removing the executable bit from evidence/rollback/bin/polymarket-raw-ops and verifies manual rollback rejects it.Source: Linters/SAST tools
deployment/aliyun/polymarket-raw-ops-cutover.sh (2)
1240-1286: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the repeated rollback runtime check.
The same three-way mode dispatch appears at Lines 1240-1251, Lines 1265-1274, and Lines 1277-1286. Only the failure message differs. A single helper that takes the message reduces the risk that a future mode is added to one copy and missed in the others.
♻️ Sketch
verify_rollback_runtime() { local failure=$1 if [[ $rollback_mode == legacy_python ]]; then verify_legacy_runtime "$rollback_pid" 0 "$rollback_invocation_id" || die "$failure" elif [[ $rollback_mode == rust_release ]]; then verify_rust_runtime "$active_target" "$started_epoch" "$rollback_pid" \ "$rollback_invocation_id" 0 "$rollback_health_policy" || die "$failure" else verify_bootstrap_rust_runtime "$bootstrap_path" "$bootstrap_sha" \ "$rollback_pid" "$rollback_invocation_id" 0 || die "$failure" fi }Note that the contract test at Line 4704 of
deployment/aliyun/test-polymarket-raw-ops-control-plane.shgreps for these literal lines, so the test would need the same update.🤖 Prompt for 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. In `@deployment/aliyun/polymarket-raw-ops-cutover.sh` around lines 1240 - 1286, Extract the repeated three-way rollback runtime dispatch into a single verify_rollback_runtime helper that accepts the failure message and performs the existing legacy_python, rust_release, and bootstrap checks before calling die. Replace all three inline dispatch blocks with helper calls using their current messages, and update the contract test expectations that grep for the removed literal lines.
1223-1239: 🩺 Stability & Availability | 🔵 TrivialBootstrap rollback clears health but never requires it to return.
Line 1099 clears the health file for every mode except
rust_release, so arust_bootstraprollback also clears it. The bootstrap branch at Lines 1229-1232 breaks on runtime identity alone, and Line 1237 exempts bootstrap from the health-advance assertion. A bootstrap rollback can therefore report success while the restored collector writes no health file. The degraded baseline contract makes this intentional. Add a post-rollback alert on health-file staleness for the bootstrap path so the operator detects a silent collector.🤖 Prompt for 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. In `@deployment/aliyun/polymarket-raw-ops-cutover.sh` around lines 1223 - 1239, Add a post-rollback staleness alert for the rust_bootstrap path after the rollback verification loop, checking whether the health file is missing or its SHA remains unchanged from previous_health_sha. Keep bootstrap success based on verify_bootstrap_rust_runtime and preserve the degraded-baseline behavior; report the stale health condition as an operator-visible alert rather than failing the rollback.deployment/aliyun/polymarket-shadow-gate-policy.jq (1)
219-241: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider pinning
baseline_degradedin the non-bootstrap branches.The
rust_bootstrapbranch requires.baseline_degraded == true. Thelegacy_pythonbranch at Lines 141-195 and therust_releasebranch at Lines 198-218 do not constrain.baseline_degraded. Evidence that setsbaseline_degradedtotrueunderrust_releasestill passes the policy. Adding an explicitfalseassertion to both branches makes the field authoritative for every baseline mode.♻️ Proposed tightening for the `rust_release` branch
.baseline_mode == "rust_release" + and .baseline_degraded == false and .baseline_health_start_required == false🤖 Prompt for 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. In `@deployment/aliyun/polymarket-shadow-gate-policy.jq` around lines 219 - 241, Pin the baseline_degraded field in both non-bootstrap policy branches: add an explicit .baseline_degraded == false condition to the legacy_python branch and the rust_release branch. Leave the rust_bootstrap branch’s existing true assertion unchanged, ensuring every baseline mode authoritatively constrains this field.
🤖 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.
Nitpick comments:
In `@deployment/aliyun/polymarket-raw-ops-cutover.sh`:
- Around line 1240-1286: Extract the repeated three-way rollback runtime
dispatch into a single verify_rollback_runtime helper that accepts the failure
message and performs the existing legacy_python, rust_release, and bootstrap
checks before calling die. Replace all three inline dispatch blocks with helper
calls using their current messages, and update the contract test expectations
that grep for the removed literal lines.
- Around line 1223-1239: Add a post-rollback staleness alert for the
rust_bootstrap path after the rollback verification loop, checking whether the
health file is missing or its SHA remains unchanged from previous_health_sha.
Keep bootstrap success based on verify_bootstrap_rust_runtime and preserve the
degraded-baseline behavior; report the stale health condition as an
operator-visible alert rather than failing the rollback.
In `@deployment/aliyun/polymarket-shadow-gate-policy.jq`:
- Around line 219-241: Pin the baseline_degraded field in both non-bootstrap
policy branches: add an explicit .baseline_degraded == false condition to the
legacy_python branch and the rust_release branch. Leave the rust_bootstrap
branch’s existing true assertion unchanged, ensuring every baseline mode
authoritatively constrains this field.
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Around line 4941-4944: Update the occurrence lookups for
active_target_guard_line and active_rm_line to assert the expected match count
before selecting the final occurrence, following the existing invocation-ID
check pattern at Line 4579. Preserve selecting the intended legacy_python block
while making the ordering assertion fail if additional matching branches are
introduced later.
- Around line 1387-1400: Extend the drift loop in the test around
verify_baseline_identity with cases that independently violate the
bootstrap-specific guards: set baseline_release_path different from
RUST_ACTIVE_BINARY, make RUST_ACTIVE_BINARY a symlink, and make
baseline_release_path non-executable. Ensure each case causes
verify_baseline_identity to reject the baseline while preserving the existing
active, proc, digest, and runtime cases.
- Around line 2331-2339: The non-executable bootstrap test currently validates
only recorded mode metadata because make_bootstrap_lineage_evidence restores
executable permissions. Rename the existing bootstrap-nonexec case and its
failure message to identify the mode-metadata rejection, then add a separate
case that keeps mode 0755 while removing the executable bit from
evidence/rollback/bin/polymarket-raw-ops and verifies manual rollback rejects
it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b20ffbac-33a4-4f9c-b6cb-d594c25d0f87
📒 Files selected for processing (4)
deployment/aliyun/polymarket-raw-ops-cutover.shdeployment/aliyun/polymarket-raw-ops-shadow-gate.shdeployment/aliyun/polymarket-shadow-gate-policy.jqdeployment/aliyun/test-polymarket-raw-ops-control-plane.sh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37fcc5186a
ℹ️ 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.
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`:
- Around line 5172-5183: Update the control-plane identity assertion near the
legacy_test_reference_allowlist check to match the workflow’s actual
exec_start|cmdline allowlist regex rather than the hardcoded line-17 LEGACY_EXEC
string. Use the real LEGACY_EXEC definition at line 1411 and preserve validation
against legacy_test_reference_allowlist.
🪄 Autofix (Beta)
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: 75972bdb-e80d-45bb-b6fa-68bcb5c75da2
📒 Files selected for processing (4)
.github/workflows/ci.ymldeployment/aliyun/polymarket-raw-ops-cutover.shdeployment/aliyun/polymarket-shadow-gate-policy.jqdeployment/aliyun/test-polymarket-raw-ops-control-plane.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- deployment/aliyun/polymarket-shadow-gate-policy.jq
- deployment/aliyun/polymarket-raw-ops-cutover.sh
6392007 to
5e3ccb2
Compare
1 participant
Change contract
Permit exactly one bounded recovery path: a verified Rust candidate may replace an unhealthy, unregistered direct Rust binary. Normal Rust-to-Rust releases still require the existing immutable, healthy baseline.
Acceptance evidence
rust_bootstrapwith degraded baseline evidence, stable runtime identity, candidate health, full Gate evidence, and independent OSS data/manifest/_SUCCESS readback.Out of scope
Normal Rust health-policy redesign, generic Gate bypasses, manual active-binary replacement, collector/uploader restart, OSS cleanup, and research/snapshot work.
Dependency / merge order
Base:
main. Depends on already-merged #620 and #626.Issue relationship
Refs #627
Focused validation
bash deployment/aliyun/test-polymarket-raw-ops-control-plane.shbash -nandshellcheckon the changed shell scriptscargo test -p hft-collector --lib systemd_reference_resource_envelope_is_pinned --lockedgit diff --check origin/main...HEADRollout / rollback impact
No runtime change in this PR. After merge, a separate runtime operation must publish a candidate and run
stage → Gate → cutover; failed cutover restores the recorded binary and service state. No collector or uploader restart occurs from this PR itself.Summary by CodeRabbit
New Features
Bug Fixes
Tests