diff --git a/cmd/workspace/up/agent.go b/cmd/workspace/up/agent.go index 6ed71b150..8a5525974 100644 --- a/cmd/workspace/up/agent.go +++ b/cmd/workspace/up/agent.go @@ -35,7 +35,7 @@ func (cmd *UpCmd) devsyUp( // Preserve a structured error envelope the agent may have forwarded // over the tunnel before exiting non-zero (e.g. host requirements // not met), so the caller can surface the real cause. - if result != nil && result.Error != "" { + if result.Err() != nil { return result, err } return nil, err diff --git a/cmd/workspace/up/up.go b/cmd/workspace/up/up.go index 36c6c30bb..8899a7ccc 100644 --- a/cmd/workspace/up/up.go +++ b/cmd/workspace/up/up.go @@ -371,23 +371,8 @@ func (cmd *UpCmd) executeDevsyUp( client client2.BaseWorkspaceClient, ) (*workspaceContext, error) { result, err := cmd.devsyUp(ctx, devsyConfig, client) - // Prefer the structured error message forwarded from the agent over - // the generic SSH-level wrapper, so callers see the actual cause - // (e.g. host requirements not met) instead of a generic fallback. - if result != nil && result.Error != "" { - if err != nil { - return nil, fmt.Errorf("start workspace: %s: %w", result.Error, err) - } - return nil, fmt.Errorf("start workspace: %s", result.Error) - } - if err != nil { - return nil, fmt.Errorf("start workspace: %w", err) - } - if result == nil { - return nil, fmt.Errorf( - "agent exited without sending a result; the underlying error was logged " + - "to the agent's stderr above — re-run with --debug for the full trace", - ) + if err := validateUpResult(result, err); err != nil { + return nil, err } if cmd.Platform.Enabled { return nil, nil @@ -405,6 +390,36 @@ func (cmd *UpCmd) executeDevsyUp( } user := config2.GetRemoteUser(result) + workdir := cmd.resolveWorkdir(result, client) + return &workspaceContext{result: result, user: user, workdir: workdir}, nil +} + +// validateUpResult turns the (result, err) pair from devsyUp into a single +// error. It prefers the structured message the agent forwarded in the result +// over the generic transport error so callers see the actual cause; when both +// are present the transport error is wrapped for the full chain. +func validateUpResult(result *config2.Result, err error) error { + if resultErr := result.Err(); resultErr != nil { + if err != nil { + return fmt.Errorf("start workspace: %s: %w", resultErr, err) + } + return fmt.Errorf("start workspace: %w", resultErr) + } + if err != nil { + return fmt.Errorf("start workspace: %w", err) + } + if result == nil { + return config2.ErrNoAgentResult + } + return nil +} + +// resolveWorkdir determines the container workspace folder, honoring a git +// subpath and an explicit --workspace-folder override. +func (cmd *UpCmd) resolveWorkdir( + result *config2.Result, + client client2.BaseWorkspaceClient, +) string { workdir := "" if result.MergedConfig != nil && result.MergedConfig.WorkspaceFolder != "" { workdir = result.MergedConfig.WorkspaceFolder @@ -420,8 +435,7 @@ func (cmd *UpCmd) executeDevsyUp( result.SubstitutionContext.ContainerWorkspaceFolder = cmd.WorkspaceFolder workdir = cmd.WorkspaceFolder } - - return &workspaceContext{result: result, user: user, workdir: workdir}, nil + return workdir } func WithSignals(ctx context.Context) (context.Context, func()) { diff --git a/pkg/devcontainer/config/result.go b/pkg/devcontainer/config/result.go index e54f3fb4a..e52fe8b1d 100644 --- a/pkg/devcontainer/config/result.go +++ b/pkg/devcontainer/config/result.go @@ -1,6 +1,7 @@ package config import ( + "errors" "maps" "slices" @@ -9,6 +10,19 @@ import ( const UserLabel = pkgconfig.DockerUserLabel +// ErrNoAgentResult indicates the agent exited without forwarding a result. +var ErrNoAgentResult = errors.New("agent exited without sending a result") + +// Err returns the structured error the agent forwarded in this result, or nil +// if the result carries no error. A nil receiver returns nil so callers can +// check the transport error separately. +func (r *Result) Err() error { + if r == nil || r.Error == "" { + return nil + } + return errors.New(r.Error) +} + type Result struct { DevContainerConfigWithPath *DevContainerConfigWithPath `json:"DevContainerConfigWithPath"` MergedConfig *MergedDevContainerConfig `json:"MergedConfig"` diff --git a/pkg/devcontainer/config/result_test.go b/pkg/devcontainer/config/result_test.go new file mode 100644 index 000000000..b6e865312 --- /dev/null +++ b/pkg/devcontainer/config/result_test.go @@ -0,0 +1,31 @@ +package config + +import "testing" + +func TestResultErr(t *testing.T) { + tests := []struct { + name string + result *Result + wantErr bool + wantMsg string + }{ + {name: "nil result", result: nil, wantErr: false}, + {name: "no error", result: &Result{}, wantErr: false}, + {name: "with error", result: &Result{Error: "boom"}, wantErr: true, wantMsg: "boom"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.result.Err() + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + if err.Error() != tt.wantMsg { + t.Fatalf("expected %q, got %q", tt.wantMsg, err.Error()) + } + } else if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + }) + } +}