Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 7 additions & 4 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,18 @@ func (cmd *BuildCmd) build(
ctx context.Context,
workspaceClient client.WorkspaceClient,
) error {
err := workspaceClient.Lock(ctx)
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
emitJSON := mode == output.ModeJSON

if err = workspaceClient.Lock(ctx); err != nil {
return err
}
defer workspaceClient.Unlock()

err = clientimplementation.StartWait(ctx, workspaceClient, true)
if err != nil {
if err = clientimplementation.StartWait(ctx, workspaceClient, true); err != nil {
return err
}

Expand All @@ -226,7 +230,6 @@ func (cmd *BuildCmd) build(
log.Debugf("done building devcontainer")
log.Infof("cleaning up temporary workspace")
}()
emitJSON := output.ResolveMode(cmd.ResultFormat) == output.ModeJSON

result, err := clientimplementation.BuildAgentClient(
ctx,
Expand Down
20 changes: 8 additions & 12 deletions cmd/context/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ import (

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/config"
"github.com/devsy-org/devsy/pkg/output"
"github.com/devsy-org/devsy/pkg/table"
"github.com/spf13/cobra"
)

// ListCmd holds the list cmd flags.
type ListCmd struct {
*flags.GlobalFlags

Output string
}

// NewListCmd creates a new command.
Expand All @@ -34,8 +33,6 @@ func NewListCmd(flags *flags.GlobalFlags) *cobra.Command {
},
}

listCmd.Flags().
StringVar(&cmd.Output, "output", "plain", "The output format to use. Can be json or plain")
return listCmd
}

Expand All @@ -52,8 +49,12 @@ func (cmd *ListCmd) Run(ctx context.Context) error {
return err
}

switch cmd.Output {
case "plain":
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
switch mode {
case output.ModePlain:
tableEntries := [][]string{}
for contextName := range devsyConfig.Contexts {
tableEntries = append(tableEntries, []string{
Expand All @@ -69,7 +70,7 @@ func (cmd *ListCmd) Run(ctx context.Context) error {
"Name",
"Default",
}, tableEntries)
case "json":
case output.ModeJSON:
ides := []ContextWithDefault{}
for contextName := range devsyConfig.Contexts {
ides = append(ides, ContextWithDefault{
Expand All @@ -83,11 +84,6 @@ func (cmd *ListCmd) Run(ctx context.Context) error {
return err
}
fmt.Print(string(out))
default:
return fmt.Errorf(
"unexpected output format, choose either json or plain. Got %s",
cmd.Output,
)
}

return nil
Expand Down
20 changes: 8 additions & 12 deletions cmd/context/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ import (

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/config"
"github.com/devsy-org/devsy/pkg/output"
"github.com/devsy-org/devsy/pkg/table"
"github.com/spf13/cobra"
)

// OptionsCmd holds the options cmd flags.
type OptionsCmd struct {
*flags.GlobalFlags

Output string
}

// NewOptionsCmd creates a new command.
Expand All @@ -32,8 +31,6 @@ func NewOptionsCmd(flags *flags.GlobalFlags) *cobra.Command {
},
}

optionsCmd.Flags().
StringVar(&cmd.Output, "output", "plain", "The output format to use. Can be json or plain")
return optionsCmd
}

Expand All @@ -55,8 +52,12 @@ func (cmd *OptionsCmd) Run(ctx context.Context, args []string) error {
entryOptions = map[string]config.OptionValue{}
}

switch cmd.Output {
case "plain":
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
switch mode {
case output.ModePlain:
tableEntries := [][]string{}
for _, entry := range config.ContextOptions {
value := entryOptions[entry.Name].Value
Expand All @@ -78,7 +79,7 @@ func (cmd *OptionsCmd) Run(ctx context.Context, args []string) error {
"Default",
"Value",
}, tableEntries)
case "json":
case output.ModeJSON:
options := map[string]optionWithValue{}
for _, entry := range config.ContextOptions {
options[entry.Name] = optionWithValue{
Expand All @@ -92,11 +93,6 @@ func (cmd *OptionsCmd) Run(ctx context.Context, args []string) error {
return err
}
fmt.Print(string(out))
default:
return fmt.Errorf(
"unexpected output format, choose either json or plain. Got %s",
cmd.Output,
)
}

return nil
Expand Down
16 changes: 14 additions & 2 deletions cmd/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ func (cmd *ExecCmd) Run(ctx context.Context, args []string) error {
return err
}

if _, err := output.ResolveMode(cmd.ResultFormat); err != nil {
return err
}

if cmd.ContainerID != "" {
return cmd.runWithContainerID(ctx, args)
}
Expand Down Expand Up @@ -173,7 +177,11 @@ func (cmd *ExecCmd) Run(ctx context.Context, args []string) error {
probedEnv := probeContainerEnv(ctx, target, userEnvProbe)
envMap := buildExecEnv(result, cmd.RemoteEnv, probedEnv)

emitJSON := output.ResolveMode(cmd.ResultFormat) == output.ModeJSON
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
emitJSON := mode == output.ModeJSON
Comment on lines +180 to +184

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Resolve output mode before Docker/container probing.

Line 176 and Line 235 validate --result-format only after Docker inspect and env probing paths run. Invalid format should fail fast before those external calls to avoid unnecessary work and side effects.

Proposed fix
 func (cmd *ExecCmd) Run(ctx context.Context, args []string) error {
+	mode, err := output.ResolveMode(cmd.ResultFormat)
+	if err != nil {
+		return err
+	}
+	emitJSON := mode == output.ModeJSON
+
@@
-	mode, err := output.ResolveMode(cmd.ResultFormat)
-	if err != nil {
-		return err
-	}
-	emitJSON := mode == output.ModeJSON
-
 	err = cmd.execInContainer(ctx, execOpts{
@@
 func (cmd *ExecCmd) runWithContainerID(ctx context.Context, args []string) error {
+	mode, err := output.ResolveMode(cmd.ResultFormat)
+	if err != nil {
+		return err
+	}
+	emitJSON := mode == output.ModeJSON
+
 	dockerCommand := defaultDockerCommand
@@
-	mode, err := output.ResolveMode(cmd.ResultFormat)
-	if err != nil {
-		return err
-	}
-	emitJSON := mode == output.ModeJSON
-
 	err = cmd.execInContainer(ctx, execOpts{

Also applies to: 235-239

🤖 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/exec.go` around lines 176 - 180, Move validation of the result format so
it runs before any Docker/container probing: call
output.ResolveMode(cmd.ResultFormat) at the start of the command execution
(before any docker inspect or environment probing logic), return the error
immediately if ResolveMode fails, and then derive emitJSON from the validated
mode; apply the same change for the other occurrence around the probe code (the
block currently around lines referencing emitJSON and the later ResolveMode) so
invalid --result-format fails fast and avoids running external probes.


err = cmd.execInContainer(ctx, execOpts{
target: target,
Expand Down Expand Up @@ -228,7 +236,11 @@ func (cmd *ExecCmd) runWithContainerID(ctx context.Context, args []string) error

workdir := containerDetails.Config.WorkingDir

emitJSON := output.ResolveMode(cmd.ResultFormat) == output.ModeJSON
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
emitJSON := mode == output.ModeJSON

err = cmd.execInContainer(ctx, execOpts{
target: target,
Expand Down
16 changes: 8 additions & 8 deletions cmd/features/generatedocs.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/devcontainer/config"
"github.com/devsy-org/devsy/pkg/output"
"github.com/spf13/cobra"
)

Expand All @@ -18,7 +19,6 @@ type GenerateDocsCmd struct {
ProjectFolder string
OutputFolder string
Namespace string
Output string
}

func NewGenerateDocsCmd(globalFlags *flags.GlobalFlags) *cobra.Command {
Expand All @@ -44,16 +44,14 @@ documentation for each feature based on its devcontainer-feature.json.`,
generateDocsCmd.Flags().StringVar(
&cmd.Namespace, "namespace", "", "Registry namespace for linking (e.g. ghcr.io/myorg/features)",
)
generateDocsCmd.Flags().StringVar(
&cmd.Output, "output", "text", "Output format (text or json)",
)
_ = generateDocsCmd.MarkFlagRequired("project-folder")

return generateDocsCmd
}

func (cmd *GenerateDocsCmd) Run() error {
if err := validateOutputFormat(cmd.Output); err != nil {
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}

Expand All @@ -72,11 +70,13 @@ func (cmd *GenerateDocsCmd) Run() error {
return err
}

if cmd.Output == outputJSON {
switch mode {
case output.ModeJSON:
return cmd.writeJSON(features)
case output.ModePlain:
return cmd.writeDocs(features, outputFolder)
}

return cmd.writeDocs(features, outputFolder)
return nil
}

func (cmd *GenerateDocsCmd) resolveOutputFolder(
Expand Down
8 changes: 5 additions & 3 deletions cmd/features/generatedocs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"path/filepath"
"testing"

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/output"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -40,7 +42,7 @@ func TestGenerateDocsCmd_Run(t *testing.T) {
ProjectFolder: projectDir,
OutputFolder: outputDir,
Namespace: "ghcr.io/test/features",
Output: "text",
GlobalFlags: &flags.GlobalFlags{ResultFormat: output.ModePlain},
}

err := cmd.Run()
Expand Down Expand Up @@ -78,7 +80,7 @@ func TestGenerateDocsCmd_NoFeatures(t *testing.T) {

cmd := &GenerateDocsCmd{
ProjectFolder: projectDir,
Output: "text",
GlobalFlags: &flags.GlobalFlags{ResultFormat: output.ModePlain},
}

err := cmd.Run()
Expand Down Expand Up @@ -110,7 +112,7 @@ func TestGenerateDocsCmd_MultipleFeatures(t *testing.T) {
cmd := &GenerateDocsCmd{
ProjectFolder: projectDir,
OutputFolder: outputDir,
Output: "text",
GlobalFlags: &flags.GlobalFlags{ResultFormat: output.ModePlain},
}

err := cmd.Run()
Expand Down
18 changes: 10 additions & 8 deletions cmd/features/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/devcontainer/config"
"github.com/devsy-org/devsy/pkg/devcontainer/feature"
"github.com/devsy-org/devsy/pkg/output"
"github.com/devsy-org/devsy/pkg/table"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
Expand All @@ -17,7 +18,6 @@ import (
type InfoCmd struct {
*flags.GlobalFlags

Output string
ShowTags bool
ShowDependencies bool
}
Expand Down Expand Up @@ -55,7 +55,6 @@ to display metadata.`,
},
}

infoCmd.Flags().StringVar(&cmd.Output, "output", "text", "Output format (text or json)")
infoCmd.Flags().BoolVar(
&cmd.ShowTags, "show-tags", false, "List available tags from the registry",
)
Expand All @@ -70,10 +69,6 @@ to display metadata.`,
}

func (cmd *InfoCmd) Run(featureID string) error {
if err := validateOutputFormat(cmd.Output); err != nil {
return err
}

ref, err := name.ParseReference(featureID)
if err != nil {
return fmt.Errorf("invalid feature reference %q: %w", featureID, err)
Expand All @@ -92,10 +87,17 @@ func (cmd *InfoCmd) Run(featureID string) error {
info.Tags = tags
}

if cmd.Output == outputJSON {
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
switch mode {
case output.ModeJSON:
return writeJSON(os.Stdout, info)
case output.ModePlain:
return cmd.printText(info)
}
return cmd.printText(info)
return nil
}

func (cmd *InfoCmd) fetchInfo(
Expand Down
20 changes: 10 additions & 10 deletions cmd/features/info_manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/devsy-org/devsy/cmd/flags"
"github.com/devsy-org/devsy/pkg/devcontainer/feature"
"github.com/devsy-org/devsy/pkg/output"
"github.com/devsy-org/devsy/pkg/table"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
Expand All @@ -15,8 +16,6 @@ import (

type InfoManifestCmd struct {
*flags.GlobalFlags

Output string
}

func NewInfoManifestCmd(globalFlags *flags.GlobalFlags) *cobra.Command {
Expand All @@ -36,16 +35,10 @@ ghcr.io/devcontainers/features/go:1 and outputs the OCI image manifest.`,
},
}

manifestCmd.Flags().StringVar(&cmd.Output, "output", "json", "Output format (text or json)")

return manifestCmd
}

func (cmd *InfoManifestCmd) Run(featureID string) error {
if err := validateOutputFormat(cmd.Output); err != nil {
return err
}

ref, err := name.ParseReference(featureID)
if err != nil {
return fmt.Errorf("invalid feature reference %q: %w", featureID, err)
Expand All @@ -56,10 +49,17 @@ func (cmd *InfoManifestCmd) Run(featureID string) error {
return fmt.Errorf("fetch manifest: %w", err)
}

if cmd.Output == outputJSON {
mode, err := output.ResolveMode(cmd.ResultFormat)
if err != nil {
return err
}
switch mode {
case output.ModeJSON:
return writeJSON(os.Stdout, manifest)
case output.ModePlain:
return cmd.printText(manifest)
}
return cmd.printText(manifest)
return nil
}

func (cmd *InfoManifestCmd) printText(manifest *v1.Manifest) error {
Expand Down
Loading
Loading