Skip to content

fix: recognize the audited operator pause acknowledgment - #4706

Merged
devantler merged 1 commit into
mainfrom
codex/pause-ack-protocol-4705
Oct 10, 2026
Merged

devantler merged 1 commit into
mainfrom
codex/pause-ack-protocol-4705

Conversation

@devantler

@devantler devantler commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Database recovery can stop after the operator confirms its pause because the confirmation uses a different log level than the recovery expects. That leaves the database below its declared readiness.

What

Recognize the audited operator's confirmation while retaining the identity, freshness and replay protections. This does not authorize another recovery attempt or resume an existing pause.

Fixes #4705

Part of #4673

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository YAML (base), Organization UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d352bebf-68fd-4d46-a193-972474770d17


📥 Commits

Reviewing files that changed from the base of the PR and between 3738188 and 27222dd.



📒 Files selected for processing (4)
  • scripts/replace-wedding-standby/README.md
  • scripts/replace-wedding-standby/pause_protocol_test.go
  • scripts/replace-wedding-standby/quarantine.go
  • scripts/replace-wedding-standby/quarantine_test.go


Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📜 Recent review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: 🔍 Detect Changes
  • GitHub Check: 🔐 Validate Consumer Discovery Regressions
  • GitHub Check: 🔏 Validate Cosign Matcher Efficacy


🔇 Additional comments (4)
scripts/replace-wedding-standby/quarantine.go (1)

200-202: LGTM!

Also applies to: 211-211


scripts/replace-wedding-standby/quarantine_test.go (1)

82-84: LGTM!

Also applies to: 161-161


scripts/replace-wedding-standby/pause_protocol_test.go (1)

9-52: LGTM!


scripts/replace-wedding-standby/README.md (1)

23-26: LGTM!





📝 Walkthrough
📝 Walkthrough

Walkthrough

The pause acknowledgment parser now requires the controller’s structured info level instead of warning. Its other acknowledgment checks remain unchanged. Tests cover the expected record, rejected levels and malformed records, and replayed reconciliation IDs. The fixture and README now describe the info-level protocol.



Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 27222

This change makes recovery recognize the controller's info-level pause acknowledgment, and the existing identity, freshness, and replay protections stay in place. No merge-blocking risk was found, and the tests and README cover the new level.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check Passed Issue #4705 is active and directly linked. pauseAcknowledged now requires the exact info level and retains the message, namespace, cluster, reconcile-ID, freshness, and replay checks. The focused …
Out of Scope Changes check Passed The changes stay within issue #4705. The production change updates the acknowledgment protocol level. The test changes verify the protocol and preserve the existing recovery guards. The README records…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (1 skipped: 1 …
Title check Passed The title clearly summarizes the primary change: recognizing the audited operator pause acknowledgment.
Description check Passed The description explains the recovery problem, the intended recognition change, and the protections that remain unchanged. It matches the changeset.

  • 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

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Readiness receipt for 27222dd1d2c1793119f016c7475f2a50ab45c279.

The regression first failed against the previous parser: it rejected the audited info record and accepted warning. It now passes. Both the complete recovery test suite and its API-server patch variant pass with the race detector; go vet also passes. The independent protocol fixture and the stateful recovery fixtures exercise the acknowledgment consumed by the recovery transaction, including rejection of other levels and replayed records.

As a recovery operator, I also ran the current command in read-only mode against the existing paused state. It correctly returned HOLD rather than admitting a paused cluster into a new initial recovery. That check made no production writes and is not evidence that the database has recovered.

CodeRabbit's substantive review covers this exact head and reports no actionable findings. PR CI, retained-recovery tests, replacement safety tests, and pause-diagnostic tests passed. Production recovery jobs were skipped. There are no unresolved review threads or non-thread findings, and GitHub reports MERGEABLE/CLEAN.

The required-gate helper leaves the separate code-quality ruleset analysis UNVERIFIED because its head-specific result is not exposed through that helper. I have not relabeled it PASS; GitHub's native merge gate remains authoritative and will enforce the rule.

This is the narrow parser repair, not permission to replay the consumed recovery attempt or proof of restored database redundancy. A new identity-bound continuation of the existing owned pause remains separately approval-gated.

@devantler
devantler marked this pull request as ready for review October 10, 2026 13:23
@devantler
devantler added this pull request to the merge queue Oct 10, 2026
Merged via the queue into main with commit 9f3783e Oct 10, 2026
38 checks passed
@devantler
devantler deleted the codex/pause-ack-protocol-4705 branch October 10, 2026 13:32
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 10, 2026
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.

Retained standby recovery rejects the operator pause acknowledgment

1 participant