Skip to content

fix: complete standby recovery despite diagnostic status churn - #4704

Merged
devantler merged 2 commits into
mainfrom
codex/pause-rebind-4703-2
Oct 10, 2026
Merged

devantler merged 2 commits into
mainfrom
codex/pause-rebind-4703-2

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

The stalled database replica keeps updating its diagnostic status. A guarded recovery request can be rejected when its observed object version changes, even though the protected database state is unchanged. Stopping after every such rejection prevents the repair from completing.

What

  • Allow at most five newly guarded requests within one minute per pause or resume phase, only after a complete native rejection proves that request was not persisted.
  • Recheck all recovery guards and compare the protected state with the phase's original observation before every new request. Keep fresh identity/version tests and stop on changed state or an uncertain response.
  • Keep storage deletions and diagnostics single-shot, require a new explicit recovery confirmation, and test both successful races and every refusal boundary.

The API-server patch library is pinned in a test-only module. Production dependencies, manifests and permissions are unchanged. Publishing this change does not execute the repair; the approved recovery runs separately from reviewed main.

Fixes #4703
Part of #4673

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

User-path evaluation at ff568ae8f68f5a680118203155ed7a74efd107aa: exercised the actual native certificate transport against a synthetic API server using the pinned API-server JSON-patch implementation. A version change rejects the first request without persistence; a fully re-proven second request succeeds with fresh UID/version tests. Exhaustion stops at five. A lost success response is not retried, and changed protected state stops before another write.

The stateful completed-join recovery also completes pause, acknowledged cleanup, resume and separated replacement checks after verified pause/resume rejection. Changed source, backup, peer, leader, backing identity, retention or consumer evidence stops at the original write boundary in both phases. As the operator, these outcomes provide the needed liveness without turning an uncertain response into a destructive repeat. Production repair remains a separately approved current-main dispatch, not a claim made by this fixture.

Validation: full tagged race suite, tagged Go vet, affected production-path lint (zero findings), and source-freshness behavior/wiring checks pass. The diagnostic-only workflow remains one-shot. The test dependency is isolated from production modules and cannot route this scripts-only repair into a speculative production deployment.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 21 minutes.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

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

Next included review available in 21 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository YAML (base), Organization UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d9a1d699-9ad5-4c00-9b04-0bca680654d8

📥 Commits

Reviewing files that changed from the base of the PR and between 0528f7d and ff568ae.


⛔ Files ignored due to path filters (1)
  • scripts/replace-wedding-standby/cas-test.sum is excluded by !**/*.sum

📒 Files selected for processing (8)
  • .github/workflows/recover-retained-wedding-standby.yaml
  • scripts/replace-wedding-standby/README.md
  • scripts/replace-wedding-standby/cas-test.mod
  • scripts/replace-wedding-standby/guarded_cluster_patch.go
  • scripts/replace-wedding-standby/guarded_cluster_patch_test.go
  • scripts/replace-wedding-standby/main.go
  • scripts/replace-wedding-standby/quarantine.go
  • scripts/replace-wedding-standby/quarantine_test.go

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@devantler devantler left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Generated by the Agentic Engineer

Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)

Reviewed commit: ff568ae8f68f5a680118203155ed7a74efd107aa

Provider evidence re-read at 2026-10-10 11:39 UTC; current PR reviews, conversation, inline comments, threads and Bugbot checks were read directly. There are no substantive provider reviews or unresolved findings at this head.

  • CodeRabbit: this PR's current-head refusal, updated 2026-10-10 11:34:14 UTC, reports exhausted included OSS review capacity. Its 21-minute reset ends at 11:55:14 UTC. No paid review was requested.
  • Codex: account code-review usage refusal, updated 2026-10-09 18:11:46 UTC, applies to the authenticated account. It names credits/settings as recovery, with no reset timestamp. Bounded recent checks found no newer completion; this is not a full portfolio census.
  • Cursor Bugbot: user/team usage or spend refusal, updated 2026-10-09 17:30:48 UTC, applies to the configured user/team. Admin adjustment is the stated recovery; no reset timestamp is given. No limit increase was performed.

Reviewed correctness and security, including an independent exact-head review. Only a complete typed native rejection proving no commit permits another Cluster request. Every new request retains UID and version tests, re-proves the phase-specific guards, and compares frozen protected state. Diagnostic churn is narrowly normalized; generation, ownership, primary history and other status remain protected. Requests stop after five attempts or one minute per phase. Unknown or lost responses do not retry; diagnostic and storage deletion writes remain one-shot. New confirmation, main-only dispatch, first-attempt execution and the shared production lock remain enforced.

The full tagged race suite, ordinary race suite, tagged vet, production-path lint and source freshness checks pass. Stateful recovery and native transport fixtures cover rejection followed by safe completion, changed guard evidence, exhaustion and uncertain committed outcomes. No production repair is claimed by these fixtures.

Verdict: no P0/P1 findings

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

User-path evaluation at 9c9b77fd8c50631c4b42d85ef36d11d9b3935d20: the first CI run exposed a real compiler mismatch before executing the recovery tests. The test-only module required Go 1.26.9, while the workflow installs the root module's Go 1.26.6. A new regression check first failed on that mismatch; the test module now requires 1.26.6 and cannot demand a newer compiler than the workflow installs.

Ran the complete native-transport and stateful recovery race suite with the actual Go 1.26.6 toolchain: PASS (4.666 seconds). Tagged vet, production-path lint and diff checks pass. The exercised behavior still includes rejected pause/resume requests followed by fully re-proven success, uncertain committed responses without retry, exhausted request bounds and changed protection evidence stopping before another write. No production module dependency or recovery permission changed.

This corrects CI execution, not the database itself. Production recovery remains gated on this head's fresh review, CI, merge and the separately approved first main dispatch.

@devantler devantler left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 Generated by the Agentic Engineer

Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)

Reviewed commit: 9c9b77fd8c50631c4b42d85ef36d11d9b3935d20

Provider evidence re-read at 2026-10-10 11:45 UTC; current PR reviews, conversation, inline comments, threads and Bugbot checks were read directly. There are no substantive provider reviews or unresolved findings at this head. The ongoing applicable service limits are not reset by this test-only head update.

  • CodeRabbit: this PR's current-head refusal, updated 2026-10-10 11:34:14 UTC, reports exhausted included OSS review capacity. Its 21-minute reset ends at 11:55:14 UTC. No paid review was requested.
  • Codex: account code-review usage refusal, updated 2026-10-09 18:11:46 UTC, applies to the authenticated account. It names credits/settings as recovery, with no reset timestamp. Bounded recent checks found no newer completion; this is not a full portfolio census.
  • Cursor Bugbot: user/team usage or spend refusal, updated 2026-10-09 17:30:48 UTC, applies to the configured user/team. Admin adjustment is the stated recovery; no reset timestamp is given. No limit increase was performed.

Reviewed correctness and security, including an independent exact-head review. Only a complete typed native rejection proving no commit permits another Cluster request. Every new request retains UID and version tests, re-proves the phase-specific guards, and compares frozen protected state. Diagnostic churn is narrowly normalized; generation, ownership, primary history and other status remain protected. Requests stop after five attempts or one minute per phase. Unknown or lost responses do not retry; diagnostic and storage deletion writes remain one-shot. New confirmation, main-only dispatch, first-attempt execution and the shared production lock remain enforced.

The complete tagged race suite and tagged vet pass using the actual workflow compiler, Go 1.26.6; production-path lint and diff checks pass. The independent reviewer repeated tagged race and vet at this exact signed head with no P0/P1/P2 findings. Hosted recovery validation run 38049396822 also passes at this head. The added RED/GREEN regression prevents the test-only module demanding a newer compiler than the workflow installs; it compares numeric versions and fails closed on missing or malformed requirements. Stateful recovery and native transport fixtures cover rejection followed by safe completion, changed guard evidence, exhaustion and uncertain committed outcomes. No production dependency or guard changed in the toolchain correction, and no production repair is claimed by these fixtures.

Verdict: no P0/P1 findings

@devantler

devantler commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Readiness at 9c9b77fd8c50631c4b42d85ef36d11d9b3935d20:

  • CI 38049396829 passes, including manifest validation and required checks. Recovery validation 38049396822 runs the complete tagged race suite successfully with Go 1.26.6. All reported checks at this head are success or intentionally skipped.
  • The clean exact-head local review satisfies the reviewed fallback. Independent review also reports no P0/P1/P2 findings. The quota refusal is not a review verdict.
  • User-path evaluation exercises the real native transport fixture and complete guarded transaction; a fresh OIDC production plan passes without writes. These are not a claim of live recovery.
  • Fresh readback: zero unresolved review threads, no non-thread findings, no conflict, signed current head, owned isolated worktree, and native merge state CLEAN.

The separately approved production recovery remains a first current-main dispatch only after merge. No production manifests, production dependencies or storage protections are weakened by this PR.

The required-gate helper verifies the readable check/workflow gates but reports code_quality setup=configured UNVERIFIED because it has no head-analysis read surface. This is not recorded as PASS. The freshly read native CLEAN state remains authoritative for required checks under the merge policy; the normal merge queue still enforces the configured rule.

@devantler
devantler marked this pull request as ready for review October 10, 2026 11:59
@devantler
devantler added this pull request to the merge queue Oct 10, 2026
Merged via the queue into main with commit 3738188 Oct 10, 2026
39 checks passed
@devantler
devantler deleted the codex/pause-rebind-4703-2 branch October 10, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Keep guarded standby recovery live during status churn

1 participant