[devantler]

🤖 Generated by the Agentic Engineer

Why

OpenBao server upgrades are on hold because nothing can prove the secrets service is healthy before the next server is replaced. Kubernetes reports a sealed server as ready on purpose, for bootstrap, so its readiness cannot be that proof.

What

As the first step toward safe upgrades, add a tool that reads one captured health snapshot of every OpenBao server and changes nothing in production. It says "proven" only when each expected server is unsealed, initialized and on the expected release, in one cluster with exactly one active server. Missing, old or ambiguous evidence is never proven.

🤖 Generated with Claude Code

Fixes #4962
Part of #4756

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Validation at head 93b7405b4ea4784a2477ba7d3257fb4c1e10aa47

• Regressions (GOTOOLCHAIN=go1.26.9 go test ./internal/strictjson ./scripts/judge-openbao-health ./scripts/verify-arc-transport): all pass. The judge's suite covers 7 HOLD cases, 31 UNKNOWN cases, UNKNOWN outranking HOLD, unread native fields, argument validation, and the built command's exit codes. It also checks that the output never echoes a pod or cluster identity. The ARC transport proof's existing table still passes on the shared strict-JSON check, which has its own 13-case table.
• Mutation checks: with the sealed rule disabled, the HOLD and ordering tests fail. With the HTTP-status rule disabled, the status-disagreement case fails. Both pass again once restored.
• Real response shape: OpenBao's published api/v2 HealthResponse omits cluster_name and cluster_id while a server is sealed or uninitialized. The first commit required the identity before reading the seal state, which turned a sealed server into UNKNOWN instead of HOLD. The second commit judges the seal state first, and the fixtures now mirror that shape.
• Tried as a user: I ran the built command on captures shaped like real 2.6.3 responses:
  • three healthy servers → PROVEN, exit 0;
  • one sealed server answering 503 → HOLD naming openbao-1: server is sealed, exit 1;
  • one server missing → UNKNOWN, exit 2;
  • a ten-minute-old capture → UNKNOWN: capture is stale, exit 2;
  • the same healthy capture judged against release 2.6.4 → HOLD on all three servers, exit 1.
• Lint: golangci-lint run reports no issues.
• No runtime change: nothing in production calls the judge yet. It is an offline tool that Gate OpenBao upgrades on unsealed recovery proofs #4756's later protected procedure will call, so there is no flag to add and nothing to deploy-verify.

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Validation at head 93f397e4a2db8f695cf766fd0160609cdffa91ef, after an internal review pass before requesting an external review.

The internal review found three gaps at 93b7405b, each reproduced with the built command. All three are fixed in the latest commit:

• -h or --help exited 0 with nothing printed, which a caller gating on the exit code would read as PROVEN. Every argument error now exits 2, help included.
• A repeated flag silently replaced the earlier value. For example, a trailing --expect-servers openbao-0 judged a one-server capture PROVEN. Any flag given twice is now refused.
• Within one server, a definite finding was checked before evidence that should have made it UNKNOWN, such as a missing HTTP status or an empty cluster identity. Every completeness check now runs before the check it guards.

Also new in this commit: each body's own server_time_utc must fall inside the capture window, within a minute either way, so a fresh envelope cannot carry an old body. The escaped-key regressions now contain a real \u escape; the earlier cases had been unescaped when the file was written.

• Regressions: go test for internal/strictjson, scripts/judge-openbao-health and scripts/verify-arc-transport passes, including under -race. The judge covers 7 HOLD cases, 40 UNKNOWN cases and 10 command-level exit-code cases. golangci-lint reports no issues.
• Mutation checks: disabling the server-time window fails both stale-body cases. Disabling the repeated-flag refusal fails both repeated-flag cases.
• The earlier user-level run of the built command (healthy, sealed, missing, stale and wrong-release captures) still holds, now with bodies stamped inside the capture window.

[devantler]

🤖 Generated by the Agentic Engineer

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-16T20:56:39.654316Z	93f397e	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

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]

🤖 Generated by the Agentic Engineer

@coderabbitai review

[coderabbitai]

✅ 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]

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: ce84e133-a723-48cc-840e-b7d24a069a67

📥 Commits

Reviewing files that changed from the base of the PR and between 6c58640 and 93f397e.

📒 Files selected for processing: 6

• internal/strictjson/strictjson.go
• internal/strictjson/strictjson_test.go
• scripts/judge-openbao-health/README.md
• scripts/judge-openbao-health/main.go
• scripts/judge-openbao-health/main_test.go
• scripts/verify-arc-transport/json_proof.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

🪛 golangci-lint (2.14.0)

scripts/judge-openbao-health/main_test.go

[error] 255-255: type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors

(errorlint)

[medium] 215-215: G204: Subprocess launched with variable

(gosec)

[medium] 250-250: G204: Subprocess launched with variable

(gosec)

🔇 Additional comments: 5

internal/strictjson/strictjson.go: 1

1-86: LGTM!

internal/strictjson/strictjson_test.go: 1

1-49: LGTM!

scripts/verify-arc-transport/json_proof.go: 1

3-3: LGTM!

Also applies to: 22-22

scripts/judge-openbao-health/main.go: 1

1-373: LGTM!

scripts/judge-openbao-health/README.md: 1

1-72: LGTM!

📝 Walkthrough

The change adds strictjson.Validate and uses it in the ARC transport proof. It also adds an offline command that judges OpenBao health from one captured observation. The command checks freshness, server identity, health response, version, cluster identity, and active-server count. It reports PROVEN, HOLD, or UNKNOWN. Tests cover the validator, judgment outcomes, argument handling, and report output. Documentation describes the capture format and the scope of the health verdict.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 93f39

No actionable issue remains from this review; the change is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 5 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Issue #4962 has coding requirements, and the reviewed changes satisfy them. judge-openbao-health takes expected servers and release from arguments, validates one bounded capture with shared strict J…
Out of Scope Changes check		The changes remain within issue #4962. The shared internal/strictjson validator and its ARC transport integration directly implement the required common strict-JSON check. The judge documentation an…
Description check		The description explains why the offline OpenBao health judge is needed and what it does. It directly matches the changeset.
Title check		The title clearly identifies the main change: adding an OpenBao tool that judges server health from captured evidence.

Full details: Linked Issues check

Explanation

Issue #4962 has coding requirements, and the reviewed changes satisfy them. judge-openbao-health takes expected servers and release from arguments, validates one bounded capture with shared strict JSON checks, requires fresh complete evidence for each expected server, and returns PROVEN only for initialized, unsealed, expected-release servers in one non-empty cluster with exactly one active server. The tests cover HOLD and UNKNOWN controls, precedence, exit codes, private output, freshness, malformed or ambiguous JSON, and argument validation. The command documents that health is not Raft quorum proof.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #4962. The shared internal/strictjson validator and its ARC transport integration directly implement the required common strict-JSON check. The judge documentation and regression tests support the offline health decision. The reviewed changes do not add production collection, replacement or restart actions, or Raft membership or quorum reads.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 5 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Readiness at head 93f397e4a2db8f695cf766fd0160609cdffa91ef

• Programmatically tested: every check at this head is green, including the Go helper suite and the ARC transport proof boundaries. The required gates are CI - Required Checks plus the three required workflows, all PASS; the code_quality rule reports its standing UNVERIFIED. There are zero review threads and no conflict with main. The regression and mutation evidence is in the validation comment above.
• Reviewed: a CodeRabbit full review of 6c586407..93f397e4 reports no actionable comments. Codex was requested first at this head, and its review failed on the account's usage limit (20:56 UTC); that failure carried no findings.
• Tried as a user: I ran the built command on realistic captures at this head. It returned PROVEN (exit 0) for three healthy servers, HOLD (exit 1) for a sealed server and for the wrong release, and UNKNOWN (exit 2) for a missing server, a stale capture, -h and repeated flags.
• Runtime surface: none yet. Nothing in production calls the judge until Replace OpenBao servers one at a time through a protected, health-gated procedure #4970's protected procedure exists, so the merge-group deploy changes no running workload.

Promoting and queueing at this head.

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Merged. This PR went through the queue at 21:29 UTC as e53121f7f09c1828e1b7cfb5639e982171f6c880, after its merge-group run, including the production deployment, passed. The judge is an offline tool that nothing in production calls yet, so there is no live behaviour to read back. Its first consumer is #4970's protected procedure, preceded by the native-server exercise in #4969.