From cbfb285bf7fc5c93a26b1321bf0fbd7c9c0f8c20 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 6 Aug 2026 13:48:46 +0000 Subject: [PATCH 1/2] fix(lint): resolve dupl findings Extracted a shared dialAndExecute helper (cmd/pro/provider/exec.go) for the genuinely duplicated dial+execute body in ssh.go/stop.go, parameterized by sub-resource action and env flags. The remaining three pairs are cobra command factory boilerplate (NewCheckUpdateCmd/NewHealthCmd, NewNetcheckCmd/NewStatusCmd) that already have an established repo convention of //nolint:dupl with a 'structurally similar; intentional sibling factory' reason (see NewCreateCmd/NewUpdateCmd, NewHealthCmd) -- followed the same convention rather than introducing a generic cobra-wiring abstraction across unrelated command types. Extracted a setupParentChildGraph helper for the two near-identical resolver_test.go cases, keeping them as separate, individually runnable test functions since they assert distinct precedence scenarios. pkg/ts/workspace_server.go's pair is intentionally left as-is: it's the exact handler pair refactored in #890 (Director -> Rewrite migration), not yet merged. Deferred to a follow-up to avoid a conflicting refactor. --- cmd/pro/check_update.go | 2 + cmd/pro/daemon/netcheck.go | 2 + cmd/pro/daemon/status.go | 2 + cmd/pro/provider/exec.go | 53 +++++++++++++++++++++++++++ cmd/pro/provider/ssh.go | 39 ++------------------ cmd/pro/provider/stop.go | 39 ++------------------ pkg/options/resolver/resolver_test.go | 18 ++++----- 7 files changed, 74 insertions(+), 81 deletions(-) create mode 100644 cmd/pro/provider/exec.go diff --git a/cmd/pro/check_update.go b/cmd/pro/check_update.go index b1c01e57f..290ebb63d 100644 --- a/cmd/pro/check_update.go +++ b/cmd/pro/check_update.go @@ -25,6 +25,8 @@ type CheckUpdateCmd struct { } // NewCheckUpdateCmd creates a new command. +// +//nolint:dupl // structurally similar to NewHealthCmd; intentional sibling factory func NewCheckUpdateCmd(globalFlags *flags.GlobalFlags) *cobra.Command { cmd := &CheckUpdateCmd{ GlobalFlags: globalFlags, diff --git a/cmd/pro/daemon/netcheck.go b/cmd/pro/daemon/netcheck.go index ee9e39dd6..ea593fb07 100644 --- a/cmd/pro/daemon/netcheck.go +++ b/cmd/pro/daemon/netcheck.go @@ -27,6 +27,8 @@ type NetcheckCmd struct { } // NewNetcheckCmd creates a new command. +// +//nolint:dupl // structurally similar to NewStatusCmd; intentional sibling factory func NewNetcheckCmd(flags *proflags.GlobalFlags) *cobra.Command { cmd := &NetcheckCmd{ GlobalFlags: flags, diff --git a/cmd/pro/daemon/status.go b/cmd/pro/daemon/status.go index 84be6a2f5..c23f272f4 100644 --- a/cmd/pro/daemon/status.go +++ b/cmd/pro/daemon/status.go @@ -24,6 +24,8 @@ type StatusCmd struct { } // NewStatusCmd creates a new command. +// +//nolint:dupl // structurally similar to NewNetcheckCmd; intentional sibling factory func NewStatusCmd(flags *proflags.GlobalFlags) *cobra.Command { cmd := &StatusCmd{ GlobalFlags: flags, diff --git a/cmd/pro/provider/exec.go b/cmd/pro/provider/exec.go new file mode 100644 index 000000000..91c2119bf --- /dev/null +++ b/cmd/pro/provider/exec.go @@ -0,0 +1,53 @@ +package provider + +import ( + "context" + "fmt" + "io" + "net/url" + + "github.com/devsy-org/devsy/pkg/platform" + "github.com/devsy-org/devsy/pkg/platform/client" + "github.com/devsy-org/devsy/pkg/platform/remotecommand" +) + +// dialAndExecute finds the current workspace, dials the given sub-resource +// action on it, and streams the resulting connection through stdin/stdout/stderr. +func dialAndExecute( + ctx context.Context, + configPath string, + action string, + envFlags url.Values, + stdin io.Reader, + stdout io.Writer, + stderr io.Writer, +) error { + baseClient, err := client.InitClientFromPath(ctx, configPath) + if err != nil { + return err + } + + info, err := platform.GetWorkspaceInfoFromEnv() + if err != nil { + return err + } + opts := platform.FindInstanceOptions{UID: info.UID, ProjectName: info.ProjectName} + workspace, err := platform.FindInstance(ctx, baseClient, opts) + if err != nil { + return err + } else if workspace == nil { + return fmt.Errorf("couldn't find workspace") + } + + conn, err := platform.DialInstance(baseClient, workspace, action, envFlags) + if err != nil { + return err + } + + _, err = remotecommand.ExecuteConn(ctx, conn, stdin, stdout, stderr) + if err != nil { + return fmt.Errorf("error executing: %w", err) + } + + return nil +} diff --git a/cmd/pro/provider/ssh.go b/cmd/pro/provider/ssh.go index 976789751..37f220f28 100644 --- a/cmd/pro/provider/ssh.go +++ b/cmd/pro/provider/ssh.go @@ -2,15 +2,12 @@ package provider import ( "context" - "fmt" "io" "os" "github.com/devsy-org/devsy/cmd/pro/flags" "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/platform" - "github.com/devsy-org/devsy/pkg/platform/client" - "github.com/devsy-org/devsy/pkg/platform/remotecommand" "github.com/spf13/cobra" ) @@ -43,43 +40,13 @@ func (cmd *SshCmd) Run( stdout io.Writer, stderr io.Writer, ) error { - baseClient, err := client.InitClientFromPath(ctx, cmd.Config) - if err != nil { - return err - } - - info, err := platform.GetWorkspaceInfoFromEnv() - if err != nil { - return err - } - opts := platform.FindInstanceOptions{UID: info.UID, ProjectName: info.ProjectName} - workspace, err := platform.FindInstance(ctx, baseClient, opts) - if err != nil { - return err - } else if workspace == nil { - return fmt.Errorf("couldn't find workspace") - } - - conn, err := platform.DialInstance( - baseClient, - workspace, + return dialAndExecute( + ctx, + cmd.Config, "ssh", platform.OptionsFromEnv(config.EnvFlagsSSH), - ) - if err != nil { - return err - } - - _, err = remotecommand.ExecuteConn( - ctx, - conn, stdin, stdout, stderr, ) - if err != nil { - return fmt.Errorf("error executing: %w", err) - } - - return nil } diff --git a/cmd/pro/provider/stop.go b/cmd/pro/provider/stop.go index a811d49b5..9786a8781 100644 --- a/cmd/pro/provider/stop.go +++ b/cmd/pro/provider/stop.go @@ -2,15 +2,12 @@ package provider import ( "context" - "fmt" "io" "os" storagev1 "github.com/devsy-org/api/pkg/apis/storage/v1" "github.com/devsy-org/devsy/cmd/pro/flags" "github.com/devsy-org/devsy/pkg/platform" - "github.com/devsy-org/devsy/pkg/platform/client" - "github.com/devsy-org/devsy/pkg/platform/remotecommand" "github.com/spf13/cobra" ) @@ -43,43 +40,13 @@ func (cmd *StopCmd) Run( stdout io.Writer, stderr io.Writer, ) error { - baseClient, err := client.InitClientFromPath(ctx, cmd.Config) - if err != nil { - return err - } - - info, err := platform.GetWorkspaceInfoFromEnv() - if err != nil { - return err - } - opts := platform.FindInstanceOptions{UID: info.UID, ProjectName: info.ProjectName} - workspace, err := platform.FindInstance(ctx, baseClient, opts) - if err != nil { - return err - } else if workspace == nil { - return fmt.Errorf("couldn't find workspace") - } - - conn, err := platform.DialInstance( - baseClient, - workspace, + return dialAndExecute( + ctx, + cmd.Config, "stop", platform.OptionsFromEnv(storagev1.DevsyFlagsStop), - ) - if err != nil { - return err - } - - _, err = remotecommand.ExecuteConn( - ctx, - conn, stdin, stdout, stderr, ) - if err != nil { - return fmt.Errorf("error executing: %w", err) - } - - return nil } diff --git a/pkg/options/resolver/resolver_test.go b/pkg/options/resolver/resolver_test.go index 122de9bf1..9299b8654 100644 --- a/pkg/options/resolver/resolver_test.go +++ b/pkg/options/resolver/resolver_test.go @@ -214,7 +214,10 @@ func (suite *ResolverTestSuite) TestResolveOptions_ExpiredCache() { suite.Equal("new_default", result["cached_option"].Value) } -func (suite *ResolverTestSuite) TestResolveOptions_PreserveChildWhenParentUnchanged() { +// setupParentChildGraph creates a resolver graph with a "parent" option that +// has a "child" option depending on it, used by tests that verify +// user-provided-value precedence during resolution. +func (suite *ResolverTestSuite) setupParentChildGraph() { suite.resolver.graph = graph.NewGraph[*types.Option]() parentOption := &types.Option{Description: "Parent", Default: "new_parent_value"} @@ -223,6 +226,10 @@ func (suite *ResolverTestSuite) TestResolveOptions_PreserveChildWhenParentUnchan suite.Require().NoError(suite.resolver.graph.AddNode("parent", parentOption)) suite.Require().NoError(suite.resolver.graph.AddNode("child", childOption)) suite.Require().NoError(suite.resolver.graph.AddEdge("parent", "child")) +} + +func (suite *ResolverTestSuite) TestResolveOptions_PreserveChildWhenParentUnchanged() { + suite.setupParentChildGraph() existingValues := map[string]config.OptionValue{ "parent": {Value: "old_parent_value", UserProvided: true}, @@ -237,14 +244,7 @@ func (suite *ResolverTestSuite) TestResolveOptions_PreserveChildWhenParentUnchan } func (suite *ResolverTestSuite) TestResolveOptions_PreserveUserProvidedChild() { - suite.resolver.graph = graph.NewGraph[*types.Option]() - - parentOption := &types.Option{Description: "Parent", Default: "new_parent_value"} - childOption := &types.Option{Description: "Child", Default: "child_default"} - - suite.Require().NoError(suite.resolver.graph.AddNode("parent", parentOption)) - suite.Require().NoError(suite.resolver.graph.AddNode("child", childOption)) - suite.Require().NoError(suite.resolver.graph.AddEdge("parent", "child")) + suite.setupParentChildGraph() existingValues := map[string]config.OptionValue{ "parent": {Value: "old_parent_value", UserProvided: true}, From e4e198fff60edbea100b5122efacd5c01d4722af Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 6 Aug 2026 14:11:51 +0000 Subject: [PATCH 2/2] fix(lint): resolve funcorder and argument-limit findings from dupl PR Two self-inflicted findings from the earlier dupl fix: - pkg/options/resolver/resolver_test.go: moved the unexported setupParentChildGraph helper after all exported ResolverTestSuite test methods (funcorder wants unexported methods last on a struct). - cmd/pro/provider/exec.go: dialAndExecute had 7 params, over the repo's revive argument-limit of 4. Bundled configPath/action/envFlags/stdin/ stdout/stderr into a dialAndExecuteParams struct (ctx stays separate), matching the existing Params convention used elsewhere in cmd/pro (loginParams, createClusterParams, waitForWorkspacePhaseParams). Updated both call sites in ssh.go and stop.go. That struct extraction reduced ssh.go/stop.go's Run bodies to a single parameterized call, which made the two files dupl-flag themselves as a whole (lines 1-51 in each) -- the same cobra-sibling-factory pattern as the check_update/health and netcheck/status pairs already suppressed elsewhere in this PR. Added matching //nolint:dupl directives (had to place them above the package clause, since dupl's clone range for these two files starts at line 1). --- cmd/pro/provider/exec.go | 25 ++++++++++++------------ cmd/pro/provider/ssh.go | 18 ++++++++--------- cmd/pro/provider/stop.go | 18 ++++++++--------- pkg/options/resolver/resolver_test.go | 28 +++++++++++++-------------- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/cmd/pro/provider/exec.go b/cmd/pro/provider/exec.go index 91c2119bf..3da7ded54 100644 --- a/cmd/pro/provider/exec.go +++ b/cmd/pro/provider/exec.go @@ -11,18 +11,19 @@ import ( "github.com/devsy-org/devsy/pkg/platform/remotecommand" ) +type dialAndExecuteParams struct { + configPath string + action string + envFlags url.Values + stdin io.Reader + stdout io.Writer + stderr io.Writer +} + // dialAndExecute finds the current workspace, dials the given sub-resource // action on it, and streams the resulting connection through stdin/stdout/stderr. -func dialAndExecute( - ctx context.Context, - configPath string, - action string, - envFlags url.Values, - stdin io.Reader, - stdout io.Writer, - stderr io.Writer, -) error { - baseClient, err := client.InitClientFromPath(ctx, configPath) +func dialAndExecute(ctx context.Context, params dialAndExecuteParams) error { + baseClient, err := client.InitClientFromPath(ctx, params.configPath) if err != nil { return err } @@ -39,12 +40,12 @@ func dialAndExecute( return fmt.Errorf("couldn't find workspace") } - conn, err := platform.DialInstance(baseClient, workspace, action, envFlags) + conn, err := platform.DialInstance(baseClient, workspace, params.action, params.envFlags) if err != nil { return err } - _, err = remotecommand.ExecuteConn(ctx, conn, stdin, stdout, stderr) + _, err = remotecommand.ExecuteConn(ctx, conn, params.stdin, params.stdout, params.stderr) if err != nil { return fmt.Errorf("error executing: %w", err) } diff --git a/cmd/pro/provider/ssh.go b/cmd/pro/provider/ssh.go index 37f220f28..371212b26 100644 --- a/cmd/pro/provider/ssh.go +++ b/cmd/pro/provider/ssh.go @@ -1,3 +1,4 @@ +//nolint:dupl // structurally similar to stop.go; intentional sibling command sharing dialAndExecute package provider import ( @@ -40,13 +41,12 @@ func (cmd *SshCmd) Run( stdout io.Writer, stderr io.Writer, ) error { - return dialAndExecute( - ctx, - cmd.Config, - "ssh", - platform.OptionsFromEnv(config.EnvFlagsSSH), - stdin, - stdout, - stderr, - ) + return dialAndExecute(ctx, dialAndExecuteParams{ + configPath: cmd.Config, + action: "ssh", + envFlags: platform.OptionsFromEnv(config.EnvFlagsSSH), + stdin: stdin, + stdout: stdout, + stderr: stderr, + }) } diff --git a/cmd/pro/provider/stop.go b/cmd/pro/provider/stop.go index 9786a8781..0f93a631a 100644 --- a/cmd/pro/provider/stop.go +++ b/cmd/pro/provider/stop.go @@ -1,3 +1,4 @@ +//nolint:dupl // structurally similar to ssh.go; intentional sibling command sharing dialAndExecute package provider import ( @@ -40,13 +41,12 @@ func (cmd *StopCmd) Run( stdout io.Writer, stderr io.Writer, ) error { - return dialAndExecute( - ctx, - cmd.Config, - "stop", - platform.OptionsFromEnv(storagev1.DevsyFlagsStop), - stdin, - stdout, - stderr, - ) + return dialAndExecute(ctx, dialAndExecuteParams{ + configPath: cmd.Config, + action: "stop", + envFlags: platform.OptionsFromEnv(storagev1.DevsyFlagsStop), + stdin: stdin, + stdout: stdout, + stderr: stderr, + }) } diff --git a/pkg/options/resolver/resolver_test.go b/pkg/options/resolver/resolver_test.go index 9299b8654..a196766ff 100644 --- a/pkg/options/resolver/resolver_test.go +++ b/pkg/options/resolver/resolver_test.go @@ -214,20 +214,6 @@ func (suite *ResolverTestSuite) TestResolveOptions_ExpiredCache() { suite.Equal("new_default", result["cached_option"].Value) } -// setupParentChildGraph creates a resolver graph with a "parent" option that -// has a "child" option depending on it, used by tests that verify -// user-provided-value precedence during resolution. -func (suite *ResolverTestSuite) setupParentChildGraph() { - suite.resolver.graph = graph.NewGraph[*types.Option]() - - parentOption := &types.Option{Description: "Parent", Default: "new_parent_value"} - childOption := &types.Option{Description: "Child", Default: "child_default"} - - suite.Require().NoError(suite.resolver.graph.AddNode("parent", parentOption)) - suite.Require().NoError(suite.resolver.graph.AddNode("child", childOption)) - suite.Require().NoError(suite.resolver.graph.AddEdge("parent", "child")) -} - func (suite *ResolverTestSuite) TestResolveOptions_PreserveChildWhenParentUnchanged() { suite.setupParentChildGraph() @@ -321,3 +307,17 @@ func (suite *ResolverTestSuite) TestAddOptionsToGraph_MultipleCalls() { nodes := g.GetNodes() suite.Len(nodes, 2, "Multiple calls to addOptionsToGraph should not duplicate nodes.") } + +// setupParentChildGraph creates a resolver graph with a "parent" option that +// has a "child" option depending on it, used by tests that verify +// user-provided-value precedence during resolution. +func (suite *ResolverTestSuite) setupParentChildGraph() { + suite.resolver.graph = graph.NewGraph[*types.Option]() + + parentOption := &types.Option{Description: "Parent", Default: "new_parent_value"} + childOption := &types.Option{Description: "Child", Default: "child_default"} + + suite.Require().NoError(suite.resolver.graph.AddNode("parent", parentOption)) + suite.Require().NoError(suite.resolver.graph.AddNode("child", childOption)) + suite.Require().NoError(suite.resolver.graph.AddEdge("parent", "child")) +}