Skip to content

feat: populate Config attribute in hook params #924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
gustavogama-cll wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: populate Config attribute in hook params #924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion engine/cld/changeset/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ type Configurations struct {
type internalChangeSet interface {
noop() // unexported function to prevent arbitrary structs from implementing ChangeSet.
Apply(env fdeployment.Environment) (fdeployment.ChangesetOutput, error)
applyWithInput(env fdeployment.Environment, inputStr string) (fdeployment.ChangesetOutput, error)
Configurations() (Configurations, error)
applyWithInput(env fdeployment.Environment, inputStr string) (fdeployment.ChangesetOutput, error)
configuration() (any, error)
}

type ChangeSet internalChangeSet
Expand Down Expand Up @@ -335,6 +336,10 @@ func (ccs ChangeSetImpl[C]) Configurations() (Configurations, error) {
}, nil
}

func (ccs ChangeSetImpl[C]) configuration() (any, error) {
return ccs.configProvider()
}

// WithPreHooks appends pre-hooks to this changeset. Multiple calls are additive.
func (ccs ChangeSetImpl[C]) WithPreHooks(hooks ...PreHook) ConfiguredChangeSet {
ccs.preHooks = append(slices.Clone(ccs.preHooks), hooks...)
Expand Down
35 changes: 35 additions & 0 deletions engine/cld/changeset/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,41 @@ func TestWithJSON_UseNumberForAnyPayload(t *testing.T) {
require.NoError(t, err)
}

func TestChangeSetImpl_configuration(t *testing.T) { //nolint:paralleltest
configuredChangeset := Configure(MyChangeSet)

t.Run("With", func(t *testing.T) { //nolint:paralleltest
got, err := configuredChangeset.With("config").configuration()
require.NoError(t, err)
require.Equal(t, "config", got)
})

t.Run("WithConfigFrom", func(t *testing.T) { //nolint:paralleltest
got, err := configuredChangeset.WithConfigFrom(func() (string, error) {
return "config", nil
}).configuration()
require.NoError(t, err)
require.Equal(t, "config", got)
})

t.Run("WithEnvInput", func(t *testing.T) { //nolint:paralleltest
t.Setenv("DURABLE_PIPELINE_INPUT", `{"payload":"config"}`)
got, err := configuredChangeset.WithEnvInput().configuration()
require.NoError(t, err)
require.Equal(t, "config", got)
})

t.Run("WithConfigResolver", func(t *testing.T) { //nolint:paralleltest
t.Setenv("DURABLE_PIPELINE_INPUT", `{"payload":"config"}`)
resolver := func(input string) (string, error) {
return "resolved " + input, nil
}
got, err := configuredChangeset.WithConfigResolver(resolver).configuration()
require.NoError(t, err)
require.Equal(t, "resolved config", got)
})
}

func TestConfigurations_ConfigResolverInfo(t *testing.T) {
t.Parallel()

Expand Down
1 change: 1 addition & 0 deletions engine/cld/changeset/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type PostProposalHookParams struct {
ChangesetKey string
Proposal *mcms.TimelockProposal
Input any
Config any
Reports []MCMSTimelockExecuteReport
}

Expand Down
3 changes: 2 additions & 1 deletion engine/cld/changeset/mcms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (*EVMForkContext) ChainFamily() string {
// 1. Per-changeset post-proposal-hooks
// 2. Global post-proposal-hooks
func (r *ChangesetsRegistry) RunProposalHooks(
key string, e fdeployment.Environment, proposal *mcms.TimelockProposal, input any,
key string, e fdeployment.Environment, proposal *mcms.TimelockProposal, input, config any,
reports []MCMSTimelockExecuteReport, forkCtx ForkContext,
) error {
applySnapshot, err := r.getApplySnapshot(key)
Expand All @@ -106,6 +106,7 @@ func (r *ChangesetsRegistry) RunProposalHooks(
ChangesetKey: key,
Proposal: proposal,
Input: input,
Config: config,
Reports: reports,
}

Expand Down
6 changes: 4 additions & 2 deletions engine/cld/changeset/mcms_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func Test_RunProposalHooks(t *testing.T) {
execLogs := []string{}
registry := tt.setup(&execLogs)

err := registry.RunProposalHooks(tt.key, hookTestEnv(t), &mcms.TimelockProposal{}, nil, nil, nil)
err := registry.RunProposalHooks(tt.key, hookTestEnv(t), &mcms.TimelockProposal{}, nil, nil, nil, nil)

if tt.wantErr == "" {
require.NoError(t, err)
Expand All @@ -165,6 +165,7 @@ func Test_RunProposalHooks_HookReceivesCorrectParams(t *testing.T) {

proposal := &mcms.TimelockProposal{}
input := "test-input"
config := "test-config"
reports := []MCMSTimelockExecuteReport{{Type: MCMSTimelockExecuteReportType}}

var receivedParams PostProposalHookParams
Expand All @@ -181,14 +182,15 @@ func Test_RunProposalHooks_HookReceivesCorrectParams(t *testing.T) {
}},
}

err := r.RunProposalHooks("test-cs", hookTestEnv(t), proposal, input, reports, nil)
err := r.RunProposalHooks("test-cs", hookTestEnv(t), proposal, input, config, reports, nil)
require.NoError(t, err)

expectedParams := PostProposalHookParams{
Env: ProposalHookEnv{Name: "test-env"},
ChangesetKey: "test-cs",
Proposal: proposal,
Input: input,
Config: config,
Reports: reports,
}
require.Empty(t, cmp.Diff(expectedParams, receivedParams,
Expand Down
4 changes: 4 additions & 0 deletions engine/cld/changeset/postprocess.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ func (ccs PostProcessingChangeSetImpl[C]) WithPostProposalHooks(hooks ...PostPro
return ccs
}

func (ccs PostProcessingChangeSetImpl[C]) configuration() (any, error) {
return ccs.changeset.configuration()
}

func (ccs PostProcessingChangeSetImpl[C]) getPreHooks() []PreHook { return ccs.preHooks }
func (ccs PostProcessingChangeSetImpl[C]) getPostHooks() []PostHook { return ccs.postHooks }
func (ccs PostProcessingChangeSetImpl[C]) getPostProposalHooks() []PostProposalHook {
Expand Down
13 changes: 13 additions & 0 deletions engine/cld/changeset/postprocess_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,16 @@ func TestChangesets_PostProcess(t *testing.T) {
require.NoError(t, err)
assert.Nil(t, configs.InputChainOverrides)
}

func TestChangesets_PostProcess_configuration(t *testing.T) {
t.Parallel()

noopPostProcessor := func(_ fdeployment.Environment, o fdeployment.ChangesetOutput) (fdeployment.ChangesetOutput, error) {
return o, nil
}

got, err := Configure(MyChangeSet).With("config").ThenWith(noopPostProcessor).configuration()

require.NoError(t, err)
require.Equal(t, "config", got)
}
17 changes: 17 additions & 0 deletions engine/cld/changeset/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ func (r *ChangesetsRegistry) Apply(
return fdeployment.ChangesetOutput{}, err
}

changesetConfig, err := applySnapshot.registryEntry.changeset.configuration()
if err != nil {
return fdeployment.ChangesetOutput{}, fmt.Errorf("failed to get changeset configuration: %w", err)
}
Comment on lines +220 to +223
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply() now computes changesetConfig via changeset.configuration() before running hooks, but this ignores the WithInput(...) ApplyOption. When cfg.inputStr is set, the changeset execution uses applyWithInput(e, cfg.inputStr) (potentially using configProviderWithInput), while hooks will receive a config derived from the default provider (often DURABLE_PIPELINE_INPUT), so hook params can diverge from the actual config used for the apply. Consider deriving the hook Config from the same source as applyWithInput (e.g., add a configurationWithInput(inputStr string) method to internalChangeSet, or pass cfg.inputStr into configuration() and have ChangeSetImpl use configProviderWithInput when inputStr is non-empty).

Copilot uses AI. Check for mistakes.

hookEnv := HookEnv{
Name: e.Name,
Logger: e.Logger,
Expand All @@ -225,6 +230,7 @@ func (r *ChangesetsRegistry) Apply(
preParams := PreHookParams{
Env: hookEnv,
ChangesetKey: key,
Config: changesetConfig,
}

for _, h := range applySnapshot.globalPreHooks {
Expand All @@ -248,6 +254,7 @@ func (r *ChangesetsRegistry) Apply(
postParams := PostHookParams{
Env: hookEnv,
ChangesetKey: key,
Config: changesetConfig,
Output: output,
Err: applyErr,
}
Expand Down Expand Up @@ -339,6 +346,16 @@ func (r *ChangesetsRegistry) GetChangesetOptions(key string) (ChangesetConfig, e
return entry.options, nil
}

// GetConfiguration retrieves the configuration for a changeset.
func (r *ChangesetsRegistry) GetConfiguration(key string) (any, error) {
entry, ok := r.entries[key]
if !ok {
return ChangesetConfig{}, fmt.Errorf("changeset '%s' not found", key)
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetConfiguration returns ChangesetConfig{} when the key is not found, but the method’s return type is any and callers likely treat the value as the changeset’s configuration (which can be nil). Returning a ChangesetConfig struct here is misleading and inconsistent with the API intent; return nil (or a typed zero of the configuration type if you want a non-nil sentinel) alongside the error instead.

Suggested change
return ChangesetConfig{}, fmt.Errorf("changeset '%s' not found", key)
return nil, fmt.Errorf("changeset '%s' not found", key)

Copilot uses AI. Check for mistakes.
}

return entry.changeset.configuration()
}

// GetConfigurations retrieves the configurations for a changeset.
func (r *ChangesetsRegistry) GetConfigurations(key string) (Configurations, error) {
entry, ok := r.entries[key]
Expand Down
Loading
Loading