Skip to content

ci: parallelize and stabilize manifest validation - #5099

Merged
devantler merged 5 commits into
mainfrom
codex/ci-nested-5098
Oct 19, 2026
Merged

devantler merged 5 commits into
mainfrom
codex/ci-nested-5098

Conversation

@devantler

@devantler devantler commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Manifest validation waits for an independent template scan, delaying feedback. A disposable recovery check can also accidentally reuse a network endpoint and fail an otherwise unchanged validation run.

What

Run the independent scan alongside manifest validation while preserving all required outcomes. Stabilize the recovery check tracked in #5101 by keeping its initial endpoints separate until each server starts. An earlier pull-request check took 17m11s and the latest 14m57s; consistent feedback under fifteen minutes still needs verification.

Fixes #5098
Part of #4569

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Local validation at a9946463b7e5913138173923bd3ea298f244dd59 is complete:

  • RED: the new workflow tests failed before implementation because the independent PR job and strict result guard were absent. GREEN: nine unsafe graph mutations are refused and the actual Bash result guard passes its 48 selected/unselected/error cases.
  • go test ./... passed all 56 test-bearing packages. Affected merge-group, DR-signing and authorization packages also passed separately; golangci-lint reported zero issues.
  • The full, unchanged nested-template behavioral command passed with verified Trivy v0.74.0: 16 templates from two RGDs and two committed instances pass; graph, substitution, placement, registry and workload mutations are rejected. No finding baseline or scanner disposition changed.
  • Actionlint, YAML lint, ShellCheck, workflow-wiring ablations and all 32 deployment-order controls passed. Parsed merge-group scan, production deployment and recovery job definitions have identical before/after SHA-256 fingerprints.

Correctness/security and simplification passes found no remaining actionable issue. This is pre-submission hygiene, not the required substantive review. Hosted exact-head checks and a current-head green review are still required before promotion. Whole-PR budget enforcement remains under devantler-tech/.github#654; this change alone does not establish portfolio-wide compliance.

@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

Exercised behavior at a9946463b7e5913138173923bd3ea298f244dd59

I ran the complete pinned scanner regression locally and observed the actual hosted graph in run 39295841331. The independent scan passed before manifest validation finished, and the required selected-result guard passed on the real result. The PR-only production jobs were intentionally skipped. No scan baseline, version or behavioral probe was removed.

The existing reference run used the same pre-change workflow blob as this branch's base. These are individual observations, not a controlled cold/warm cohort or proof of portfolio-wide improvement:

Measure Reference Current
Manifest job execution 16m24s 13m38s
Repository CI creation to final actual job completion 17m11s 25m14s
Sum of successful jobs' occupied runner time 75m22s 73m37s
Native jobs, including intentional skips 22 23

The current scan job took 3m59s including setup; its full regression step took 3m16s. Path classification completed at 17:41:13 UTC, but manifest validation did not start until 17:48:54: a 7m41s post-dependency admission interval. Policy validation started 6m57s after classification. These intervals do not establish a particular quota or service cause, and runner-cost savings cannot be attributed to this change from one sample.

The manifest job is shorter, but the complete feedback target remains unmet. Preserve #4569 and devantler-tech/.github#654 for remaining execution/admission optimization and repeated measurement. All 41 reported checks are terminal and native mergeability reads CLEAN; the required workflow/status reads pass. The code-quality rule remains API-unverifiable in the completeness helper, not a claimed helper PASS. Substantive review is still pending; no promotion is claimed here.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

CodeRabbit did not acknowledge the composed request, created at 18:06:30 UTC, within the five-minute no-reaction window. Fresh reads after 18:11:39 show zero reactions, provider comments or review objects; its status still says automatic reviews are disabled, not that a manual review ran. The no-gate guard admitted progression at this head.

This is a PR/head-scoped unserved request, not an account-wide outage or a green review. Continue in the authorized order at Codex. Any later substantive CodeRabbit result still counts and must be read before promotion or merge.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T20:20:23.240401Z d61c31b 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

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: a9946463b7

ℹ️ 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".

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Readiness is established for a9946463b7e5913138173923bd3ea298f244dd59: the full hosted checks are terminal and green, signature is valid, native mergeability is CLEAN, and fresh reads found no unresolved threads, non-thread findings or maintainer requests. Codex completed a substantive review of this commit with no findings. The unserved CodeRabbit request and authorized progression are recorded above; its skipped status is not counted as a review.

The local regression, deployment-invariance and hosted behavioral evidence is recorded above. The measured evaluation explicitly retains the remaining 15-minute feedback work: a shorter manifest job is not portfolio-wide compliance. Promote this bounded increment and use the protected merge queue without bypassing its production or merge-group checks.

@devantler
devantler marked this pull request as ready for review October 19, 2026 18:21
@devantler
devantler added this pull request to the merge queue Oct 19, 2026
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Oct 19, 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: e8f594a5-6f8b-440b-a56e-6d6dea74f140
📥 Commits

Reviewing files that changed from the base of the PR and between bf1b90c and e4106e6.

📒 Files selected for processing: 7
  • .github/workflows/ci.yaml
  • AGENTS.md
  • docs/adr/ci-nested-template-parallelism.md
  • scripts/tests/openbao-health-native/listeners_test.go
  • scripts/tests/openbao-health-native/native_test.go
  • scripts/tests/test-rgd-template-static-scan-wiring.sh
  • scripts/validate-merge-group-heal/rgd_job_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
📚 Code guidelines: 1
AGENTS.md — auto-discovered
📓 Path-based instructions: 1
Source excerpt: **Directories are kebab-case**, named after the **application/component** *or* a **CR Kind in plural**.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • scripts/tests/openbao-health-native/listeners_test.go
  • scripts/tests/test-rgd-template-static-scan-wiring.sh
  • scripts/validate-merge-group-heal/rgd_job_test.go
  • scripts/tests/openbao-health-native/native_test.go
  • docs/adr/ci-nested-template-parallelism.md
🔇 Additional comments: 8
scripts/tests/openbao-health-native/native_test.go: 1

54-54: LGTM!

Also applies to: 217-219, 255-257, 259-260, 265-270, 324-331

scripts/tests/openbao-health-native/listeners_test.go: 1

13-60: LGTM!

.github/workflows/ci.yaml: 2

1693-1732: LGTM!

4091-4091: LGTM!

Also applies to: 4109-4130, 4169-4169

docs/adr/ci-nested-template-parallelism.md: 1

1-34: LGTM!

AGENTS.md: 1

141-147: LGTM!

scripts/tests/test-rgd-template-static-scan-wiring.sh: 1

60-81: LGTM!

Also applies to: 201-222

scripts/validate-merge-group-heal/rgd_job_test.go: 1

1-127: LGTM!

📝 Walkthrough

The nested RGD template scanner now runs in a dedicated, read-only pull-request job when Kubernetes changes are selected. The required-check aggregate validates the job result against the selection, and the scanner has been removed from the general validation job. Shell and Go tests check the workflow wiring and result guard. Documentation describes the scheduling rules and the separate merge-group and recovery scans. Native test servers retain API and Raft listener reservations until each server starts, and a test checks reservation uniqueness and release timing.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e4106

No merge-blocking issue is established. Complete the normal exact-head checks before merging.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

Failed Failed checks: 1 error · 1 warning

Check name Status Explanation Resolution
Out of Scope Changes check Error The changes in scripts/tests/openbao-health-native/native_test.go and scripts/tests/openbao-health-native/listeners_test.go reserve native OpenBao listener endpoints until each server starts and t… Move the OpenBao endpoint-reservation implementation and tests to a separate pull request linked to the issue that requires the recovery-check fix, or link an active issue that explicitly includes this work before merging this pull request.
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the two main changes: parallelizing manifest validation and stabilizing its recovery checks.
Linked Issues check Passed Issue #5098 coding requirements are met by the supplied change summary. .github/workflows/ci.yaml adds an independent, read-only validate-rgd-templates pull-request job with the existing k8s sel…
Description check Passed The description directly explains both main changes: parallelizing the independent template scan and stabilizing recovery-check endpoint reservations. It matches the pull request changeset and linked …
Full details: Linked Issues check

Explanation

Issue #5098 coding requirements are met by the supplied change summary. .github/workflows/ci.yaml adds an independent, read-only validate-rgd-templates pull-request job with the existing k8s selection, pinned Trivy setup, and the existing regression command. The required aggregate checks the new result and preserves unselected and merge-group behavior. Wiring and result-guard tests cover selected failures, cancellations, missing results, unexpected skips, unknown evidence, source mutations, shared selection, merge-group checks, and production ordering. The summary also records the required timing distinction and does not claim that #4569 is complete.

Full details: Description check

Explanation

The description directly explains both main changes: parallelizing the independent template scan and stabilizing recovery-check endpoint reservations. It matches the pull request changeset and linked objectives.

Full details: Out of Scope Changes check

Explanation

The changes in scripts/tests/openbao-health-native/native_test.go and scripts/tests/openbao-health-native/listeners_test.go reserve native OpenBao listener endpoints until each server starts and test that behavior. These changes do not implement or support issue #5098's nested-template scan scheduling or required-result logic. The current PR description states this additional recovery-check objective, but no directly linked issue supplies that coding scope.

Resolution

Move the OpenBao endpoint-reservation implementation and tests to a separate pull request linked to the issue that requires the recovery-check fix, or link an active issue that explicitly includes this work before merging this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files. (3 skipped: 3 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

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

@devantler
devantler marked this pull request as draft October 19, 2026 18:30
@devantler
devantler removed this pull request from the merge queue due to a manual request Oct 19, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

The late CodeRabbit report contains an ancillary predeclared diagnostic and a doc-comment warning even though its review summary reports no actionable comments. I addressed both in e3977c851c466fb3d0d23c5dbd027056de1e99a3: rename the local step copy to guardStep and document the four helper/test functions. The actual guard, workflow graph, production definitions and all negative scenarios are unchanged.

The affected Go package and golangci-lint pass. I explicitly removed the PR from the queue and returned it to draft before this follow-up. The earlier green review and hosted results remain evidence for the previous commit only; fresh exact-head CI and review are required before promotion and requeueing.

@devantler devantler changed the title ci: parallelize nested template validation ci: parallelize and stabilize manifest validation Oct 19, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

The failed bootstrap exposed the independently tracked fixture defect in #5101. Repeatedly opening and immediately closing each ephemeral socket left all six initial endpoints unowned; the OS could reuse one. The synthetic failure log uses an endpoint for both protocol roles, strongly consistent with this defect, although its truncated tails do not retain the complete original assignment.

Fixed in e4106e61827a1aff0cabbce30bd33fd7600ded28: retain initial listener reservations and release only the pair belonging to the server being started. Restart addresses, pinned server release, 90-second state bound, scenarios and verdicts remain unchanged. The unavoidable handoff to an external server process is not an atomic socket transfer; this prevents fixture-internal address reuse, not every possible external bind race.

  • RED: the deterministic real-allocation/start regression found all six endpoints released before startup and four peer endpoints released after starting only the first server.
  • GREEN: reservation/start cases pass three times with the race detector. The complete native server suite also passes three times with the race detector using the verified pinned 2.6.3 binary; bootstrap, sealed restart, leader change, unreachable peer, empty follower rejoin and uninitialized-peer verdicts all remain exercised.
  • The complete Go suite, affected vet and golangci-lint, formatting and diff checks pass. Advisory correctness/lifecycle review found no remaining actionable issue; it does not replace the substantive review gate.

This repairs the failed CI family rather than skipping it, increasing its timeout or retrying the same failing head. The PR remains draft for new exact-head hosted checks and review. The earlier timing observations remain limited reference samples, not proof that the complete feedback target is met. #5101 will be closed only after delivery is verified; #4569 remains open.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Batch correction for the repeated documentation/scope feedback at d61c31b2edbc7f27f26ea49aeb48a46bbc1b42b8, based on current main c9aa895d23ae607331d67afbf320b92a7e33024d:

  • The PR now directly links the existing native-fixture repair issue Reserve distinct listener addresses in the native OpenBao health fixture #5101 in its scope; this is the already-tracked repair, not an unrelated addition or a newly invented acceptance exemption.
  • Audited the complete changed-function inventory. The two missing helper comments now describe initial listener ownership and restart behavior; the other six changed functions already have meaningful comments.
  • Merged the current default branch normally and signed both commits. That base advancement changes only the existing KSail operator pin; no deployment guard, validation outcome or native recovery assertion was removed.
  • Full local Go suite passed. The unchanged credential-refresh fixture package took 378.990 seconds under its existing twenty-minute CI test budget. Its earlier local three-minute deadline failure is not counted as a pass; no repository timeout was increased.
  • The pinned native OpenBao recovery race suite also passed at this exact head in 56.318 seconds, retaining its existing strict scenario deadlines and cleanup.

The source correction is pushed. The new exact-head hosted checks and substantive review are still required before promotion; the earlier head's review does not satisfy this head. Native protected queue/deployment acceptance remains mandatory after readiness. The broader fifteen-minute objective in #4569 is not complete.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

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.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

CodeRabbit declined this exact head at 20:15:30 UTC (updated 20:15:41 UTC). Its repository summary, updated at 20:15:38 UTC, reports the next included review in 11 minutes, around 20:27 UTC. A fresh complete no-gate guard admits progression at this head. The refusal is not approval; no paid continuation is enabled.

The earlier documentation/scope findings were addressed together in the correction record, not discarded because of the limit. Requesting the next authorized provider only.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: d61c31b2ed

ℹ️ 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".

@devantler
devantler marked this pull request as ready for review October 19, 2026 20:22
@devantler
devantler added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit b763393 Oct 19, 2026
41 checks passed
@devantler
devantler deleted the codex/ci-nested-5098 branch October 19, 2026 21:07
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 19, 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.

Run nested template scanning alongside manifest validation

1 participant