WIP: merge plugin system#497
Open
honza wants to merge 6 commits into
Open
Conversation
Signed-off-by: s3rj1k <evasive.gyron@gmail.com>
Signed-off-by: s3rj1k <evasive.gyron@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a runtime Go plugin system for provisioners: load .so plugins from /plugins, validate PluginName/NewProvisionerFactory (optional HostConfigure), replace mode flags with --provisioner, and update builds/Makefile/Tiltfile/Dockerfiles to build, test, and ship plugin artifacts. ChangesGo Plugin System for Provisioners
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: honza The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Enable CGO_ENABLED=1 for the builder stage (required by Go's plugin package which uses dlopen/dlsym) and copy the ironic and demo provisioner plugin .so files to /plugins/ in the final image. The base-rhel9 runtime image already includes glibc, so no base image change is needed (unlike upstream which switched from distroless/static to distroless/base-debian13). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
hack/plugin-test/patcher/main.go (1)
65-74: 💤 Low valueGuard type assertion to prevent panic.
Line 70 performs an unchecked type assertion
bodyFile.Decls[0].(*ast.FuncDecl). WhilenewBodySrcis currently a constant that parses correctly, if the constant is modified incorrectly in the future, this will panic rather than returning a helpful error.🛡️ Proposed fix to add type assertion guard
- newBody := bodyFile.Decls[0].(*ast.FuncDecl).Body + funcDecl, ok := bodyFile.Decls[0].(*ast.FuncDecl) + if !ok || funcDecl.Body == nil { + die("internal error: newBodySrc did not parse to a function with body") + } + newBody := funcDecl.Body🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/plugin-test/patcher/main.go` around lines 65 - 74, The unchecked type assertion on bodyFile.Decls[0] can panic; guard it by verifying bodyFile.Decls has at least one declaration and using the comma-ok form to assert *ast.FuncDecl (e.g. replace the direct assertion bodyFile.Decls[0].(*ast.FuncDecl) with a safe check: ensure len(bodyFile.Decls) > 0, do decl, ok := bodyFile.Decls[0].(*ast.FuncDecl) and if !ok or decl.Body == nil call die with a clear message), then use decl.Body as newBody when calling replaceMethodBody("demoProvisioner", "GetHealth", newBody).Dockerfile.plugin-test (1)
61-65: ⚡ Quick winHarden the final test stage.
Line 61 still uses the full Go SDK image, and this stage never drops root before executing
plugin-load-teston Line 65. Since this stage only needs the two compiled binaries plus the.so, switch it to a minimal runtime base and set a non-rootUSER; if you use a shell-less base, make Line 65 an exec-formRUN. As per coding guidelines,Multi-stage builds; no build tools in final imageandUSER non-root; never run as root.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile.plugin-test` around lines 61 - 65, Change the plugin-load-tester stage to use a minimal runtime base (not the full Go SDK) and run as a non-root user: replace FROM $BUILD_IMAGE AS plugin-load-tester with a lightweight runtime image (e.g., alpine or scratch + required libc) and create or switch to a non-root user (USER <nonroot>), ensure the copied artifacts remain /usr/local/bin/baremetal-operator, /usr/local/bin/plugin-load-test and /plugins/demo-provisioner.so, and make the plugin test invocation use the exec form (RUN ["/usr/local/bin/plugin-load-test","/plugins/demo-provisioner.so","demo","healthy"]) if you pick a shell-less base so the test runs without root privileges.Sources: Coding guidelines, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/plugin-load-test/main.go`:
- Around line 76-87: The code uses context.Background() for calling
prov.GetHealth which can hang; change to a cancellable context with a timeout by
creating a context via context.WithTimeout (e.g., 5s or configurable), defer
cancel(), and pass that ctx into factory.NewProvisioner and prov.GetHealth;
update uses of ctx and ensure the timeout context is used for the GetHealth
validation to avoid indefinite blocking when prov.GetHealth hangs.
In `@Dockerfile`:
- Line 3: The ARG BASE_IMAGE currently points to gcr.io/distroless/base-debian13
which violates the repo policy; update the Dockerfile to use a compliant base
(either a UBI minimal image or a distroless image hosted on catalog.redhat.com)
by replacing the value of ARG BASE_IMAGE with the appropriate catalog.redhat.com
image (retain digest pinning) and keep the ARG name BASE_IMAGE unchanged so
downstream references (ARG BASE_IMAGE) and subsequent FROM instructions continue
to work.
In `@hack/plugin-test/patcher/main.go`:
- Around line 76-84: The current logic opens the target file with os.Create
(out) before calling format.Node, which can truncate the file if formatting
fails; change this to write to a temporary file and atomically rename it into
place: use os.CreateTemp (or ioutil.TempFile) to create a temp file in the same
directory, call format.Node writing to that temp file, close it, set file mode
to match the original (use os.Stat to get permissions if needed), then rename
the temp file over the original via os.Rename; ensure you remove the temp file
on any error and keep the die(...) error handling for failures (referencing
os.Create, format.Node, defer out.Close, die, os.Rename, os.CreateTemp/os.Stat).
In `@Makefile`:
- Around line 201-210: The Makefile's manager, run, and demo targets build/run
main.go without CGO, causing divergence from Dockerfile/Tiltfile; update the
targets (manager, run, demo) to set CGO_ENABLED=1 when invoking go build or go
run (e.g., prefix commands with CGO_ENABLED=1) so the manager binary is built
with cgo enabled for dynamic plugin loading, ensuring consistency with the
Dockerfile and Tiltfile behavior.
---
Nitpick comments:
In `@Dockerfile.plugin-test`:
- Around line 61-65: Change the plugin-load-tester stage to use a minimal
runtime base (not the full Go SDK) and run as a non-root user: replace FROM
$BUILD_IMAGE AS plugin-load-tester with a lightweight runtime image (e.g.,
alpine or scratch + required libc) and create or switch to a non-root user (USER
<nonroot>), ensure the copied artifacts remain
/usr/local/bin/baremetal-operator, /usr/local/bin/plugin-load-test and
/plugins/demo-provisioner.so, and make the plugin test invocation use the exec
form (RUN
["/usr/local/bin/plugin-load-test","/plugins/demo-provisioner.so","demo","healthy"])
if you pick a shell-less base so the test runs without root privileges.
In `@hack/plugin-test/patcher/main.go`:
- Around line 65-74: The unchecked type assertion on bodyFile.Decls[0] can
panic; guard it by verifying bodyFile.Decls has at least one declaration and
using the comma-ok form to assert *ast.FuncDecl (e.g. replace the direct
assertion bodyFile.Decls[0].(*ast.FuncDecl) with a safe check: ensure
len(bodyFile.Decls) > 0, do decl, ok := bodyFile.Decls[0].(*ast.FuncDecl) and if
!ok or decl.Body == nil call die with a clear message), then use decl.Body as
newBody when calling replaceMethodBody("demoProvisioner", "GetHealth", newBody).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 40d21e6a-71dd-47ee-a8ae-98af6f634b16
📒 Files selected for processing (15)
DockerfileDockerfile.plugin-testMakefileTiltfilecmd/plugin-load-test/main.goconfig/overlays/fixture/kustomization.yamldocs/configuration.mddocs/dev-setup.mddocs/plugin-provisioners.mdhack/plugin-test/patcher/main.gomain.gopkg/provisioner/demo/plugin/main.gopkg/provisioner/ironic/plugin/main.gopkg/provisioner/plugin.gopkg/provisioner/plugin_test.go
Comment on lines
+76
to
+87
| ctx := context.Background() | ||
|
|
||
| prov, err := factory.NewProvisioner(ctx, provisioner.HostData{}, func(_, _ string) {}) | ||
| if err != nil { | ||
| log.Fatalf("NewProvisioner: %v", err) | ||
| } | ||
| if prov == nil { | ||
| log.Fatal("NewProvisioner returned nil") | ||
| } | ||
|
|
||
| health := prov.GetHealth(ctx) | ||
| log.Printf("GetHealth OK output=%q", health) |
There was a problem hiding this comment.
Add timeout to context for GetHealth validation.
The tool uses context.Background() without a timeout when calling plugin methods. If a plugin's GetHealth() implementation hangs or blocks indefinitely, this validation tool will also hang, making it difficult to detect misbehaving plugins in CI/test environments.
⏱️ Proposed fix to add context timeout
- ctx := context.Background()
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
prov, err := factory.NewProvisioner(ctx, provisioner.HostData{}, func(_, _ string) {})As per coding guidelines, Go code should use "context.Context for cancellation and timeouts" to prevent operations from hanging indefinitely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/plugin-load-test/main.go` around lines 76 - 87, The code uses
context.Background() for calling prov.GetHealth which can hang; change to a
cancellable context with a timeout by creating a context via context.WithTimeout
(e.g., 5s or configurable), defer cancel(), and pass that ctx into
factory.NewProvisioner and prov.GetHealth; update uses of ctx and ensure the
timeout context is used for the GetHealth validation to avoid indefinite
blocking when prov.GetHealth hangs.
Source: Coding guidelines
| # Support FROM override | ||
| ARG BUILD_IMAGE=docker.io/golang:1.25.11@sha256:dd7d32e19b28621cd982082397fc0510d396805b717d5e77466aa2dd692340de | ||
| ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:9ecc53c269509f63c69a266168e4a687c7eb8c0cfd753bd8bfcaa4f58a90876f | ||
| ARG BASE_IMAGE=gcr.io/distroless/base-debian13:nonroot@sha256:a557d784ac275c287d2bdf3172f47bece8d2a0ef3c0fdefb712e95084a04a562 |
There was a problem hiding this comment.
Use a supported runtime base image.
Line 3 moves the shipped manager image to gcr.io/distroless/..., but this repo’s container policy requires a UBI minimal base or a distroless image from catalog.redhat.com. That makes the new runtime image non-compliant even though it is digest-pinned. As per coding guidelines, Base image: UBI minimal or distroless from catalog.redhat.com.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` at line 3, The ARG BASE_IMAGE currently points to
gcr.io/distroless/base-debian13 which violates the repo policy; update the
Dockerfile to use a compliant base (either a UBI minimal image or a distroless
image hosted on catalog.redhat.com) by replacing the value of ARG BASE_IMAGE
with the appropriate catalog.redhat.com image (retain digest pinning) and keep
the ARG name BASE_IMAGE unchanged so downstream references (ARG BASE_IMAGE) and
subsequent FROM instructions continue to work.
Source: Coding guidelines
Comment on lines
+76
to
+84
| out, err := os.Create(path) | ||
| if err != nil { | ||
| die("open %s: %v", path, err) | ||
| } | ||
| defer out.Close() | ||
|
|
||
| if err := format.Node(out, fset, file); err != nil { | ||
| die("format: %v", err) | ||
| } |
There was a problem hiding this comment.
Replace in-place file overwrite with atomic write.
The patcher opens the target file for writing (line 76) before format.Node() completes (line 82). If formatting fails, the original file is truncated or partially overwritten, causing data loss. This is especially problematic during development when the AST manipulation logic might have bugs.
💾 Proposed fix using temp file and atomic rename
- out, err := os.Create(path)
+ tmpPath := path + ".tmp"
+ out, err := os.Create(tmpPath)
if err != nil {
- die("open %s: %v", path, err)
+ die("create temp %s: %v", tmpPath, err)
}
- defer out.Close()
if err := format.Node(out, fset, file); err != nil {
+ out.Close()
+ os.Remove(tmpPath)
die("format: %v", err)
}
+
+ if err := out.Close(); err != nil {
+ os.Remove(tmpPath)
+ die("close temp: %v", err)
+ }
+
+ if err := os.Rename(tmpPath, path); err != nil {
+ die("rename %s to %s: %v", tmpPath, path, err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| out, err := os.Create(path) | |
| if err != nil { | |
| die("open %s: %v", path, err) | |
| } | |
| defer out.Close() | |
| if err := format.Node(out, fset, file); err != nil { | |
| die("format: %v", err) | |
| } | |
| tmpPath := path + ".tmp" | |
| out, err := os.Create(tmpPath) | |
| if err != nil { | |
| die("create temp %s: %v", tmpPath, err) | |
| } | |
| if err := format.Node(out, fset, file); err != nil { | |
| out.Close() | |
| os.Remove(tmpPath) | |
| die("format: %v", err) | |
| } | |
| if err := out.Close(); err != nil { | |
| os.Remove(tmpPath) | |
| die("close temp: %v", err) | |
| } | |
| if err := os.Rename(tmpPath, path); err != nil { | |
| die("rename %s to %s: %v", tmpPath, path, err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/plugin-test/patcher/main.go` around lines 76 - 84, The current logic
opens the target file with os.Create (out) before calling format.Node, which can
truncate the file if formatting fails; change this to write to a temporary file
and atomically rename it into place: use os.CreateTemp (or ioutil.TempFile) to
create a temp file in the same directory, call format.Node writing to that temp
file, close it, set file mode to match the original (use os.Stat to get
permissions if needed), then rename the temp file over the original via
os.Rename; ensure you remove the temp file on any error and keep the die(...)
error handling for failures (referencing os.Create, format.Node, defer
out.Close, die, os.Rename, os.CreateTemp/os.Stat).
Comment on lines
+201
to
+210
| manager: generate lint ironic-plugin demo-plugin ## Build manager binary and bundled provisioner plugins | ||
| go build -ldflags $(LDFLAGS) -o bin/$(OPERATOR_NAME) main.go | ||
|
|
||
| .PHONY: run | ||
| run: generate lint manifests ## Run against the configured Kubernetes cluster in ~/.kube/config | ||
| go run -ldflags $(LDFLAGS) ./main.go -namespace=$(RUN_NAMESPACE) -dev -webhook-port=0 $(RUN_FLAGS) | ||
| run: generate lint manifests ironic-plugin ## Run against the configured Kubernetes cluster in ~/.kube/config | ||
| PROVISIONER_PLUGIN_DIR=$(BIN_DIR) go run -ldflags $(LDFLAGS) ./main.go -namespace=$(RUN_NAMESPACE) -dev -provisioner=ironic -webhook-port=0 $(RUN_FLAGS) | ||
|
|
||
| .PHONY: demo | ||
| demo: generate lint manifests ## Run in demo mode | ||
| go run -ldflags $(LDFLAGS) ./main.go -namespace=$(RUN_NAMESPACE) -dev -demo-mode -webhook-port=0 $(RUN_FLAGS) | ||
| demo: generate lint manifests demo-plugin ## Run in demo mode | ||
| PROVISIONER_PLUGIN_DIR=$(BIN_DIR) go run -ldflags $(LDFLAGS) ./main.go -namespace=$(RUN_NAMESPACE) -dev -provisioner=demo -webhook-port=0 $(RUN_FLAGS) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== Makefile plugin-enabled targets ===\n'
sed -n '200,210p' Makefile
printf '\n=== Dockerfile build flags ===\n'
sed -n '21,37p' Dockerfile
printf '\n=== Tiltfile build flags ===\n'
sed -n '99,106p' TiltfileRepository: openshift/baremetal-operator
Length of output: 1983
Align Makefile local plugin-enabled manager/run/demo builds with CGO-enabled plugin loading.
Makefile’s manager, run, and demo targets build/run main.go without CGO_ENABLED=1, while the Dockerfile sets ENV CGO_ENABLED=1 and Tiltfile builds the manager with CGO_ENABLED=1 (comment: “CGO and dynamic linking are required for the manager to load provisioner plugins”). Add CGO_ENABLED=1 to the Makefile targets to prevent local/prod build divergence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 201 - 210, The Makefile's manager, run, and demo
targets build/run main.go without CGO, causing divergence from
Dockerfile/Tiltfile; update the targets (manager, run, demo) to set
CGO_ENABLED=1 when invoking go build or go run (e.g., prefix commands with
CGO_ENABLED=1) so the manager binary is built with cgo enabled for dynamic
plugin loading, ensuring consistency with the Dockerfile and Tiltfile behavior.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Dockerfile.ocp (2)
14-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun the runtime image as non-root.
The final stage has no
USERdirective, so the container defaults to root. Please set an explicit non-root UID/GID in the runtime stage before shipping plugin artifacts.As per coding guidelines, "USER non-root; never run as root".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile.ocp` around lines 14 - 24, The runtime Docker stage leaves the container running as root; update the final stage (the one using FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 and the COPY lines for baremetal-operator, get-hardware-details, make-bm-worker, make-virt-host and /plugins/*) to create or use a non-root UID/GID (e.g., 1001), chown the copied binaries/plugins and manifests to that UID:GID, and add an explicit USER <uid>:<gid> directive so the container runs non-root at runtime; ensure ownership changes apply to /plugins and all copied files before switching USER.Source: Coding guidelines
14-24: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd a container
HEALTHCHECKin the runtime stage.There is no health check in the final image. Add a lightweight probe so orchestrators can detect unhealthy instances.
As per coding guidelines, "HEALTHCHECK defined".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile.ocp` around lines 14 - 24, Add a Docker HEALTHCHECK to the final/runtime stage (after the LABEL io.openshift.release.operator=true) so orchestrators can detect unhealthy containers; configure a lightweight probe (interval, timeout, start-period, retries) that validates the running baremetal-operator binary (for example by checking the process via pgrep -f baremetal-operator or hitting the operator health endpoint if it exposes one) and returns non-zero on failure; ensure the instruction is added to Dockerfile.ocp in the final stage where files like baremetal-operator and plugins are copied.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Dockerfile.ocp`:
- Around line 14-24: The runtime Docker stage leaves the container running as
root; update the final stage (the one using FROM
registry.ci.openshift.org/ocp/5.0:base-rhel9 and the COPY lines for
baremetal-operator, get-hardware-details, make-bm-worker, make-virt-host and
/plugins/*) to create or use a non-root UID/GID (e.g., 1001), chown the copied
binaries/plugins and manifests to that UID:GID, and add an explicit USER
<uid>:<gid> directive so the container runs non-root at runtime; ensure
ownership changes apply to /plugins and all copied files before switching USER.
- Around line 14-24: Add a Docker HEALTHCHECK to the final/runtime stage (after
the LABEL io.openshift.release.operator=true) so orchestrators can detect
unhealthy containers; configure a lightweight probe (interval, timeout,
start-period, retries) that validates the running baremetal-operator binary (for
example by checking the process via pgrep -f baremetal-operator or hitting the
operator health endpoint if it exposes one) and returns non-zero on failure;
ensure the instruction is added to Dockerfile.ocp in the final stage where files
like baremetal-operator and plugins are copied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c2971134-5c35-45aa-b368-5bca4a343110
📒 Files selected for processing (1)
Dockerfile.ocp
Downstream was missing the upstream .golangci.yaml which configures golangci-lint to suppress deprecated API warnings (SA1019), De Morgan's law suggestions (QF1001), and embedded field selector hints (QF1008). Without it, the default linter config flagged 29 issues that upstream intentionally suppresses. Also fix the 4 real lint issues caught by the upstream config: - gci: import grouping in action_result.go - govet: variable shadowing in register_test.go (2 instances) - nakedret: naked return in host_state_machine.go ensureRegistered - unused: restore nolint directive on deleteComplete.actionComplete Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test/ module imports libvirt.org/go/libvirt which requires libvirt-dev C headers for CGO compilation. These are not available in the downstream CI environment. Downstream does not run upstream e2e tests so linting this module is not needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@honza: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests