feat: add CRE runner to environment object#877
Conversation
🦋 Changeset detectedLatest commit: 55275dc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
Adds a CRE CLI runner abstraction and threads it through environment construction so changesets/tests can invoke CRE via a pluggable runner.
Changes:
- Introduce
cre.Runnerinterface,CLIRunnerimplementation,ExitError, plus unit tests and a mock. - Add
CRERunnertodeployment.Environment, propagate it through cloning/runtime env regeneration, and expose options for CLD/test environments. - Minor mermaid renderer change to avoid
fmt.Sprintf+WriteStringpattern.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cre/runner.go | Defines the core Runner interface and CallResult. |
| pkg/cre/cli_runner.go | Implements Runner using os/exec and captures stdout/stderr/exit code. |
| pkg/cre/exit.go | Adds ExitError for non-zero process exits. |
| pkg/cre/cli_runner_test.go | Tests CLIRunner behavior (success, non-zero exit, missing binary, canceled ctx). |
| pkg/cre/exit_test.go | Tests ExitError formatting and errors.As behavior. |
| pkg/cre/mocks/mock_runner.go | Adds mockery-generated mock for cre.Runner. |
| deployment/environment.go | Adds CRERunner field + WithCRERunner option + clones the field. |
| engine/test/environment/components.go | Adds CRERunner to test environment components. |
| engine/test/environment/options.go | Adds WithCRERunner LoadOpt for test environments. |
| engine/test/environment/environment.go | Wires cmps.CRERunner into the returned test Environment. |
| engine/test/runtime/environment.go | Preserves CRERunner when regenerating env from state. |
| engine/cld/environment/options.go | Adds CLD options for CRE runner override and binary path + resolver helper. |
| engine/cld/environment/environment.go | Sets Environment.CRERunner during CLD env load. |
| engine/cld/environment/fork.go | Injects resolved runner into fork env creation. |
| engine/cld/config/env/config.go | Adds viper env bindings for cre.* config keys. |
| engine/cld/mcms/proposalanalysis/examples/ccip/renderers/mermaid/renderer.go | Switches to fmt.Fprintf(&builder, ...) for a few writes. |
| .mockery.yml | Configures mockery generation for cre.Runner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
it'd be good to hear from cldf original maintainers if a root cre package is the way to go
There was a problem hiding this comment.
Yes I agree, maybe @graham-chainlink and @jkongie you guys can give us your thoughts?
There was a problem hiding this comment.
hmm, so far everything i see in cre package is quite generic except for the string cre . In the future will there be cre golang api code that will be implemented in this package? So far it looks to me like a generic cli command runner. I dont mind the cre root level package if it eventually becomes a very cre specific package.
There was a problem hiding this comment.
Yeah eventually we'll probably have to import a Go SDK and this interface will have a bit more CRE specific terminology. Right now the goal is just to provide a single interface to CRE while we get more clarity on the go sdk and then we can refactor.
There was a problem hiding this comment.
I wonder if it should be in internal thoujgh. Do we want people to use this outside of CLDF directly?
There was a problem hiding this comment.
It would be accessed mainly through the Environment object. So it could make sense to make it internal. But seeing the implementation of the OffChain client seems the interface is still outside internal/ so not sure if we keep it that way to be consistent with both.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // CRERunner invokes the CRE CLI from changesets. Optional in environment loading, so it won't | ||
| // fail to load environment if cre binary not available; failures (e.g. binary not found) only occur | ||
| // when CRERunner.Run() is used inside changesets. May be nil in test environments that did not set it. | ||
| CRERunner cre.Runner |
There was a problem hiding this comment.
Does this means this will become a guarantee breaking change? If we update CRE runner eventually to use golang api instead of cli runner then this whole thing will either break or will it be replaced with a new field?
There was a problem hiding this comment.
Yes for now this is the only option, we don't have any clarity on the Go API or even the CLI command flags that are needed for the offchain registry yet, so we opted to give the user flexibility and have them call the commands as they required. Once we have some changesets with usages we can decide on the best API to abstract this away, but it will always be a breaking change. Although for now I'm thinking about offering a couple default operations and changesets, these could be used initially so we have more control on where the CRERunner is being called
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
looks good but there's now a minor linting issue and you probably want to add a changeset file |
|
Adds CRE CLI runner to the environment object as well as config variables.
AI Summary
This pull request introduces a new abstraction for invoking the CRE CLI tool within the deployment framework. The main changes are the addition of the
Runnerinterface (with a default implementationCLIRunner), integration of this runner into the environment loading process, and support for configuration and testing of CRE CLI invocations. These changes enable flexible and testable interaction with the CRE CLI, including error handling and output inspection.CRE CLI Integration
cre.Runnerinterface andcre.CLIRunnerimplementation for running the CRE CLI, along with supporting types (CallResult,ExitError) and comprehensive unit tests. [1] [2] [3] [4] [5]CRERunnerinto theEnvironmentstruct and environment loading logic, allowing CRE CLI invocation from changesets and supporting cloning of environments with the runner. [1] [2] [3] [4] [5] [6] [7] [8]Configuration and Options
WithCRERunner,WithCREBinaryPath), and implemented logic to resolve the runner based on overrides or binary path. [1] [2]Testing and Mocking
Runnerinterface using mockery and added tests for runner configuration and resolution logic. [1] [2]Dependency and Import Updates
crepackage and runner abstraction. [1] [2] [3] [4]Test Environment Support
These changes collectively provide a robust, configurable, and testable mechanism for interacting with the CRE CLI in deployment environments.