diff --git a/cmd/internal/agent_daemon.go b/cmd/internal/agent_daemon.go index f615ba9ec..67e2813fa 100644 --- a/cmd/internal/agent_daemon.go +++ b/cmd/internal/agent_daemon.go @@ -3,6 +3,7 @@ package cmdinternal import ( "bytes" "context" + "errors" "fmt" "os" "path/filepath" @@ -12,6 +13,7 @@ import ( "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/client/clientimplementation" + agentconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/driver/custom" "github.com/devsy-org/devsy/pkg/log" @@ -19,7 +21,11 @@ import ( "github.com/spf13/cobra" ) -// DaemonCmd holds the cmd flags. +const ( + defaultPatrolInterval = time.Minute + busyGracePeriod = 20 * time.Minute +) + type DaemonCmd struct { *flags.GlobalFlags @@ -27,11 +33,8 @@ type DaemonCmd struct { ShutdownAction string } -// NewDaemonCmd creates a new command. func NewDaemonCmd(flags *flags.GlobalFlags) *cobra.Command { - cmd := &DaemonCmd{ - GlobalFlags: flags, - } + cmd := &DaemonCmd{GlobalFlags: flags} daemonCmd := &cobra.Command{ Use: "daemon", Short: "Watches for activity and stops the server due to inactivity", @@ -51,125 +54,145 @@ func NewDaemonCmd(flags *flags.GlobalFlags) *cobra.Command { } func (cmd *DaemonCmd) Run(ctx context.Context) error { - // The agent daemon is a container/machine-side process; the host - // never runs `devsy agent daemon`. Reject host invocations explicitly - // to avoid silently scanning a non-existent legacy glob. + // The daemon runs only container/machine-side; the host never invokes it. if agent.IsHostAgentInvocation(cmd.AgentDir) { - return fmt.Errorf( + return errors.New( "`devsy internal agent daemon` is only valid inside the workspace container or machine", ) } - logFolder, err := agent.GetAgentDaemonLogFolder(cmd.AgentDir) + logDir, err := agent.GetAgentDaemonLogDir(cmd.AgentDir) if err != nil { return err } - log.Infof("starting Devsy daemon patrol at %s", logFolder) - - // start patrolling + log.Infof("starting Devsy daemon patrol at %s", logDir) cmd.patrol(ctx) - return nil } func (cmd *DaemonCmd) patrol(ctx context.Context) { - // make sure we don't immediately resleep on startup cmd.initialTouch() - // parse the daemon interval - interval := time.Second * 60 - if cmd.Interval != "" { - parsed, err := time.ParseDuration(cmd.Interval) - if err == nil { - interval = parsed - } - } - - // loop over workspace configs and check their last ModTime + ticker := time.NewTicker(cmd.pollInterval()) + defer ticker.Stop() for { - timer := time.NewTimer(interval) select { case <-ctx.Done(): - timer.Stop() return - case <-timer.C: - cmd.doOnce(ctx) + case <-ticker.C: + cmd.patrolOnce(ctx) } } } -func (cmd *DaemonCmd) doOnce(ctx context.Context) { - var latestActivity *time.Time - var workspace *provider2.AgentWorkspaceInfo +func (cmd *DaemonCmd) pollInterval() time.Duration { + if cmd.Interval == "" { + return defaultPatrolInterval + } + parsed, err := time.ParseDuration(cmd.Interval) + if err != nil { + log.Errorf("parse interval %q, using %s: %v", cmd.Interval, defaultPatrolInterval, err) + return defaultPatrolInterval + } + if parsed <= 0 { + log.Errorf("non-positive interval %q, using %s", cmd.Interval, defaultPatrolInterval) + return defaultPatrolInterval + } + return parsed +} - // get base folder — only reachable from Run, which rejects host - // invocations, so FindAgentHomeFolder always resolves the legacy - // container/machine layout here. - baseFolder, err := agent.FindAgentHomeFolder(cmd.AgentDir) +func (cmd *DaemonCmd) workspaceConfigs() (baseDir string, configs []string, err error) { + baseDir, err = agent.FindAgentHomeDir(cmd.AgentDir) if err != nil { - return + return "", nil, err } + pattern := filepath.Join( + baseDir, + "contexts", + "*", + "workspaces", + "*", + provider2.WorkspaceConfigFile, + ) + configs, err = filepath.Glob(pattern) + if err != nil { + return "", nil, fmt.Errorf("glob %s: %w", pattern, err) + } + return baseDir, configs, nil +} - // get all workspace configs - pattern := baseFolder + "/contexts/*/workspaces/*/" + provider2.WorkspaceConfigFile - matches, err := filepath.Glob(pattern) +func (cmd *DaemonCmd) patrolOnce(ctx context.Context) { + baseDir, configs, err := cmd.workspaceConfigs() if err != nil { - log.Errorf("error globing pattern %s: %v", pattern, err) + log.Errorf("list workspace configs: %v", err) return } - // check when the last touch was - latestActivity, workspace = findLatestActivity(matches) - - // should we run shutdown command? + latestActivity, workspace := findLatestActivity(configs) if latestActivity == nil { - if len(matches) == 0 { - log.Infof("no workspaces found in path %q", baseFolder) + if len(configs) == 0 { + log.Infof("no workspaces found in %q", baseDir) } else { log.Infof( - "%d workspaces found in path %q, but none of them had any auto-stop "+ - "configured or were still running / never completed", - len(matches), - baseFolder, + "%d workspaces found in %q, but none had auto-stop configured or were still running", + len(configs), + baseDir, ) } return } - cmd.checkAndShutdown(ctx, latestActivity, workspace) + cmd.checkAndShutdown(ctx, effectiveActivity(*latestActivity), workspace) +} + +var activityFilePath = agentconfig.ContainerActivityFile + +func effectiveActivity(configActivity time.Time) time.Time { + if hb := activityHeartbeat(); hb.After(configActivity) { + return hb + } + return configActivity +} + +func activityHeartbeat() time.Time { + stat, err := os.Stat(activityFilePath) + if err != nil { + return time.Time{} + } + return stat.ModTime() } func (cmd *DaemonCmd) checkAndShutdown( ctx context.Context, - latestActivity *time.Time, + latestActivity time.Time, workspace *provider2.AgentWorkspaceInfo, ) { if cmd.ShutdownAction == config.ShutdownActionNone { return } - // check timeout timeout := agent.DefaultInactivityTimeout if workspace.Agent.Timeout != "" { - var err error - timeout, err = time.ParseDuration(workspace.Agent.Timeout) + parsed, err := time.ParseDuration(workspace.Agent.Timeout) if err != nil { - log.Errorf("error parsing inactivity timeout: %v", err) - timeout = agent.DefaultInactivityTimeout + log.Errorf("parse inactivity timeout, using %s: %v", timeout, err) + } else { + timeout = parsed } } - if latestActivity.Add(timeout).After(time.Now()) { + + deadline := latestActivity.Add(timeout) + if deadline.After(time.Now()) { log.Infof( - "Workspace %q has latest activity at %q, will auto-stop machine in %s", + "workspace %q last active %s, auto-stop in %s", workspace.Workspace.ID, - latestActivity.String(), - time.Until(latestActivity.Add(timeout)).String(), + latestActivity.Format(time.RFC3339), + time.Until(deadline).Round(time.Second), ) return } - // run shutdown command cmd.runShutdownCommand(ctx, workspace) } @@ -177,81 +200,61 @@ func (cmd *DaemonCmd) runShutdownCommand( ctx context.Context, workspace *provider2.AgentWorkspaceInfo, ) { - // get environ environ, err := custom.ToEnvironWithBinaries(ctx, workspace) if err != nil { - log.Errorf("%v", err) + log.Errorf("build shutdown environment: %v", err) return } - // we run the timeout command now - buf := &bytes.Buffer{} - log.Infof( - "run shutdown command for workspace %s: %s", - workspace.Workspace.ID, - strings.Join(workspace.Agent.Exec.Shutdown, " "), - ) + shutdown := strings.Join(workspace.Agent.Exec.Shutdown, " ") + log.Infof("running shutdown command for workspace %s: %s", workspace.Workspace.ID, shutdown) + + var stdout, stderr bytes.Buffer err = clientimplementation.RunCommand(clientimplementation.RunCommandOptions{ Ctx: ctx, Command: workspace.Agent.Exec.Shutdown, Environ: environ, - Stdout: buf, - Stderr: buf, + Stdout: &stdout, + Stderr: &stderr, }) if err != nil { log.Errorf( - "error running %s %s: %v", - strings.Join(workspace.Agent.Exec.Shutdown, " "), - buf.String(), - err, + "run shutdown command %s: %v (stdout: %s, stderr: %s)", + shutdown, err, stdout.String(), stderr.String(), ) return } - log.Infof("ran command: %s", buf.String()) + log.Infof("ran shutdown command (stdout: %s, stderr: %s)", stdout.String(), stderr.String()) } func (cmd *DaemonCmd) initialTouch() { - // get base folder — only reachable from Run, which rejects host - // invocations, so this always resolves the legacy container/machine - // layout. - baseFolder, err := agent.FindAgentHomeFolder(cmd.AgentDir) + _, configs, err := cmd.workspaceConfigs() if err != nil { + log.Errorf("list workspace configs: %v", err) return } - // get workspace configs - pattern := baseFolder + "/contexts/*/workspaces/*/" + provider2.WorkspaceConfigFile - matches, err := filepath.Glob(pattern) - if err != nil { - log.Errorf("error globbing pattern %s: %v", pattern, err) - return - } - - // check when the last touch was now := time.Now() - for _, match := range matches { - if err := os.Chtimes(match, now, now); err != nil { - log.Errorf("error touching workspace config %s: %v", match, err) - continue + for _, cfg := range configs { + if err := os.Chtimes(cfg, now, now); err != nil { + log.Errorf("touch workspace config %s: %v", cfg, err) } } } -func findLatestActivity( - matches []string, -) (*time.Time, *provider2.AgentWorkspaceInfo) { +func findLatestActivity(configs []string) (*time.Time, *provider2.AgentWorkspaceInfo) { var latestActivity *time.Time var workspace *provider2.AgentWorkspaceInfo - for _, match := range matches { - activity, activityWorkspace, err := getActivity(match) + for _, cfg := range configs { + activity, activityWorkspace, err := getActivity(cfg) if err != nil { - log.Errorf("error checking for inactivity: %v", err) + log.Errorf("check inactivity for %s: %v", cfg, err) continue - } else if activity == nil { + } + if activity == nil { continue } - if latestActivity == nil || activity.After(*latestActivity) { latestActivity = activity workspace = activityWorkspace @@ -260,32 +263,23 @@ func findLatestActivity( return latestActivity, workspace } -func getActivity( - workspaceConfig string, -) (*time.Time, *provider2.AgentWorkspaceInfo, error) { +func getActivity(workspaceConfig string) (*time.Time, *provider2.AgentWorkspaceInfo, error) { workspace, err := agent.ParseAgentWorkspaceInfo(workspaceConfig) if err != nil { - log.Errorf("error reading %s: %v", workspaceConfig, err) - return nil, nil, nil + return nil, nil, fmt.Errorf("read %s: %w", workspaceConfig, err) } - - // check if shutdown is configured if len(workspace.Agent.Exec.Shutdown) == 0 { return nil, nil, nil } - // check last access time stat, err := os.Stat(workspaceConfig) if err != nil { return nil, nil, err } - // check if workspace is locked - t := stat.ModTime() + activity := stat.ModTime() if agent.HasWorkspaceBusyFile(filepath.Dir(workspaceConfig)) { - t = t.Add(time.Minute * 20) + activity = activity.Add(busyGracePeriod) } - - // check if timeout - return &t, workspace, nil + return &activity, workspace, nil } diff --git a/cmd/internal/agent_daemon_test.go b/cmd/internal/agent_daemon_test.go new file mode 100644 index 000000000..0e83476f9 --- /dev/null +++ b/cmd/internal/agent_daemon_test.go @@ -0,0 +1,114 @@ +package cmdinternal + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/agent" + provider2 "github.com/devsy-org/devsy/pkg/provider" + "github.com/devsy-org/devsy/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testEcho = "echo" + +func writeWorkspaceConfig(t *testing.T, dir string, shutdown types.StrArray) string { + t.Helper() + require.NoError(t, os.MkdirAll(dir, 0o750)) + + info := &provider2.AgentWorkspaceInfo{Workspace: &provider2.Workspace{ID: "ws-test"}} + info.Agent.Exec.Shutdown = shutdown + + data, err := json.Marshal(info) + require.NoError(t, err) + + path := filepath.Join(dir, provider2.WorkspaceConfigFile) + require.NoError(t, os.WriteFile(path, data, 0o600)) + return path +} + +func TestGetActivity_ShutdownConfigured(t *testing.T) { + cfg := writeWorkspaceConfig(t, t.TempDir(), types.StrArray{testEcho, "stop"}) + + activity, ws, err := getActivity(cfg) + require.NoError(t, err) + require.NotNil(t, activity) + require.NotNil(t, ws) + assert.Equal(t, "ws-test", ws.Workspace.ID) + + stat, err := os.Stat(cfg) + require.NoError(t, err) + assert.Equal(t, stat.ModTime(), *activity) +} + +func TestGetActivity_NoShutdownReturnsNil(t *testing.T) { + cfg := writeWorkspaceConfig(t, t.TempDir(), nil) + + activity, ws, err := getActivity(cfg) + require.NoError(t, err) + assert.Nil(t, activity) + assert.Nil(t, ws) +} + +func TestGetActivity_BusyFileAddsGrace(t *testing.T) { + dir := t.TempDir() + cfg := writeWorkspaceConfig(t, dir, types.StrArray{testEcho, "stop"}) + agent.CreateWorkspaceBusyFile(dir) + + activity, _, err := getActivity(cfg) + require.NoError(t, err) + require.NotNil(t, activity) + + stat, err := os.Stat(cfg) + require.NoError(t, err) + assert.Equal(t, stat.ModTime().Add(busyGracePeriod), *activity) +} + +func TestGetActivity_ReadError(t *testing.T) { + _, _, err := getActivity(filepath.Join(t.TempDir(), "missing.json")) + assert.Error(t, err) +} + +func TestFindLatestActivity_PicksLatest(t *testing.T) { + base := t.TempDir() + older := writeWorkspaceConfig(t, filepath.Join(base, "a"), types.StrArray{testEcho}) + newer := writeWorkspaceConfig(t, filepath.Join(base, "b"), types.StrArray{testEcho}) + + oldTime := time.Now().Add(-2 * time.Hour).Truncate(time.Second) + require.NoError(t, os.Chtimes(older, oldTime, oldTime)) + recentTime := time.Now().Add(-time.Minute).Truncate(time.Second) + require.NoError(t, os.Chtimes(newer, recentTime, recentTime)) + + activity, ws := findLatestActivity([]string{older, newer}) + require.NotNil(t, activity) + require.NotNil(t, ws) + assert.Equal(t, recentTime, *activity) +} + +func TestEffectiveActivity(t *testing.T) { + orig := activityFilePath + t.Cleanup(func() { activityFilePath = orig }) + + configActivity := time.Now().Add(-30 * time.Minute).Truncate(time.Second) + + touch := func(name string, mtime time.Time) string { + path := filepath.Join(t.TempDir(), name) + require.NoError(t, os.WriteFile(path, nil, 0o600)) + require.NoError(t, os.Chtimes(path, mtime, mtime)) + return path + } + + activityFilePath = filepath.Join(t.TempDir(), "absent.activity") + assert.Equal(t, configActivity, effectiveActivity(configActivity)) + + freshTime := time.Now().Add(-time.Minute).Truncate(time.Second) + activityFilePath = touch("fresh.activity", freshTime) + assert.Equal(t, freshTime, effectiveActivity(configActivity)) + + activityFilePath = touch("stale.activity", time.Now().Add(-2*time.Hour).Truncate(time.Second)) + assert.Equal(t, configActivity, effectiveActivity(configActivity)) +} diff --git a/cmd/internal/agentcontainer/setup.go b/cmd/internal/agentcontainer/setup.go index 9cc0e1ad2..eebc6531c 100644 --- a/cmd/internal/agentcontainer/setup.go +++ b/cmd/internal/agentcontainer/setup.go @@ -446,14 +446,13 @@ func (cmd *SetupContainerCmd) cloneRepositoryIfNeeded( return nil } - return agent.CloneRepositoryForWorkspace(ctx, - &workspaceInfo.Source, - &workspaceInfo.Agent, - setupInfo.SubstitutionContext.ContainerWorkspaceFolder, - "", - workspaceInfo.CLIOptions, - true, - ) + return agent.CloneRepositoryForWorkspace(ctx, agent.CloneWorkspaceParams{ + Source: &workspaceInfo.Source, + AgentConfig: &workspaceInfo.Agent, + WorkspaceDir: setupInfo.SubstitutionContext.ContainerWorkspaceFolder, + Options: workspaceInfo.CLIOptions, + OverwriteContent: true, + }) } func (cmd *SetupContainerCmd) startContainerDaemon( diff --git a/cmd/internal/agentworkspace/build.go b/cmd/internal/agentworkspace/build.go index d4522e134..3ad66ee7c 100644 --- a/cmd/internal/agentworkspace/build.go +++ b/cmd/internal/agentworkspace/build.go @@ -96,7 +96,6 @@ func (cmd *BuildCmd) Run(ctx context.Context) error { PushDuringBuild: workspaceInfo.CLIOptions.PushDuringBuild, }) if err != nil { - log.Errorf("Error building image: %v", err) return fmt.Errorf("build: %w", err) } diff --git a/cmd/internal/agentworkspace/logs_daemon.go b/cmd/internal/agentworkspace/logs_daemon.go index ef5cc5a67..d1b0905d1 100644 --- a/cmd/internal/agentworkspace/logs_daemon.go +++ b/cmd/internal/agentworkspace/logs_daemon.go @@ -59,12 +59,13 @@ func (cmd *LogsDaemonCmd) Run(ctx context.Context) error { return nil } - logFolder, err := agent.GetAgentDaemonLogFolder(cmd.AgentDir) + logDir, err := agent.GetAgentDaemonLogDir(cmd.AgentDir) if err != nil { return err } - f, err := os.Open(filepath.Join(logFolder, "agent-daemon.log")) + // #nosec G304 -- reads the agent's own daemon log at a derived path. + f, err := os.Open(filepath.Join(logDir, "agent-daemon.log")) if err != nil { return fmt.Errorf("open agent-daemon.log: %w", err) } diff --git a/cmd/internal/agentworkspace/setup_gpg.go b/cmd/internal/agentworkspace/setup_gpg.go index f034a3420..d8ba76706 100644 --- a/cmd/internal/agentworkspace/setup_gpg.go +++ b/cmd/internal/agentworkspace/setup_gpg.go @@ -86,8 +86,7 @@ func fetchAndDecodeKeys(ownerTrustB64 string) ([]byte, []byte, error) { log.Debugf("Fetching public key") rawPublicKeys, err := getPublicKeys() if err != nil { - log.Errorf("Fetch public key: %v", err) - return nil, nil, err + return nil, nil, fmt.Errorf("fetch public key: %w", err) } log.Debugf("Decoding public key") @@ -108,46 +107,39 @@ func fetchAndDecodeKeys(ownerTrustB64 string) ([]byte, []byte, error) { func configureGPGAgent(gpgConf *gpg.GPGConf) error { log.Debugf("Stopping container gpg-agent") if err := gpgConf.StopGpgAgent(); err != nil { - log.Errorf("stop container gpg-agent: %v", err) - return err + return fmt.Errorf("stop container gpg-agent: %w", err) } log.Debugf("Importing gpg public key in container") if err := gpgConf.ImportGpgKey(); err != nil { - log.Errorf("Import gpg public key in container: %v", err) - return err + return fmt.Errorf("import gpg public key in container: %w", err) } log.Debugf("Importing gpg owner trust in container") if err := gpgConf.ImportOwnerTrust(); err != nil { - log.Errorf("Import gpg owner trust in container: %v", err) - return err + return fmt.Errorf("import gpg owner trust in container: %w", err) } log.Debugf("Ensuring paths existence and permissions") if err := gpgConf.SetupRemoteSocketDirTree(); err != nil { - log.Errorf("Ensure paths existence and permissions: %v", err) - return err + return fmt.Errorf("ensure paths existence and permissions: %w", err) } // Now we again kill the agent and remove the socket to really be sure every // thing is clean log.Debugf("Ensure stopping container gpg-agent") if err := gpgConf.StopGpgAgent(); err != nil { - log.Errorf("Ensure stopping container gpg-agent: %v", err) - return err + return fmt.Errorf("ensure stopping container gpg-agent: %w", err) } log.Debugf("Setup local gnupg socket links") if err := gpgConf.SetupRemoteSocketLink(); err != nil { - log.Errorf("Setup local gnupg socket links: %v", err) - return err + return fmt.Errorf("setup local gnupg socket links: %w", err) } log.Debugf("Setup gpg.conf") if err := gpgConf.SetupGpgConf(); err != nil { - log.Errorf("Setup gpg.conf: %v", err) - return err + return fmt.Errorf("setup gpg.conf: %w", err) } return nil diff --git a/cmd/internal/agentworkspace/up.go b/cmd/internal/agentworkspace/up.go index d63a5528f..39519d20a 100644 --- a/cmd/internal/agentworkspace/up.go +++ b/cmd/internal/agentworkspace/up.go @@ -615,15 +615,13 @@ func prepareGitWorkspace(ctx context.Context, params prepareGitWorkspaceParams) return nil } - return agent.CloneRepositoryForWorkspace( - ctx, - ¶ms.workspaceInfo.Workspace.Source, - ¶ms.workspaceInfo.Agent, - params.workspaceInfo.ContentFolder, - params.gitHelper, - params.workspaceInfo.CLIOptions, - false, - ) + return agent.CloneRepositoryForWorkspace(ctx, agent.CloneWorkspaceParams{ + Source: ¶ms.workspaceInfo.Workspace.Source, + AgentConfig: ¶ms.workspaceInfo.Agent, + WorkspaceDir: params.workspaceInfo.ContentFolder, + Helper: params.gitHelper, + Options: params.workspaceInfo.CLIOptions, + }) } func prepareLocalWorkspace( diff --git a/cmd/mcp/errors.go b/cmd/mcp/errors.go index a06ee802b..96b29b0f1 100644 --- a/cmd/mcp/errors.go +++ b/cmd/mcp/errors.go @@ -1,39 +1,21 @@ package mcp import ( - "errors" - - cliErrors "github.com/devsy-org/devsy/pkg/errors" - "github.com/devsy-org/devsy/pkg/workspace" + "github.com/devsy-org/devsy/pkg/clierr" ) -// ErrorPayload is the JSON shape attached to MCP tool errors so the agent gets -// the same structured information the Devsy CLI shows humans (code, hint, doc URL). type ErrorPayload struct { Code string `json:"code"` Message string `json:"message"` - Hint string `json:"hint,omitempty"` - DocURL string `json:"doc_url,omitempty"` } -// ClassifyError converts any error returned by an MCP handler into a structured -// payload using the same classifier the CLI uses. func ClassifyError(err error) ErrorPayload { - if err == nil { + classified := clierr.Classify(err) + if classified == nil { return ErrorPayload{} } - classified := cliErrors.Classify(err, cliErrors.ClassifyContext{}) - code := "internal_error" - if classified.Code != "" { - code = string(classified.Code) - } - if errors.Is(err, workspace.ErrWorkspaceNotFound) { - code = "workspace_not_found" - } return ErrorPayload{ - Code: code, - Message: err.Error(), - Hint: classified.Hint, - DocURL: classified.DocURL, + Code: string(classified.Code), + Message: classified.Message, } } diff --git a/cmd/pro/start.go b/cmd/pro/start.go index 69961e3c7..32a90b84f 100644 --- a/cmd/pro/start.go +++ b/cmd/pro/start.go @@ -2041,6 +2041,10 @@ func (e *Error) Error() string { return message + e.err.Error() } +func (e *Error) Unwrap() error { + return e.err +} + func getMachineUID() string { id, err := machineid.ID() if err != nil { diff --git a/cmd/provider/configure_shared.go b/cmd/provider/configure_shared.go index c7e4cfae0..e1fccc497 100644 --- a/cmd/provider/configure_shared.go +++ b/cmd/provider/configure_shared.go @@ -1,14 +1,12 @@ package provider import ( - "bytes" "context" "fmt" "io" "github.com/devsy-org/devsy/pkg/client/clientimplementation" "github.com/devsy-org/devsy/pkg/config" - cliErrors "github.com/devsy-org/devsy/pkg/errors" "github.com/devsy-org/devsy/pkg/log" options2 "github.com/devsy-org/devsy/pkg/options" provider2 "github.com/devsy-org/devsy/pkg/provider" @@ -166,10 +164,6 @@ func initProvider( provider *provider2.ProviderConfig, io2 initIO, ) error { - // Capture the sub-binary's stderr in parallel with forwarding it to the - // regular log sink so that errors.Classify has the real provider output - // to fingerprint, not just an opaque "exit status 1". - stderrBuf := &bytes.Buffer{} err := clientimplementation.RunCommandWithBinaries(clientimplementation.CommandOptions{ Ctx: ctx, Name: "init", @@ -178,13 +172,10 @@ func initProvider( Options: devsyConfig.ProviderOptions(provider.Name), Config: provider, Stdout: io2.stdout, - Stderr: io.MultiWriter(io2.stderr, stderrBuf), + Stderr: io2.stderr, }) if err != nil { - return cliErrors.Classify(fmt.Errorf("init: %w", err), cliErrors.ClassifyContext{ - Provider: provider.Name, - Stderr: stderrBuf.String(), - }) + return fmt.Errorf("init: %w", err) } if devsyConfig.Current().Providers == nil { devsyConfig.Current().Providers = map[string]*config.ProviderConfig{} diff --git a/cmd/root.go b/cmd/root.go index a017ace95..01bddb3be 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/exec" + "runtime/debug" "strings" "github.com/devsy-org/devsy/cmd/completion" @@ -22,8 +23,8 @@ import ( "github.com/devsy-org/devsy/cmd/self" "github.com/devsy-org/devsy/cmd/template" wsCmdPkg "github.com/devsy-org/devsy/cmd/workspace" + "github.com/devsy-org/devsy/pkg/clierr" "github.com/devsy-org/devsy/pkg/config" - cliErrors "github.com/devsy-org/devsy/pkg/errors" "github.com/devsy-org/devsy/pkg/exitcode" "github.com/devsy-org/devsy/pkg/flatpak" "github.com/devsy-org/devsy/pkg/log" @@ -101,18 +102,26 @@ func Execute() { os.Exit(run()) } -func run() int { +func run() (code int) { + machineMode := false + collector := telemetry.FromContext(gocontext.Background()) // noop until BootstrapCLI + defer func() { + if r := recover(); r != nil { + log.Errorf("panic: %v\n%s", r, debug.Stack()) + panicErr := clierr.NewPanic(r) + collector.RecordCLI(panicErr) + code = exitCodeForError(panicErr, machineMode) + } + collector.Flush() + }() + rootCmd, globalFlags := BuildRoot() - target := rootCmd - if found, _, findErr := rootCmd.Find(os.Args[1:]); findErr == nil && found != nil { - target = found - } - collector := telemetry.BootstrapCLI(target) + target := resolveTarget(rootCmd) + collector = telemetry.BootstrapCLI(target) rootCmd.SetContext(telemetry.WithCollector(gocontext.Background(), collector)) - defer func() { collector.Flush() }() isInternal := topLevelCommand(target) == internalCommand - machineMode := configureOutput(rootCmd, globalFlags, isInternal) + machineMode = configureOutput(rootCmd, globalFlags, isInternal) if !isInternal { if shouldExit, err := flatpak.ReexecOnHost(); err != nil { @@ -138,6 +147,13 @@ func run() int { return 0 } +func resolveTarget(rootCmd *cobra.Command) *cobra.Command { + if found, _, err := rootCmd.Find(os.Args[1:]); err == nil && found != nil { + return found + } + return rootCmd +} + func configureOutput( rootCmd *cobra.Command, globalFlags *flags.GlobalFlags, @@ -145,7 +161,7 @@ func configureOutput( ) bool { logOutput := logOutputFromArgs(os.Args[1:]) machineMode := isMachineConsumer(logOutput, isInternal) - rootCmd.SilenceErrors = machineMode + rootCmd.SilenceErrors = true rootCmd.SilenceUsage = machineMode format := logOutput @@ -176,18 +192,23 @@ func topLevelCommand(cmd *cobra.Command) string { func exitCodeForError(err error, machineMode bool) int { if err == nil { - return 0 + return exitcode.Success } - if code, ok := passthroughExitCode(err, machineMode); ok { - return code + cliErr := clierr.Classify(err) + + // Stay transparent for unclassified child-process exits (e.g. `devsy ssh -- cmd`). + if cliErr.Code == clierr.CodeUnknown { + if code, ok := passthroughExitCode(err, machineMode); ok { + return code + } } - renderCLIError(err, machineMode) + renderCLIError(cliErr, machineMode) if errors.Is(err, workspace.ErrWorkspaceNotFound) { - return exitcode.WorkspaceNotFound + return exitcode.Retryable } - return 1 + return exitcode.Failure } func passthroughExitCode(err error, machineMode bool) (int, bool) { @@ -206,18 +227,15 @@ func passthroughExitCode(err error, machineMode bool) (int, bool) { return 0, false } -func renderCLIError(err error, machineMode bool) { - cliErr := cliErrors.Classify(err, cliErrors.ClassifyContext{}) +func renderCLIError(cliErr *clierr.CLIError, machineMode bool) { + if cliErr == nil { + return + } if machineMode { log.JSONError(cliErr) return } - if cliErr.Hint != "" { - fmt.Fprintf(os.Stderr, "Hint: %s\n", cliErr.Hint) - } - if cliErr.DocURL != "" { - fmt.Fprintf(os.Stderr, "See: %s\n", cliErr.DocURL) - } + fmt.Fprintf(os.Stderr, "Error: %s\n", cliErr.Message) } // BuildRoot constructs the root command and returns it alongside the parsed diff --git a/cmd/root_test.go b/cmd/root_test.go index 1b46fd2c9..b07c7e120 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,14 +1,26 @@ package cmd import ( + "fmt" "os" "testing" "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/exitcode" + "github.com/devsy-org/devsy/pkg/workspace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestExitCodeForError_WorkspaceNotFound(t *testing.T) { + err := fmt.Errorf("get workspace: %w", workspace.ErrWorkspaceNotFound) + assert.Equal(t, exitcode.Retryable, exitCodeForError(err, true)) +} + +func TestExitCodeForError_GenericFailure(t *testing.T) { + assert.Equal(t, exitcode.Failure, exitCodeForError(fmt.Errorf("boom"), true)) +} + func TestTopLevelCommand(t *testing.T) { rootCmd, _ := BuildRoot() @@ -129,7 +141,7 @@ func TestConfigureOutput_SilencesCobra(t *testing.T) { os.Args = append([]string{"devsy"}, tc.args...) machineMode := configureOutput(rootCmd, globalFlags, tc.isInternal) assert.Equal(t, tc.wantSilent, machineMode) - assert.Equal(t, tc.wantSilent, rootCmd.SilenceErrors) + assert.True(t, rootCmd.SilenceErrors) assert.Equal(t, tc.wantSilent, rootCmd.SilenceUsage) }) } diff --git a/desktop/src/main/__tests__/cli.test.ts b/desktop/src/main/__tests__/cli.test.ts index 24b89dfbc..4f611cc3e 100644 --- a/desktop/src/main/__tests__/cli.test.ts +++ b/desktop/src/main/__tests__/cli.test.ts @@ -79,14 +79,8 @@ describe("CliRunner", () => { typeof vi.fn > const cliErrorPayload = { - code: "AWS_PROFILE_MISSING", - message: "AWS credentials are not configured.", - hint: "Set AWS_PROFILE or create ~/.aws/credentials.", - docUrl: - "https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html", - provider: "aws", - cause: - "init: exit status 1: failed to get shared config profile, default", + code: "RATE_LIMITED", + message: "Rate limited by an upstream API. Wait and retry, or authenticate for a higher limit.", } const stderrLine = JSON.stringify({ level: "error", diff --git a/desktop/src/renderer/src/lib/components/ErrorCard.svelte b/desktop/src/renderer/src/lib/components/ErrorCard.svelte index 32d922a3e..52398b823 100644 --- a/desktop/src/renderer/src/lib/components/ErrorCard.svelte +++ b/desktop/src/renderer/src/lib/components/ErrorCard.svelte @@ -1,5 +1,5 @@