fix(polymarket-release): gate contained bootstrap recovery - #644
Conversation
📝 WalkthroughWalkthroughThe PR adds a contained Rust bootstrap recovery path. It validates candidate probes, baseline identity, systemd state, and uploader containment. Gate, policy, cutover, rollback, evidence, and control-plane tests now support recovery metadata. ChangesContained bootstrap recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant GateControl
participant ShadowGate
participant Cutover
participant Rollback
Operator->>GateControl: submit recover command
GateControl->>ShadowGate: start Gate with recovery metadata
ShadowGate->>ShadowGate: validate probe, baseline, and uploader state
ShadowGate->>Cutover: emit recovery evidence
Cutover->>Cutover: validate contained baseline and promote candidate
Cutover->>Rollback: persist recovery state for rollback
Rollback->>Cutover: restore inactive units without restarting collector
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 |
33600d2 to
5f186c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
deployment/aliyun/polymarket-raw-ops-cutover.sh (1)
1265-1294: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove the saved-unit precondition before the destructive restore steps.
The guard at Line 1266 rejects a rollback whose snapshot recorded an active collector or timer. It runs after
restore_legacyalready stopped units, reinstalled unit files, and reinstalled the active binary at Line 1150 through Line 1263. A refusal therefore leaves a partially mutated host.Evaluate the same
jq -echeck right aftercontained_recoveryis read at Line 1127. Keep the post-transition assertions at Line 1283 through Line 1291 where they are.♻️ Proposed placement
contained_recovery=$(jq -er \ '(.contained_recovery // false) | select(type == "boolean") | tostring' \ "$rollback_dir/state.json") \ || die 'rollback snapshot has no valid contained recovery state' + if [[ $contained_recovery == true ]]; then + jq -e --arg collector "$COLLECTOR_UNIT" --arg reference_timer "$REFERENCE_UPLOAD_TIMER" \ + --arg market_timer "$MARKET_UPLOAD_TIMER" ' + .units[$collector].active == false + and .units[$reference_timer].active == false + and .units[$market_timer].active == false + ' "$rollback_dir/state.json" >/dev/null \ + || die 'contained recovery rollback would restart a saved baseline unit' + fi🤖 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 1265 - 1294, Move the saved-unit activity precondition from the start of the contained_recovery restore block to immediately after contained_recovery is read, before restore_legacy or any other destructive restore operations. Preserve the existing jq check and failure message, and keep the post-transition assertions and state verification in the contained recovery block unchanged.deployment/aliyun/polymarket-shadow-gate-policy.jq (1)
64-95: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider binding the probe timestamp to the Gate window in the policy.
contained_bootstrap_recoveryvalidates the probe format and the baseline identity. It does not relate.candidate_probe.observed_atto.started_at. The 900-second freshness bound exists only in the shell admission checks, so the immutable receipt cannot be re-verified for staleness later.Add an ordering and lag predicate so the receipt proves freshness on its own.
♻️ Proposed addition to
recovery_matches_gatedef recovery_matches_gate: . as $gate | ($gate.recovery | contained_bootstrap_recovery( - $gate.candidate_sha256; $gate.deployment_source_revision)); + $gate.candidate_sha256; $gate.deployment_source_revision)) + and (($gate.recovery.candidate_probe.observed_at | utc_iso8601_unix) as $observed + | ($gate.started_at | utc_iso8601_unix) as $started + | $observed <= $started and ($started - $observed) <= 900);🤖 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 64 - 95, Update recovery_matches_gate and its contained_bootstrap_recovery validation to bind candidate_probe.observed_at to the gate's started_at, requiring the observation timestamp to be no earlier than the gate start and no more than 900 seconds later. Ensure the timestamp is parsed consistently with the existing utc_iso8601_unix handling so the immutable receipt independently proves freshness.deployment/aliyun/test-polymarket-raw-ops-control-plane.sh (1)
4048-4060: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd drift cases for the recovery-specific exclusivity rules.
The table covers candidate binding, source binding, baseline state, and runtime stability. Three predicates that gate the new branch have no counterexample: a null
.recovery, a non-null.legacy_runtime, and a.recovery.baseline.binary_sha256equal to the candidate. The coding guidelines require a targeted counterexample test for safety boundaries.If you adopt the probe-freshness predicate suggested for
deployment/aliyun/polymarket-shadow-gate-policy.jq, also deriveobserved_atat Line 4034 from.started_atinstead of the fixed literal.As per coding guidelines: "Safety boundaries require a targeted counterexample test, not only workspace compilation."
💚 Proposed additional drift rows
candidate_binding|.recovery.candidate_probe.candidate_sha256 = (if .candidate_sha256 == ("0" * 64) then ("1" * 64) else ("0" * 64) end) source_binding|.recovery.candidate_probe.source_revision = (if .deployment_source_revision == ("0" * 40) then ("1" * 40) else ("0" * 40) end) baseline_active|.recovery.baseline.active_state = "active" runtime_stability|.baseline_runtime_stability_required = true +missing_recovery|.recovery = null +baseline_is_candidate|.recovery.baseline.binary_sha256 = .candidate_sha256 +legacy_runtime_present|.legacy_runtime = {exec_start:"/opt/monday/bin/polymarket-raw-ops collect-reference --max-trade-polls-per-cycle 200"} EOF🤖 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 4048 - 4060, Add targeted drift rows to the recovery-policy loop for null .recovery, non-null .legacy_runtime, and .recovery.baseline.binary_sha256 matching the candidate value, ensuring each predicate’s safety boundary is exercised. If the policy also introduces probe freshness, update the observed_at derivation near the recovery fixture setup to use .started_at rather than a fixed literal.Source: Coding guidelines
🤖 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/polymarket-raw-ops-shadow-gate.sh`:
- Around line 1341-1343: Change both `.recovery` reads in
deployment/aliyun/polymarket-raw-ops-shadow-gate.sh:1341-1343 and
deployment/aliyun/polymarket-raw-ops-cutover.sh:1672-1673 from `jq -ce` to `jq
-c`, preserving the `// null` fallback so optional recovery values produce
literal `null` without failing. Keep the existing `[[ $recovery_json != null ]]`
branching and recovery validation behavior unchanged.
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Around line 434-437: Update the comment preceding the recovery admission call
in the test script by removing the sentence claiming it is intentionally red
until `recover` is implemented; retain the contract description about the
stopped bootstrap identity, fresh candidate probe, and contained
uploaders/timers.
---
Nitpick comments:
In `@deployment/aliyun/polymarket-raw-ops-cutover.sh`:
- Around line 1265-1294: Move the saved-unit activity precondition from the
start of the contained_recovery restore block to immediately after
contained_recovery is read, before restore_legacy or any other destructive
restore operations. Preserve the existing jq check and failure message, and keep
the post-transition assertions and state verification in the contained recovery
block unchanged.
In `@deployment/aliyun/polymarket-shadow-gate-policy.jq`:
- Around line 64-95: Update recovery_matches_gate and its
contained_bootstrap_recovery validation to bind candidate_probe.observed_at to
the gate's started_at, requiring the observation timestamp to be no earlier than
the gate start and no more than 900 seconds later. Ensure the timestamp is
parsed consistently with the existing utc_iso8601_unix handling so the immutable
receipt independently proves freshness.
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Around line 4048-4060: Add targeted drift rows to the recovery-policy loop for
null .recovery, non-null .legacy_runtime, and .recovery.baseline.binary_sha256
matching the candidate value, ensuring each predicate’s safety boundary is
exercised. If the policy also introduces probe freshness, update the observed_at
derivation near the recovery fixture setup to use .started_at rather than a
fixed literal.
🪄 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: 62edcfd0-90c1-4505-8b67-e6bbd283c82c
📒 Files selected for processing (5)
deployment/aliyun/polymarket-raw-ops-cutover.shdeployment/aliyun/polymarket-raw-ops-gate-control.shdeployment/aliyun/polymarket-raw-ops-shadow-gate.shdeployment/aliyun/polymarket-shadow-gate-policy.jqdeployment/aliyun/test-polymarket-raw-ops-control-plane.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33600d24a9
ℹ️ 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".
5f186c2 to
b032423
Compare
Change contract
Add one candidate-only
recoverGate for a verified, contained direct Rust bootstrap baseline after the known Gamma tagged-500 failure. Admission binds the stopped baseline, exact candidate probe, inactive uploader/timer units, existing 900-second Gate, real-market preflight, OSS readback, immutable receipt, cutover, and rollback.Issue relationship
Closes #637
Out of scope
Runtime rollout #632; deployment; collector or uploader restart; OSS cleanup; uploader behavior changes; Gamma failure-policy changes; research work; execution runtime.
Dependencies and merge order
None.
Focused validation
cargo test -p hft-collector --locked closed_discovery_500_retries_then_falls_back_without_the_crypto_tagcargo test -p hft-collector --locked untagged_gamma_fallback_requires_a_closed_final_500PATH=/opt/homebrew/opt/coreutils/libexec/gnubin:$PATH bash deployment/aliyun/test-polymarket-raw-ops-control-plane.shbash -nfor changed shell scripts;jq -n -f deployment/aliyun/polymarket-shadow-gate-policy.jq;git diff --check;.github/scripts/agent-worktree-preflight.sh checkRollout and rollback
Merge publishes control-plane code only. No runtime action occurs from this PR; #632 retains rollout authority. Revert this commit to remove the recovery control path; a governed runtime recovery retains its own transactional rollback receipt.
Scope exception
This is an inseparable 5-file, 988 non-generated-line fail-closed safety contract. Controller admission, Gate/policy receipt evidence, cutover/rollback behavior, and contract tests cannot be independently merged or safely rolled back. Named review approval:
standards_review(Codex Standards Review).Summary by CodeRabbit
New Features
Bug Fixes
Tests