Skip to content

fix(recovery): confirm the fenced standby without Pod ingress - #4681

Merged
devantler merged 2 commits into
mainfrom
codex/standby-proof-4673-2
Oct 9, 2026
Merged

devantler merged 2 commits into
mainfrom
codex/standby-proof-4673-2

Conversation

@devantler

@devantler devantler commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

The Wedding database repair stops before replacing its failed replica because it cannot confirm that the replica has stopped writing. This leaves the database running with reduced redundancy even though its healthy replicas and backups remain intact.

What

Restore that confirmation without opening network access or weakening the safety checks. Add a protected, read-only confirmation step before the repair proceeds. Allow a specifically approved repair to continue from the existing safe stopping point, keeping the old data and both healthy replicas untouched.

Part of #4673

Read the fencing gauge through a bounded loopback probe in the bound replica. Permit an explicitly selected continuation only while the original repair owns the fence and retained storage stays attached.\n\nPart of #4673
@devantler

devantler commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Validation at 126446d4bdbac86943cd5d0560b7dfee69c1506b:

  • Regression tests first reproduced the inaccessible exporter route, omitted ownership evidence, and loss of fence ownership before a mutation.
  • The full affected Go suite passes with the race detector and 80.1% statement coverage. Go vet, the shared CI's pinned Go linter, both workflows' validation, and whitespace checks also pass.
  • A real HTTP fixture exercises the static loopback request. The current-head safety run also passed that same request inside the immutable database image, without production credentials.
  • The read-only continuation plan passed against the current production state. It used OIDC reads only: no storage changes, Pod deletion, or command execution inside a Pod.

The new dispatch-only observation workflow is the bridge for the production acknowledgment that local OIDC access cannot reach. Its tests forbid mutations and reject failed reads, foreign ownership, changed peers and changed storage. After merge it must pass from current main before any continuation is considered.

The protected continuation is separate from this code validation. It still requires specific approval and a fresh main-bound dispatch. The failed attempt retained the original storage and did not change either healthy replica. No live recovery clearance is claimed here.

Observe the bound exporter and unchanged retained storage without entering the repair mutation path.
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Oct 9, 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: fd7e9eaf-7072-41f0-b2de-f395d43a280f

📥 Commits

Reviewing files that changed from the base of the PR and between e0c5558 and 126446d.


📒 Files selected for processing (5)
  • .github/workflows/replace-wedding-standby.yaml
  • .github/workflows/verify-wedding-fence.yaml
  • scripts/replace-wedding-standby/README.md
  • scripts/replace-wedding-standby/main.go
  • scripts/replace-wedding-standby/main_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
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-07-28T00:33:10.828Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 2825
File: .github/workflows/dr-rebuild.yaml:70-73
Timestamp: 2026-07-28T00:33:10.828Z
Learning: For GitHub Actions concurrency configuration in this repo, don’t use `queue: single` on a shared production concurrency group (e.g., `prod-deploy`) that is used for normal production deployments—`queue: single` will cancel/replace any pending run in that group. Normal production deployments must use a uniform `queue: max` for that shared group to ensure serialization without coalescing. If you need DR rebuild coalescing (replacement of pending DR rebuild runs), it must use a separate DR-only concurrency scope/group (not the shared production `prod-deploy` group).

Applied to files:

  • .github/workflows/verify-wedding-fence.yaml


🪛 ast-grep (0.45.3)
scripts/replace-wedding-standby/main_test.go

[warning] 27-36: This http.Server is constructed without a ReadTimeout. Without a read timeout, a slow or malicious client can hold connections open indefinitely (e.g. a Slowloris attack), exhausting server resources and causing a denial of service. Set ReadTimeout (and ideally ReadHeaderTimeout, WriteTimeout, and IdleTimeout) on the http.Server to bound how long the server waits while reading a request.
Context: http.Server{ReadHeaderTimeout: time.Second, Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != "GET" || r.URL.Path != "/metrics" || r.Proto != "HTTP/1.0" {
t.Errorf("unexpected exporter request: %s %s %s", r.Method, r.URL.Path, r.Proto)
http.Error(w, "unexpected request", http.StatusBadRequest)
return
}
if _, err := fmt.Fprintln(w, "cnpg_collector_fencing_on 1"); err != nil {
t.Error(err)
}
})}
Note: [CWE-400] Uncontrolled Resource Consumption.

(http-server-missing-read-timeout-go)



🪛 golangci-lint (2.13.2)
scripts/replace-wedding-standby/main.go

[error] 233-233: comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error

(errorlint)


scripts/replace-wedding-standby/main_test.go

[error] 23-23: net.Listen must not be called. use (*net.ListenConfig).Listen

(noctx)




🔇 Additional comments (6)
scripts/replace-wedding-standby/main.go (2)

348-358: LGTM!


822-839: LGTM!


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

68-87: LGTM!


.github/workflows/replace-wedding-standby.yaml (1)

117-120: LGTM!


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

20-69: LGTM!


.github/workflows/verify-wedding-fence.yaml (1)

1-82: LGTM!





📝 Walkthrough

Walkthrough

The standby tool now reads fencing metrics through a bounded request inside the identified Pod and checks the Pod identity before and after the request. A new protected workflow runs read-only fence proof. The repair workflow adds an explicit resume input, and the tool permits continuation only from a qualifying repair-owned fenced HOLD. Tests and documentation cover the observation, proof, and resume paths.


Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 12644

No PR-introduced issue requiring a fix before merge was established. The protected proof remains read-only, and continuation retains its stated safety checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 96.43% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 2 files. (3 skipped: 3 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly summarizes the main change: confirming the fenced standby without Pod ingress.
Description check Passed The description directly explains the recovery problem and the added protected confirmation and continuation behavior.


  • 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

Resolution of the three ancillary findings in the current-head review, at 126446d4bdbac86943cd5d0560b7dfee69c1506b:

  • main.go:233, direct io.EOF comparison: this reader is constructed directly over a completed bytes.Reader, not a network reader or a reader that wraps errors. bufio.Reader.Peek(1) returns that reader's literal EOF once exhausted. Equality therefore checks the exact required condition here; any other error rejects the proof. The truncated, trailing-data and oversized-response regression cases pass. This is not a wrapped-error correctness defect.
  • main_test.go:23, net.Listen: this is a test-only listener bound to the literal IPv4 loopback address, with no name lookup or outbound connection to cancel. Its lifetime is closed by test cleanup; the sole probe process has a 20-second context deadline and the suite has a two-minute deadline. Adding a dial/listen context would not change the observed fixture behaviour. This does not expose an unbounded production operation.
  • main_test.go:27-36, server read timeout: the test fixture already has a one-second ReadHeaderTimeout. Its only client is the static, bodyless HTTP/1.0 GET shown in metricsRequest, not an untrusted production client; it has no request body read, streaming handler or externally reachable listener. Cleanup closes the server. The suggested Slowloris/resource-exhaustion production scenario does not apply to this bounded fixture.

These findings are refuted on their specific code paths, not waived because CI passes. No file or reviewed SHA changes. body_findings=0-resolved@126446d4bdbac86943cd5d0560b7dfee69c1506b. Restarting the ordered review loop at this same head before promotion.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 9, 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 50 minutes.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

After the disclosed same-head resolution at 2026-10-09T08:12:06Z, CodeRabbit's reply reports the included review limit reached at 08:12:20Z, with the next included review in 50 minutes. This is fresh PR-scoped unavailability, not a code verdict. The head status corroborates “Review rate limited”; the round-scoped no-gate guard returned ADMIT. No paid allowance is enabled or authorized. Continuing to Codex under the same request lock.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@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.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

At 2026-10-09T08:13:39Z the authenticated Codex reply reports the code-review usage limit reached on this PR. Observed freshly at 08:15Z; no reset was supplied. The current-round no-gate guard returned ADMIT. No credits are purchased or enabled. Continuing to Bugbot under the same current-head lock.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@devantler

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor

cursor Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: a79f8bf6-7ebb-4af5-bbe9-21d86ff4f072)

@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: 126446d4bdbac86943cd5d0560b7dfee69c1506b

  • CodeRabbit: PR-scoped included-review limit, event 2026-10-09T08:12:20Z; next included review 50 minutes later. The previous round's three ancillary findings have an explicit same-head resolution record. The restarted round returned no review.
  • Codex: PR-scoped code-review usage limit, event 2026-10-09T08:13:39Z; no reset supplied.
  • Cursor Bugbot: PR-scoped usage/spend limit, event 2026-10-09T08:14:55Z; no reset supplied. Check 113732239666 is neutral/Error, not a green review.

All three sources were freshly read before this fallback. Review objects and inline comments are empty; none of the limit messages reports a code finding. Each current-round no-gate guard returned ADMIT. No paid overage, credits or permission changes are used.

Reviewed all five changed files for correctness and security, with particular attention to: fixed target/container/command selection; first-dispatch and current-main binding; no mutation in the proof entry point; remote command timeout and complete HTTP framing; identity reads surrounding the exporter observation; exclusive fence ownership and retained original storage before continuation writes; conditional deletion of only the identified failed Pod; unchanged healthy peers; and two separated recovery observations. Reviewed the regression cases and the actual immutable database-image probe CI evidence. The privileged production acknowledgment is deliberately reserved for the new reviewed, read-only main workflow; it is not claimed as already observed.

Verdict: no P0/P1 findings

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

User-path evaluation at 126446d4bdbac86943cd5d0560b7dfee69c1506b: safety CI 37901055989 completed successfully at this exact head. It exercised the actual static HTTP probe inside the immutable database image, against a real loopback exporter fixture, and tested the guarded continuation and read-only proof boundaries. The OIDC-only command was exercised again against production and returned PLAN=PASS no writes, with the expected original storage and healthy-peer identities still present.

The specific unreachable gate is the instance's live in-Pod fencing acknowledgment: local access remains OIDC-only. The new dispatch-only, read-only, typed-confirmation main workflow is the reviewed bridge required by the deployment contract. It must pass after merge before the separate, specifically approved continuation can proceed. Neither the CI fixture nor the plan is represented as a production fencing acknowledgment or completed repair.

The current-head local-review fallback is posted as a real review and classified GREEN by the reviewed helper. Required status/workflow checks pass. The completeness helper explicitly reports the active code-quality rule as UNVERIFIED, not complete; GitHub's native merge gate remains the authority for that rule. No bypass is used.

@devantler
devantler marked this pull request as ready for review October 9, 2026 08:22
@devantler
devantler added this pull request to the merge queue Oct 9, 2026
@devantler
devantler removed this pull request from the merge queue due to a manual request Oct 9, 2026
@devantler
devantler added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit ddb8016 Oct 9, 2026
36 checks passed
@devantler
devantler deleted the codex/standby-proof-4673-2 branch October 9, 2026 08:47
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 9, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Historical attribution correction for comment 6077129558: it was an automated Agentic Engineer review request at 126446d4bdbac86943cd5d0560b7dfee69c1506b, paired with the preceding disclosed request marker. This disclosed correction supersedes any interpretation of that comment as a human maintainer instruction.

The original body includes a trailing newline, so it fails the byte-exact bare-trigger disclosure exemption. The historical request and provider's usage-limit response remain intact. This comment records attribution only. The disclosure guard still reports the original malformed body; no clean-sweep or new review verdict is claimed.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Correction for the record: this earlier comment was posted by an agent run, not typed by the maintainer. It was a bare review trigger sent without the disclosure line. It is agent output and carries no instruction.

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.

1 participant