Skip to content
Closed
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
22 changes: 13 additions & 9 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,19 +234,23 @@ func (cmd *BuildCmd) build(
},
)
if err != nil {
_ = devcconfig.WriteErrorJSON(os.Stdout, err.Error())
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = devcconfig.WriteErrorJSON(os.Stdout, err.Error())
}
return err
}

if cmd.OutputFormat != flags.OutputFormatJSON {
return nil
}

containerID := ""
workdir := "" // uses substituted path directly; build doesn't support git subpath overrides
if result != nil {
if result.ContainerDetails != nil {
containerID = result.ContainerDetails.ID
}
if result.SubstitutionContext != nil {
workdir = result.SubstitutionContext.ContainerWorkspaceFolder
}
workdir := ""
if result != nil && result.ContainerDetails != nil {
containerID = result.ContainerDetails.ID
}
if result != nil && result.SubstitutionContext != nil {
workdir = result.SubstitutionContext.ContainerWorkspaceFolder
}
user := devcconfig.GetRemoteUser(result)
_ = devcconfig.WriteResultJSON(os.Stdout, containerID, user, workdir, nil)
Expand Down
16 changes: 12 additions & 4 deletions cmd/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,15 @@ func (cmd *ExecCmd) Run(ctx context.Context, args []string) error {
envMap: envMap,
}, args)
if err != nil {
_ = devcconfig.WriteErrorJSON(os.Stderr, err.Error())
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = devcconfig.WriteErrorJSON(os.Stdout, err.Error())
}
return err
}

_ = devcconfig.WriteResultJSON(os.Stderr, containerDetails.ID, user, workdir, nil)
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = devcconfig.WriteResultJSON(os.Stdout, containerDetails.ID, user, workdir, nil)
}
Comment on lines +178 to +186

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 | 🟠 Major | 🏗️ Heavy lift

JSON envelope is interleaved with container command output on stdout.

execInContainer wires os.Stdout as the container process's stdout (line 414), so any command output is already flushed there. The WriteResultJSON call then appends the JSON object to the same stream. A CI/CD consumer expecting clean JSON on stdout will receive something like:

<command stdout>
{"containerID":"...","user":"...","workdir":"..."}

making the envelope unparseable by a standard JSON decoder. Common resolutions: write the envelope to a dedicated file/fd, move it to stderr, or redirect the container process's stdout to a buffer that gets embedded in the envelope.

🤖 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 178 - 186, The JSON envelope is being written to
os.Stdout while execInContainer already wires the container's stdout to
os.Stdout (see execInContainer), so command output and
devcconfig.WriteResultJSON/WriteErrorJSON are interleaved; change the JSON
writes to a separate stream (preferably os.Stderr) when cmd.OutputFormat ==
flags.OutputFormatJSON so the envelope is not mixed with the container stdout
(update the WriteResultJSON and WriteErrorJSON calls in the code paths around
containerDetails.ID, user, workdir and the error handling to use os.Stderr
instead of os.Stdout), or alternatively capture the container stdout to a buffer
in execInContainer and pass that into WriteResultJSON if you need stdout
embedded in the JSON.

return nil
}

Expand Down Expand Up @@ -224,11 +228,15 @@ func (cmd *ExecCmd) runWithContainerID(ctx context.Context, args []string) error
envMap: envMap,
}, args)
if err != nil {
_ = devcconfig.WriteErrorJSON(os.Stderr, err.Error())
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = devcconfig.WriteErrorJSON(os.Stdout, err.Error())
}
return err
}

_ = devcconfig.WriteResultJSON(os.Stderr, containerDetails.ID, "", workdir, nil)
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = devcconfig.WriteResultJSON(os.Stdout, containerDetails.ID, "", workdir, nil)
}
return nil
}

Expand Down
20 changes: 20 additions & 0 deletions cmd/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,23 @@ func TestExecCmd_SkipPostCreateFlagParsesValue(t *testing.T) {
require.NoError(t, err)
assert.True(t, val)
}

func TestExecCmd_NoEnvelopeWithoutOutputFormat(t *testing.T) {
cmd := &ExecCmd{
GlobalFlags: &flags.GlobalFlags{OutputFormat: ""},
ContainerID: "nonexistent-container-id-99999",
}
err := cmd.runWithContainerID(t.Context(), []string{"echo", "hello"})
require.Error(t, err)
assert.Contains(t, err.Error(), "nonexistent-container-id-99999")
}

func TestExecCmd_EnvelopeWithOutputFormatJSON(t *testing.T) {
cmd := &ExecCmd{
GlobalFlags: &flags.GlobalFlags{OutputFormat: formatJSON},
ContainerID: "nonexistent-container-id-88888",
}
err := cmd.runWithContainerID(t.Context(), []string{"echo", "hello"})
require.Error(t, err)
assert.Contains(t, err.Error(), "nonexistent-container-id-88888")
}
16 changes: 16 additions & 0 deletions cmd/flag_aliases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,19 @@ func TestRemoveExistingContainerAlias_IsHidden(t *testing.T) {
require.NotNil(t, f)
assert.True(t, f.Hidden, flagRemoveExistingContainer+" alias should be hidden")
}

func TestGlobalFlags_OutputFormat(t *testing.T) {
rootCmd := BuildRoot()
rootCmd.SetArgs([]string{"--output-format", formatJSON, "version"})
err := rootCmd.Execute()
require.NoError(t, err)
assert.Equal(t, formatJSON, globalFlags.OutputFormat)
}

func TestGlobalFlags_OutputFormatDefault(t *testing.T) {
rootCmd := BuildRoot()
rootCmd.SetArgs([]string{"version"})
err := rootCmd.Execute()
require.NoError(t, err)
assert.Equal(t, "", globalFlags.OutputFormat)
}
17 changes: 13 additions & 4 deletions cmd/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
flag "github.com/spf13/pflag"
)

const OutputFormatJSON = "json"

type GlobalFlags struct {
Context string
Provider string
Expand All @@ -14,10 +16,11 @@ type GlobalFlags struct {
UID string
Owner platform.OwnerFilter

LogOutput string
Verbosity int
Quiet bool
Debug bool
LogOutput string
OutputFormat string
Verbosity int
Quiet bool
Debug bool
}

// SetGlobalFlags applies the global flags.
Expand Down Expand Up @@ -59,6 +62,12 @@ func SetGlobalFlags(flags *flag.FlagSet) *GlobalFlags {
"Suppress all log output except fatal errors",
)
flags.BoolVar(&globalFlags.Debug, "debug", false, "Enable debug logging (equivalent to -vv)")
flags.StringVar(
&globalFlags.OutputFormat,
"output-format",
"",
"Machine-readable output format for command results (json)",
)

flags.Var(&globalFlags.Owner, "owner", "Show pro workspaces for owner")
_ = flags.MarkHidden("owner")
Expand Down
28 changes: 18 additions & 10 deletions cmd/up/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ func (cmd *UpCmd) Run(

wctx, err := cmd.executeDevsyUp(ctx, devsyConfig, client)
if err != nil {
_ = config2.WriteErrorJSON(os.Stdout, err.Error())
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = config2.WriteErrorJSON(os.Stdout, err.Error())
}
return err
}
if wctx == nil {
Expand All @@ -80,24 +82,30 @@ func (cmd *UpCmd) Run(
}

if err := cmd.configureWorkspace(devsyConfig, client, wctx); err != nil {
_ = config2.WriteErrorJSON(os.Stdout, err.Error())
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = config2.WriteErrorJSON(os.Stdout, err.Error())
}
return err
}

if err := cmd.openIDE(ctx, devsyConfig, client, wctx); err != nil {
_ = config2.WriteErrorJSON(os.Stdout, err.Error())
if cmd.OutputFormat == flags.OutputFormatJSON {
_ = config2.WriteErrorJSON(os.Stdout, err.Error())
}
return err
}

containerID := ""
var warnings []string
if wctx.result != nil {
if wctx.result.ContainerDetails != nil {
containerID = wctx.result.ContainerDetails.ID
if cmd.OutputFormat == flags.OutputFormatJSON {
containerID := ""
var warnings []string
if wctx.result != nil {
if wctx.result.ContainerDetails != nil {
containerID = wctx.result.ContainerDetails.ID
}
warnings = wctx.result.HostWarnings
}
warnings = wctx.result.HostWarnings
_ = config2.WriteResultJSON(os.Stdout, containerID, wctx.user, wctx.workdir, warnings)
}
Comment on lines +107 to 108

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

WriteResultJSON failure silently returns nil.

Same silent-discard pattern as build.go line 256 — if writing to stdout fails, the command returns nil (success) while the CI/CD consumer receives no envelope. Consider propagating the error:

-	_ = config2.WriteResultJSON(os.Stdout, containerID, wctx.user, wctx.workdir, warnings)
+	return config2.WriteResultJSON(os.Stdout, containerID, wctx.user, wctx.workdir, warnings)
📝 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
_ = config2.WriteResultJSON(os.Stdout, containerID, wctx.user, wctx.workdir, warnings)
}
return config2.WriteResultJSON(os.Stdout, containerID, wctx.user, wctx.workdir, warnings)
}
🤖 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/up/up.go` around lines 107 - 108, The call to
config2.WriteResultJSON(containerID, wctx.user, wctx.workdir, warnings)
currently discards any error (_ = ...), so failures writing the envelope to
stdout are ignored; update the code to capture the returned error from
config2.WriteResultJSON, and if non-nil return or propagate that error from the
enclosing function (or log+return) so the command fails instead of silently
succeeding when stdout write fails; reference the call site where containerID,
wctx.user, wctx.workdir and warnings are passed to config2.WriteResultJSON and
ensure the enclosing function's signature/flow returns the error.

_ = config2.WriteResultJSON(os.Stdout, containerID, wctx.user, wctx.workdir, warnings)
return nil
}

Expand Down
8 changes: 8 additions & 0 deletions cmd/up/up_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,11 @@ func TestUpCmd_RemoteUserFlagParsesValue(t *testing.T) {
flag := upCmd.Flags().Lookup("remote-user")
assert.Equal(t, "vscode", flag.Value.String())
}

func TestUpCmd_OutputFormatGatesEnvelope(t *testing.T) {
cmd := &UpCmd{GlobalFlags: &flags.GlobalFlags{OutputFormat: ""}}
assert.Empty(t, cmd.OutputFormat, "default should be empty (no envelope)")

cmd2 := &UpCmd{GlobalFlags: &flags.GlobalFlags{OutputFormat: "json"}}
assert.Equal(t, "json", cmd2.OutputFormat)
}
Loading