diff --git a/cmd/flags/flags.go b/cmd/flags/flags.go index a8e8284ec3..05edd74513 100644 --- a/cmd/flags/flags.go +++ b/cmd/flags/flags.go @@ -16,6 +16,7 @@ type GlobalFlags struct { Owner platform.OwnerFilter LogOutput string + LogLevel string ResultFormat string Verbosity int Quiet bool @@ -42,6 +43,12 @@ func registerCoreFlags(flags *flag.FlagSet, globalFlags *GlobalFlags) { "", "If defined will override the default devsy home", ) + flags.StringVar( + &globalFlags.LogLevel, + names.LogLevel, + "", + "The log level override. Can be error, warn, info, debug, or trace", + ) flags.StringVar(&globalFlags.Context, names.Context, "", "The context to use") flags.StringVar( &globalFlags.Provider, @@ -80,7 +87,7 @@ func registerVerbosityFlags(flags *flag.FlagSet, globalFlags *GlobalFlags) { names.Quiet, "q", false, - "Suppress all log output except fatal errors", + "Suppress all log output except errors", ) flags.BoolVar( &globalFlags.Debug, diff --git a/cmd/root.go b/cmd/root.go index a6b23e1378..3792d84ad8 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -31,6 +31,7 @@ import ( "github.com/devsy-org/devsy/pkg/clierr" "github.com/devsy-org/devsy/pkg/clihelp" "github.com/devsy-org/devsy/pkg/config" + devcconfig "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/exitcode" "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/flatpak" @@ -102,6 +103,31 @@ func logOutputFromArgs(args []string) string { return "" } +func defaultLogLevel(globalFlags *flags.GlobalFlags) string { + devsyConfig, err := config.LoadConfig(globalFlags.Context, globalFlags.Provider) + if err != nil { + return log.DefaultLevel + } + if level := devsyConfig.ContextOption(config.ContextOptionLogLevel); level != "" { + return level + } + return log.DefaultLevel +} + +func validateLogLevel(level string) error { + if level == "" { + return nil + } + if _, ok := log.LevelFromString(level); !ok { + return fmt.Errorf( + "invalid log level %q, expected one of: %s", + level, + strings.Join(log.ValidLevels(), ", "), + ) + } + return nil +} + func isMachineConsumer(logOutput string, isInternal bool) bool { switch { case isInternal: @@ -190,11 +216,14 @@ func configureOutput( } } log.Init(log.Config{ - Verbosity: globalFlags.Verbosity, - Quiet: globalFlags.Quiet, - Debug: globalFlags.Debug, - Format: format, - Redactor: secrets.NewEnvironmentRedactor(os.Environ()), + Verbosity: globalFlags.Verbosity, + Quiet: globalFlags.Quiet, + Debug: globalFlags.Debug, + Level: globalFlags.LogLevel, + DefaultLevel: defaultLogLevel(globalFlags), + VerbositySet: rootCmd.PersistentFlags().Changed(names.Verbose), + Format: format, + Redactor: secrets.NewEnvironmentRedactor(os.Environ()), }) return machineMode } @@ -239,13 +268,25 @@ func exitCodeForError(err error, machineMode bool) int { func passthroughExitCode(err error, machineMode bool) (int, bool) { if sshExitErr, ok := errors.AsType[*ssh.ExitError](err); ok { if machineMode { - log.Errorf("SSH command failed with exit code %d", sshExitErr.ExitStatus()) + renderCLIError( + clierr.Classify(fmt.Errorf( + "SSH command failed with exit code %d", + sshExitErr.ExitStatus(), + )), + true, + ) } return sshExitErr.ExitStatus(), true } if execExitErr, ok := errors.AsType[*exec.ExitError](err); ok { if machineMode { - log.Errorf("Command failed with exit code %d", execExitErr.ExitCode()) + renderCLIError( + clierr.Classify(fmt.Errorf( + "command failed with exit code %d", + execExitErr.ExitCode(), + )), + true, + ) } return execExitErr.ExitCode(), true } @@ -257,7 +298,9 @@ func renderCLIError(cliErr *clierr.CLIError, machineMode bool) { return } if machineMode { - log.JSONError(cliErr) + if err := devcconfig.WriteCLIErrorJSON(os.Stderr, cliErr); err != nil { + log.Errorf("failed to write CLI error envelope: %v", err) + } return } redactor := secrets.NewEnvironmentRedactor(os.Environ()) @@ -307,6 +350,12 @@ func BuildRoot() (*cobra.Command, *flags.GlobalFlags) { _ = completion.RegisterFlagCompletionFuns(rootCmd, globalFlags) rootCmd.PersistentPreRunE = func(cobraCmd *cobra.Command, _ []string) error { + if err := validateLogLevel(globalFlags.LogLevel); err != nil { + return err + } + if globalFlags.DevsyHome != "" { + _ = os.Setenv(config.EnvHome, globalFlags.DevsyHome) + } cobraCmd.SilenceUsage = true logFormat := globalFlags.LogOutput if logFormat == "" { @@ -318,18 +367,17 @@ func BuildRoot() (*cobra.Command, *flags.GlobalFlags) { } log.Init(log.Config{ - Verbosity: globalFlags.Verbosity, - Quiet: globalFlags.Quiet, - Debug: globalFlags.Debug, - Format: logFormat, - Redactor: secrets.NewEnvironmentRedactor(os.Environ()), + Verbosity: globalFlags.Verbosity, + Quiet: globalFlags.Quiet, + Debug: globalFlags.Debug, + Level: globalFlags.LogLevel, + DefaultLevel: defaultLogLevel(globalFlags), + VerbositySet: persistentFlags.Changed(names.Verbose), + Format: logFormat, + Redactor: secrets.NewEnvironmentRedactor(os.Environ()), }) klog.SetLogger(logr.New(log.LogrSink())) - if globalFlags.DevsyHome != "" { - _ = os.Setenv(config.EnvHome, globalFlags.DevsyHome) - } - devsyConfig, err := config.LoadConfig(globalFlags.Context, globalFlags.Provider) if err == nil { current := telemetry.FromContext(cobraCmd.Context()) diff --git a/cmd/root_test.go b/cmd/root_test.go index 20ac10db37..5591fabf5f 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -11,6 +11,7 @@ import ( "github.com/devsy-org/devsy/pkg/clierr" "github.com/devsy-org/devsy/pkg/config" + devcconfig "github.com/devsy-org/devsy/pkg/devcontainer/config" "github.com/devsy-org/devsy/pkg/exitcode" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/workspace" @@ -50,6 +51,49 @@ func TestRenderCLIErrorRedactsEnvironmentSecrets(t *testing.T) { assert.Contains(t, output, "***") } +func TestRenderCLIErrorMachineModeWritesDirectEnvelope(t *testing.T) { + r, w, err := os.Pipe() + require.NoError(t, err) + original := os.Stderr + os.Stderr = w + renderCLIError(&clierr.CLIError{Code: clierr.CodeUnknown, Message: "machine failure"}, true) + require.NoError(t, w.Close()) + os.Stderr = original + var envelope struct { + Kind string `json:"kind"` + Outcome string `json:"outcome"` + Message string `json:"message"` + } + require.NoError(t, json.NewDecoder(r).Decode(&envelope)) + assert.Equal(t, "error", envelope.Kind) + assert.Equal(t, "error", envelope.Outcome) + assert.Equal(t, "machine failure", envelope.Message) +} + +func TestRenderCLIErrorMachineModeIgnoresDiagnosticLevel(t *testing.T) { + for _, level := range log.ValidLevels() { + t.Run(level, func(t *testing.T) { + log.Init(log.Config{Level: level}) + r, w, err := os.Pipe() + require.NoError(t, err) + original := os.Stderr + os.Stderr = w + renderCLIError( + &clierr.CLIError{Code: clierr.CodeUnknown, Message: "machine failure"}, + true, + ) + require.NoError(t, w.Close()) + os.Stderr = original + + var envelope devcconfig.ErrorEnvelope + require.NoError(t, json.NewDecoder(r).Decode(&envelope)) + assert.Equal(t, "error", envelope.Kind) + assert.Equal(t, "error", envelope.Outcome) + assert.Equal(t, "machine failure", envelope.Message) + }) + } +} + func TestTopLevelCommand(t *testing.T) { rootCmd, _ := BuildRoot() diff --git a/desktop/e2e/fixtures/mock-devsy.cjs b/desktop/e2e/fixtures/mock-devsy.cjs index da0084f670..b3ac213864 100755 --- a/desktop/e2e/fixtures/mock-devsy.cjs +++ b/desktop/e2e/fixtures/mock-devsy.cjs @@ -144,9 +144,20 @@ function parseArgs(args) { let providerFlag = "" let ideFlag = "" let nameFlag = "" + let logLevel = "warn" let i = 0 while (i < args.length) { const arg = args[i] + if (arg === "--log-level") { + logLevel = args[i + 1] || "warn" + i += 2 + continue + } + if (arg.startsWith("--log-level=")) { + logLevel = arg.slice("--log-level=".length) + i++ + continue + } if (arg === "--result-format") { i += 2 continue @@ -194,7 +205,14 @@ function parseArgs(args) { positional.push(arg) i++ } - return { positional, idFlag, providerFlag, ideFlag, nameFlag } + return { positional, idFlag, providerFlag, ideFlag, nameFlag, logLevel } +} + +function emitDiagnostics(logLevel) { + const levels = { error: 0, warn: 1, info: 2, debug: 3, trace: 4 } + const selected = levels[logLevel] ?? levels.warn + if (selected >= levels.info) process.stderr.write("INFO diagnostic line\n") + if (selected >= levels.warn) process.stderr.write("WARN diagnostic line\n") } // Workspace verb handlers. Each accepts the slice of args AFTER its verb, @@ -570,6 +588,7 @@ const featureHandlers = { } const cmd = rawArgs[0] || "" +emitDiagnostics(parseArgs(rawArgs).logLevel) // Canonical form: `devsy feature ...` if (cmd === "feature") { diff --git a/desktop/e2e/log-level-protocol.e2e.ts b/desktop/e2e/log-level-protocol.e2e.ts new file mode 100644 index 0000000000..d42b105e46 --- /dev/null +++ b/desktop/e2e/log-level-protocol.e2e.ts @@ -0,0 +1,69 @@ +import type { ElectronApplication, Page } from "@playwright/test" +import { expect, test } from "@playwright/test" +import { launchApp, resetMockState } from "./electron-app.js" + +let app: ElectronApplication +let page: Page + +async function invoke(channel: string, args?: Record) { + return page.evaluate( + ({ channel, args }) => window.electronAPI.invoke(channel, args), + { channel, args }, + ) +} + +async function readLatestLog(workspaceId: string): Promise { + const logs = (await invoke("workspace_logs_list", { workspaceId })) as Array<{ + filename: string + }> + const latest = logs[0] + return latest + ? ((await invoke("workspace_log_read", { + workspaceId, + filename: latest.filename, + })) as string) + : "" +} + +test.beforeAll(async () => { + resetMockState() + ;({ app, page } = await launchApp()) +}) + +test.afterAll(async () => { + await app?.close() +}) + +test("status and result protocols are independent of CLI diagnostic level", async () => { + test.setTimeout(120000) + for (const [index, level] of (["info", "warn", "error"] as const).entries()) { + const workspaceId = `protocol-${index}` + await invoke("set_app_settings", { patch: { cliCaptureLogLevel: level } }) + await invoke("workspace_up", { + source: `https://example.com/${workspaceId}.git`, + workspaceId, + }) + await expect + .poll(async () => { + const snapshot = (await invoke("workspace_snapshot")) as { + jobs: Record + } + return snapshot.jobs[workspaceId]?.state + }, { timeout: 30000 }) + .toBe("succeeded") + await expect + .poll( + async () => { + const log = await readLatestLog(workspaceId) + return ( + log.includes("INFO diagnostic line") === (level === "info") && + log.includes("WARN diagnostic line") === (level !== "error") + ) + }, + { timeout: 10000 }, + ) + .toBe(true) + const workspaces = (await invoke("workspace_list")) as Array<{ id: string; status: string }> + expect(workspaces.find((workspace) => workspace.id === workspaceId)?.status).toBe("Running") + } +}) diff --git a/desktop/e2e/workspaces.e2e.ts b/desktop/e2e/workspaces.e2e.ts index e158528f38..6220ffde2f 100644 --- a/desktop/e2e/workspaces.e2e.ts +++ b/desktop/e2e/workspaces.e2e.ts @@ -61,8 +61,28 @@ test.describe("Workspace lifecycle badges", () => { [channel, args] as const, ) + const workspaceSnapshot = async () => + (await api("workspace_snapshot", {})) as { + workspaces: Array<{ id: string }> + jobs: Record + } + + const waitForDeleteToSettle = async () => { + await expect + .poll( + async () => { + const snapshot = await workspaceSnapshot() + return snapshot.jobs.deleteprobe?.state ?? "missing" + }, + { timeout: 10000 }, + ) + .toMatch(/^(succeeded|failed|missing)$/) + } + test("shows a Deleting badge while removal is in flight, then removes the row", async () => { const main = page.locator('[data-slot="sidebar-inset"] main') + let deleteAccepted = false + let testError: unknown try { await api("workspace_up", { @@ -73,19 +93,41 @@ test.describe("Workspace lifecycle badges", () => { timeout: 5000, }) - // Not awaited: the assertions below run while the delete is in flight. - void api("workspace_delete", { workspaceId: "deleteprobe" }).catch( - () => undefined, - ) + // IPC acceptance is immediate; keep these assertions ahead of CLI completion. + await api("workspace_delete", { workspaceId: "deleteprobe" }) + deleteAccepted = true await expect(main).toContainText("Deleting", { timeout: 3000 }) await expect(main.locator("text=deleteprobe")).not.toBeVisible({ timeout: 5000, }) - } finally { - // Mock CLI state is shared across specs, so don't leave this behind. - await api("workspace_delete", { workspaceId: "deleteprobe" }) + await waitForDeleteToSettle() + } catch (error) { + testError = error + } + // Let an accepted delete finish before retrying cleanup. + try { + if (deleteAccepted) await waitForDeleteToSettle() + const snapshot = await workspaceSnapshot() + if (snapshot.workspaces.some(({ id }) => id === "deleteprobe")) { + await api("workspace_delete", { workspaceId: "deleteprobe" }) + await waitForDeleteToSettle() + // Avoid leaking this fixture into later tests. + const finalSnapshot = await workspaceSnapshot() + if (finalSnapshot.workspaces.some(({ id }) => id === "deleteprobe")) { + throw new Error("deleteprobe workspace still present after delete retry") + } + } + } catch (cleanupError) { + if (testError) { + throw new AggregateError( + [testError, cleanupError], + "Test and cleanup failed", + ) + } + throw cleanupError } + if (testError) throw testError }) }) diff --git a/desktop/src/main/__tests__/app-settings.test.ts b/desktop/src/main/__tests__/app-settings.test.ts index 7679f874d6..5ef8c54402 100644 --- a/desktop/src/main/__tests__/app-settings.test.ts +++ b/desktop/src/main/__tests__/app-settings.test.ts @@ -29,6 +29,8 @@ describe("normalizeAppSettings", () => { runAtStartup: true, openToTrayOnStartup: true, trayNotifications: "all", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) }) @@ -39,6 +41,8 @@ describe("normalizeAppSettings", () => { runAtStartup: false, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) }) @@ -48,6 +52,12 @@ describe("normalizeAppSettings", () => { ).toBe("failures") }) + it("accepts only the finite logging level set", () => { + expect(normalizeAppSettings({ logLevel: "trace" })).toMatchObject({ desktopLogLevel: "trace", cliCaptureLogLevel: "trace" }) + expect(normalizeAppSettings({ logLevel: "verbose" })).toMatchObject({ desktopLogLevel: "info", cliCaptureLogLevel: "info" }) + expect(() => sanitizeAppSettingsPatch({ logLevel: "verbose" })).toThrow() + }) + it("ignores non-boolean toggles", () => { expect(normalizeAppSettings({ runAtStartup: "yes" }).runAtStartup).toBe( false, @@ -61,11 +71,15 @@ describe("patchAppSettings", () => { runAtStartup: true, openToTrayOnStartup: true, trayNotifications: "all" as const, + desktopLogLevel: "info" as const, + cliCaptureLogLevel: "info" as const, } expect(patchAppSettings(current, { runAtStartup: false })).toEqual({ runAtStartup: false, openToTrayOnStartup: false, trayNotifications: "all", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) }) @@ -74,11 +88,15 @@ describe("patchAppSettings", () => { runAtStartup: true, openToTrayOnStartup: false, trayNotifications: "off" as const, + desktopLogLevel: "info" as const, + cliCaptureLogLevel: "info" as const, } expect(patchAppSettings(current, { openToTrayOnStartup: true })).toEqual({ runAtStartup: true, openToTrayOnStartup: true, trayNotifications: "off", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) }) }) @@ -143,6 +161,8 @@ describe("AppSettingsStore", () => { runAtStartup: true, openToTrayOnStartup: true, trayNotifications: "off", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) }) @@ -158,6 +178,8 @@ describe("AppSettingsStore", () => { runAtStartup: false, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) }) diff --git a/desktop/src/main/__tests__/cli-error-envelope.test.ts b/desktop/src/main/__tests__/cli-error-envelope.test.ts index 3f43e1aa6a..a714dc5300 100644 --- a/desktop/src/main/__tests__/cli-error-envelope.test.ts +++ b/desktop/src/main/__tests__/cli-error-envelope.test.ts @@ -1,12 +1,19 @@ import { describe, expect, it } from "vitest" import { + cliErrorFromEnvelope, + cliErrorFromLegacy, normalizeOperationStatus, parseCliEnvelope, type CliStatusEnvelope, } from "../../shared/cli-error" describe("CLI envelopes", () => { + it("validates direct error envelopes and rejects malformed fields", () => { + expect(cliErrorFromEnvelope({ kind: "error", outcome: "error", code: "X", message: "boom" })).toMatchObject({ code: "X", message: "boom" }) + expect(cliErrorFromEnvelope({ kind: "error", outcome: "error", message: "" })).toBeUndefined() + expect(cliErrorFromEnvelope({ kind: "error", outcome: "error", message: "boom", context: { attempt: 1 } })).toBeUndefined() + }) it("parses and normalizes current status", () => { const envelope = parseCliEnvelope( JSON.stringify({ kind: "status", schemaVersion: 1, phase: "building_image", state: "started" }), @@ -57,4 +64,18 @@ describe("CLI envelopes", () => { expect(parseCliEnvelope(JSON.stringify({ kind: "status", schemaVersion: 1, phase: "ready", state: "started", error: { message: "x", context: { attempt: 1 } } }))).toBeUndefined() expect(parseCliEnvelope(JSON.stringify({ kind: "diagnostic", message: "raw" }))).toBeUndefined() }) + + it("keeps legacy zap errors when context is a flattened string", () => { + const legacy = { + level: "error", + cliError: { code: "BUILD_FAILED", message: "build failed", context: "map[attempt:1]" }, + } + expect(cliErrorFromLegacy(legacy)).toMatchObject({ code: "BUILD_FAILED", message: "build failed" }) + expect(cliErrorFromLegacy(legacy)?.context).toBeUndefined() + const structured = { + level: "error", + cliError: { code: "BUILD_FAILED", message: "build failed", context: { attempt: "1" } }, + } + expect(cliErrorFromLegacy(structured)?.context).toEqual({ attempt: "1" }) + }) }) diff --git a/desktop/src/main/__tests__/cli.test.ts b/desktop/src/main/__tests__/cli.test.ts index 7448482f81..26c427bbcc 100644 --- a/desktop/src/main/__tests__/cli.test.ts +++ b/desktop/src/main/__tests__/cli.test.ts @@ -58,6 +58,70 @@ describe("CliRunner", () => { cli = new CliRunner("/usr/local/bin/devsy") }) + it("passes the configured level without overriding explicit CLI levels", async () => { + const mockExecFile = vi.mocked(execFile) as unknown as ReturnType + mockExecFile.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, callback: ExecCb) => { + callback(null, { stdout: "{}", stderr: "" }) + }, + ) + cli.setDiagnosticLogLevel("debug") + await cli.run(["workspace", "list"]) + expect(mockExecFile.mock.calls[0][1]).toContain("--log-level") + expect(mockExecFile.mock.calls[0][1]).toContain("debug") + await cli.run(["--log-level", "error", "workspace", "list"]) + const explicitArgs = mockExecFile.mock.calls[1][1] as string[] + expect(explicitArgs.filter((arg) => arg === "--log-level")).toHaveLength(1) + expect(explicitArgs).toContain("error") + }) + + it("owns protocol defaults and preserves explicit protocol arguments", async () => { + const mockExecFile = vi.mocked(execFile) as unknown as ReturnType + mockExecFile.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, callback: ExecCb) => { + callback(null, { stdout: "{}", stderr: "" }) + }, + ) + await cli.run(["workspace", "list", "--log-output", "text"]) + const args = mockExecFile.mock.calls[0][1] as string[] + expect(args).toContain("--result-format") + expect(args).toContain("json") + expect(args).toContain("--log-output") + expect(args).toContain("text") + expect(args.filter((arg) => arg === "--result-format")).toHaveLength(1) + expect(args.filter((arg) => arg === "--log-output")).toHaveLength(1) + }) + + it("rejects non-JSON result formats on run before executing", async () => { + const mockExecFile = vi.mocked(execFile) as unknown as ReturnType + await expect( + cli.run(["workspace", "list", "--result-format=plain"]), + ).rejects.toThrow("Use runRaw() for non-JSON output") + expect(mockExecFile).not.toHaveBeenCalled() + }) + + it("recognizes the legacy log-format alias as an explicit protocol argument", async () => { + const mockExecFile = vi.mocked(execFile) as unknown as ReturnType + mockExecFile.mockImplementation( + (_cmd: string, _args: string[], _opts: unknown, callback: ExecCb) => { + callback(null, { stdout: "{}", stderr: "" }) + }, + ) + await cli.run(["workspace", "list", "--log-format=logfmt"]) + const args = mockExecFile.mock.calls[0][1] as string[] + expect(args).toContain("--log-format=logfmt") + expect(args.filter((arg) => arg === "--log-output")).toHaveLength(0) + }) + + it("defaults desktop-launched CLI diagnostics to info", async () => { + const mockExecFile = vi.mocked(execFile) as unknown as ReturnType + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: unknown, callback: ExecCb) => { + callback(null, { stdout: "{}", stderr: "" }) + }) + await cli.run(["workspace", "list"]) + expect(mockExecFile.mock.calls[0][1]).toContain("info") + }) + describe("run", () => { it("parses JSON stdout and returns typed result", async () => { const mockExecFile = vi.mocked(execFile) as unknown as ReturnType< @@ -81,6 +145,8 @@ describe("CliRunner", () => { "workspace", "list", "--skip-pro", + "--log-level", + "info", "--result-format", "json", "--log-output", @@ -165,6 +231,21 @@ describe("CliRunner", () => { expect(rejection.cliError).toEqual(cliErrorPayload) expect(rejection.message).toBe(cliErrorPayload.message) }) + + it("extracts a direct error envelope independently of diagnostic level", async () => { + const mockExecFile = vi.mocked(execFile) as unknown as ReturnType + const envelope = { kind: "error", outcome: "error", code: "BROKEN", message: "failure", hint: "retry" } + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: unknown, callback: ExecCb) => { + const error = new Error("Command failed") as Error & { code: number; stderr: string } + error.code = 1 + error.stderr = JSON.stringify(envelope) + callback(error, { stdout: "", stderr: error.stderr }) + }) + cli.setDiagnosticLogLevel("error") + const rejection = await cli.run(["workspace", "up", "."]).catch((e) => e as Error & { cliError?: unknown }) + expect(rejection.message).toBe("failure") + expect(rejection.cliError).toEqual({ code: "BROKEN", message: "failure", hint: "retry", context: undefined }) + }) }) describe("runRaw", () => { @@ -202,7 +283,7 @@ describe("CliRunner", () => { await expect(promise).resolves.toBe("ok\n") expect(mockSpawn).toHaveBeenCalledWith( "/usr/local/bin/devsy", - ["secret", "set", "FOO", "--stdin", "--log-output", "json"], + ["secret", "set", "FOO", "--stdin", "--log-level", "info", "--log-output", "json"], expect.objectContaining({ env: expect.any(Object) }), ) }) @@ -239,6 +320,8 @@ describe("CliRunner", () => { [ "/tmp/mock.cjs", "list", + "--log-level", + "info", "--result-format", "json", "--log-output", @@ -278,6 +361,31 @@ describe("CliRunner", () => { await vi.waitFor(() => expect(onExit).toHaveBeenCalledTimes(1)) }) + + it("passes a direct error envelope to the exit callback", async () => { + const child = fakeStreamingChild() + const mockSpawn = vi.mocked(spawn) as unknown as ReturnType + mockSpawn.mockReturnValue(child) + + const onExit = vi.fn() + const onLine = vi.fn() + await cli.runStreaming(["workspace", "up", "."], onLine, onExit) + child.stderr.push(JSON.stringify({ + kind: "error", + outcome: "error", + code: "BROKEN", + message: "build failed", + }) + "\n") + child.stderr.push(null) + await vi.waitFor(() => expect(onLine).toHaveBeenCalledTimes(1)) + child.emit("close", 1) + + await vi.waitFor(() => expect(onExit).toHaveBeenCalledTimes(1)) + expect(onExit.mock.calls[0]).toEqual([ + 1, + { code: "BROKEN", message: "build failed", hint: undefined, context: undefined }, + ]) + }) }) describe("stripAnsi", () => { diff --git a/desktop/src/main/__tests__/logging.test.ts b/desktop/src/main/__tests__/logging.test.ts new file mode 100644 index 0000000000..ff99334ad4 --- /dev/null +++ b/desktop/src/main/__tests__/logging.test.ts @@ -0,0 +1,31 @@ +// @vitest-environment node +import { afterEach, describe, expect, it, vi } from "vitest" +import { getMainLogLevel, mainLog, setMainLogLevel } from "../logging.js" + +describe("main logger", () => { + afterEach(() => setMainLogLevel("info")) + + it("filters diagnostics without replacing global console methods", () => { + const originalInfo = console.info + setMainLogLevel("error") + expect(console.info).toBe(originalInfo) + const info = vi.spyOn(console, "info").mockImplementation(() => {}) + mainLog.info("hidden") + mainLog.error("visible") + expect(info).not.toHaveBeenCalled() + expect(getMainLogLevel()).toBe("error") + info.mockRestore() + expect(console.info).toBe(originalInfo) + }) + + it("emits trace diagnostics only at the trace level", () => { + const debug = vi.spyOn(console, "debug").mockImplementation(() => {}) + setMainLogLevel("debug") + mainLog.trace("hidden") + setMainLogLevel("trace") + mainLog.trace("visible") + expect(debug).toHaveBeenCalledTimes(1) + expect(debug).toHaveBeenCalledWith("visible") + debug.mockRestore() + }) +}) diff --git a/desktop/src/main/__tests__/settings-service.test.ts b/desktop/src/main/__tests__/settings-service.test.ts index cf9ce9fe75..c1c8eaf4f9 100644 --- a/desktop/src/main/__tests__/settings-service.test.ts +++ b/desktop/src/main/__tests__/settings-service.test.ts @@ -172,6 +172,8 @@ describe("SettingsService", () => { runAtStartup: true, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) expect(onChanged).not.toHaveBeenCalled() }) @@ -204,6 +206,8 @@ describe("SettingsService", () => { runAtStartup: false, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) expect(result.startup.status).toBe("disabled") }) @@ -227,12 +231,16 @@ describe("SettingsService", () => { runAtStartup: true, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) expect(result.startup.status).toBe("error") expect(store.get()).toEqual({ runAtStartup: true, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) expect(onChanged).toHaveBeenCalledTimes(1) expect(onChanged).toHaveBeenCalledWith({ @@ -263,6 +271,8 @@ describe("SettingsService", () => { runAtStartup: true, openToTrayOnStartup: true, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }) expect(result.startup).toMatchObject({ applied: false, enabled: true }) expect(store.get().runAtStartup).toBe(true) diff --git a/desktop/src/main/analytics.ts b/desktop/src/main/analytics.ts index a75b397079..37c3b15278 100644 --- a/desktop/src/main/analytics.ts +++ b/desktop/src/main/analytics.ts @@ -3,6 +3,7 @@ import { arch, homedir, platform } from "node:os" import { app } from "electron" import { PostHog } from "posthog-node" import { machineIdSync } from "./machine-id.js" +import { mainLog } from "./logging.js" declare const __DEVSY_POSTHOG_API_KEY__: string | undefined // `typeof` guard keeps the module loadable under vitest, which doesn't @@ -46,7 +47,7 @@ function isTelemetryDisabled(): boolean { export function initAnalytics(): void { if (isTelemetryDisabled()) return if (!DEVSY_POSTHOG_API_KEY) { - console.warn("[telemetry] analytics disabled: API key not configured") + mainLog.warn("[telemetry] analytics disabled: API key not configured") return } diff --git a/desktop/src/main/app-settings.ts b/desktop/src/main/app-settings.ts index 2a4b452c3a..c4cac9a9f4 100644 --- a/desktop/src/main/app-settings.ts +++ b/desktop/src/main/app-settings.ts @@ -1,18 +1,22 @@ import { readFileSync, renameSync, writeFileSync } from "node:fs" import type { AppSettings, + LogLevel, TrayNotificationLevel, } from "../shared/app-settings.js" -export type { AppSettings, TrayNotificationLevel } +export type { AppSettings, LogLevel, TrayNotificationLevel } export const DEFAULT_APP_SETTINGS: AppSettings = { runAtStartup: false, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", } const LEVELS: readonly TrayNotificationLevel[] = ["off", "failures", "all"] +const LOG_LEVELS: readonly LogLevel[] = ["error", "warn", "info", "debug", "trace"] export function normalizeAppSettings(raw: unknown): AppSettings { const input = (typeof raw === "object" && raw !== null ? raw : {}) as Record< @@ -26,7 +30,16 @@ export function normalizeAppSettings(raw: unknown): AppSettings { ) ? (input.trayNotifications as TrayNotificationLevel) : DEFAULT_APP_SETTINGS.trayNotifications - return { runAtStartup, openToTrayOnStartup, trayNotifications } + const legacy = LOG_LEVELS.includes(input.logLevel as LogLevel) + ? (input.logLevel as LogLevel) + : undefined + const desktopLogLevel = LOG_LEVELS.includes(input.desktopLogLevel as LogLevel) + ? (input.desktopLogLevel as LogLevel) + : (legacy ?? DEFAULT_APP_SETTINGS.desktopLogLevel) + const cliCaptureLogLevel = LOG_LEVELS.includes(input.cliCaptureLogLevel as LogLevel) + ? (input.cliCaptureLogLevel as LogLevel) + : (legacy ?? DEFAULT_APP_SETTINGS.cliCaptureLogLevel) + return { runAtStartup, openToTrayOnStartup, trayNotifications, desktopLogLevel, cliCaptureLogLevel } } export function patchAppSettings( @@ -57,6 +70,16 @@ export function sanitizeAppSettingsPatch(raw: unknown): Partial { throw new Error("trayNotifications must be off, failures, or all") patch.trayNotifications = input.trayNotifications as TrayNotificationLevel } + for (const key of ["desktopLogLevel", "cliCaptureLogLevel", "logLevel"] as const) { + if (key in input) { + if (!LOG_LEVELS.includes(input[key] as LogLevel)) + throw new Error(`${key} must be error, warn, info, debug, or trace`) + if (key === "logLevel") { + patch.desktopLogLevel = input[key] as LogLevel + patch.cliCaptureLogLevel = input[key] as LogLevel + } else patch[key] = input[key] as LogLevel + } + } return patch } diff --git a/desktop/src/main/cli.ts b/desktop/src/main/cli.ts index 62211905f9..a97cad9439 100644 --- a/desktop/src/main/cli.ts +++ b/desktop/src/main/cli.ts @@ -3,11 +3,17 @@ import { execFile as execFileCb, spawn } from "node:child_process" import { createInterface } from "node:readline" import type { Readable } from "node:stream" import { promisify } from "node:util" +import { + cliErrorFromEnvelope, + cliErrorFromLegacy, +} from "../shared/cli-error.js" import type { CLIError, CliLogLine } from "../shared/cli-error.js" import { getAnalyticsDistinctId } from "./analytics.js" const execFile = promisify(execFileCb) const MAX_CONCURRENT = 50 +type CliLogOutput = "json" | "logfmt" | "text" +type CliResultFormat = "auto" | "json" | "plain" function backpressureController(source: Readable): (r: void | Promise) => void { let pending = 0 @@ -32,6 +38,12 @@ export interface StreamLine { level?: "info" | "warn" | "error" } +export interface CliInvocationPolicy { + diagnosticLogLevel: import("../shared/app-settings.js").LogLevel + logOutput: CliLogOutput + resultFormat: CliResultFormat +} + /** * Coerce an arbitrary zap level string into the narrow set the IPC payload uses. * Unknown levels map to "info". @@ -59,7 +71,9 @@ function extractCliErrorFromStderr(stderr: string): CLIError | undefined { let found: CLIError | undefined for (const line of stderr.split(/\r?\n/)) { const parsed = parseStderrLine(line) - if (parsed?.cliError) found = parsed.cliError + const direct = cliErrorFromEnvelope(parsed) + const legacy = cliErrorFromLegacy(parsed) + if (direct ?? legacy) found = direct ?? legacy } return found } @@ -163,12 +177,26 @@ function buildEnv(): NodeJS.ProcessEnv { } } +function requestedResultFormat(args: string[]): string | undefined { + const index = args.findIndex( + (arg) => arg === "--result-format" || arg.startsWith("--result-format="), + ) + if (index === -1) return undefined + const arg = args[index] + return arg.includes("=") ? arg.split("=", 2)[1] : args[index + 1] +} + export class CliRunner { private execPath: string private prefixArgs: string[] private env: NodeJS.ProcessEnv private running = 0 private queue: Array<() => void> = [] + private policy: CliInvocationPolicy = { + diagnosticLogLevel: "info", + logOutput: "json", + resultFormat: "json", + } constructor(private binaryPath: string) { if (/\.[cm]?js$/.test(binaryPath)) { @@ -181,6 +209,34 @@ export class CliRunner { this.env = buildEnv() } + setDiagnosticLogLevel(level: import("../shared/app-settings.js").LogLevel): void { + this.policy.diagnosticLogLevel = level + } + + private argsWithLogLevel(args: string[]): string[] { + const explicit = args.some( + (arg) => arg === "--log-level" || arg.startsWith("--log-level="), + ) + return [ + ...this.prefixArgs, + ...args, + ...(explicit ? [] : ["--log-level", this.policy.diagnosticLogLevel]), + ] + } + + private argsWithProtocol(args: string[], includeResultFormat = false): string[] { + const fullArgs = this.argsWithLogLevel(args) + const hasFlag = (flag: string) => + fullArgs.some((arg) => arg === flag || arg.startsWith(`${flag}=`)) + if (includeResultFormat && !hasFlag("--result-format")) { + fullArgs.push("--result-format", this.policy.resultFormat) + } + if (!hasFlag("--log-output") && !hasFlag("--log-format")) { + fullArgs.push("--log-output", this.policy.logOutput) + } + return fullArgs + } + private acquire(): Promise { if (this.running < MAX_CONCURRENT) { this.running++ @@ -201,16 +257,15 @@ export class CliRunner { } async run(args: string[]): Promise { + const requestedFormat = requestedResultFormat(args) ?? this.policy.resultFormat + if (requestedFormat !== "json") { + throw new Error( + `run() parses JSON results; got --result-format=${requestedFormat}. Use runRaw() for non-JSON output`, + ) + } await this.acquire() try { - const fullArgs = [ - ...this.prefixArgs, - ...args, - "--result-format", - "json", - "--log-output", - "json", - ] + const fullArgs = this.argsWithProtocol(args, true) const { stdout } = await execFile(this.execPath, fullArgs, { env: this.env, }) @@ -227,7 +282,7 @@ export class CliRunner { try { const { stdout } = await execFile( this.execPath, - [...this.prefixArgs, ...args, "--log-output", "json"], + this.argsWithProtocol(args), { env: this.env }, ) return stdout @@ -243,7 +298,7 @@ export class CliRunner { await this.acquire() try { return await this.spawnWithStdin( - [...this.prefixArgs, ...args, "--log-output", "json"], + this.argsWithProtocol(args), input, ) } catch (error: unknown) { @@ -307,7 +362,7 @@ export class CliRunner { await this.acquire() const child = spawn( this.execPath, - [...this.prefixArgs, ...args, "--log-output", "json"], + this.argsWithProtocol(args), { env: this.env }, ) @@ -341,13 +396,13 @@ export class CliRunner { rl.on("line", (line) => { if (suppressCallbacks) return const parsed = parseStderrLine(line) - if (parsed?.cliError) { - lastCliError = parsed.cliError - } + const direct = cliErrorFromEnvelope(parsed) + const legacy = cliErrorFromLegacy(parsed) + if (direct ?? legacy) lastCliError = direct ?? legacy const meta: StreamLine = { raw: line, parsed, - cliError: parsed?.cliError, + cliError: direct ?? legacy, level: normalizeLevel(parsed?.level), } applyBackpressure(onLine(line, "stderr", meta)) diff --git a/desktop/src/main/image-catalog.ts b/desktop/src/main/image-catalog.ts index 608bc1e024..bfb926af51 100644 --- a/desktop/src/main/image-catalog.ts +++ b/desktop/src/main/image-catalog.ts @@ -6,6 +6,7 @@ import type { ImageCatalog, LoadCatalogResult, } from "../shared/image-catalog-types.js" +import { mainLog } from "./logging.js" const FETCH_TIMEOUT_MS = 10_000 @@ -67,7 +68,7 @@ async function readCache(cachePath: string): Promise { raw = await readFile(cachePath, "utf8") } catch (err) { if ((err as NodeJS.ErrnoException).code !== "ENOENT") { - console.warn("[image-catalog] failed to read cache:", err) + mainLog.warn("[image-catalog] failed to read cache:", err) } return null } @@ -77,7 +78,7 @@ async function readCache(cachePath: string): Promise { ? parsed : null } catch (err) { - console.warn("[image-catalog] corrupt cache file, ignoring:", err) + mainLog.warn("[image-catalog] corrupt cache file, ignoring:", err) return null } } @@ -89,7 +90,7 @@ async function readSeed(seedPath: string): Promise { if (!isImageCatalog(seed)) throw new Error("seed malformed") return seed } catch (err) { - console.error( + mainLog.error( "[image-catalog] bundled seed unreadable/corrupt (packaging bug):", err, ) @@ -122,14 +123,14 @@ export async function loadCatalog( const toWrite: CatalogCacheFile = { fetchedAt: Date.now(), catalog } await writeFile(opts.cachePath, JSON.stringify(toWrite)) } catch (err) { - console.warn( + mainLog.warn( "[image-catalog] failed to write cache (continuing with remote):", err, ) } return { catalog, origin: "remote" } } catch (err) { - console.warn("[image-catalog] remote fetch failed, using fallback:", err) + mainLog.warn("[image-catalog] remote fetch failed, using fallback:", err) if (cache) return { catalog: cache.catalog, origin: "cache" } return { catalog: await readSeed(opts.seedPath), origin: "seed" } } diff --git a/desktop/src/main/index.ts b/desktop/src/main/index.ts index 76f5c9e92c..b7c9de7995 100644 --- a/desktop/src/main/index.ts +++ b/desktop/src/main/index.ts @@ -17,6 +17,7 @@ import { shouldSuppressInitialWindow, } from "./launch-context.js" import { LogStore } from "./log-store.js" +import { mainLog, setMainLogLevel } from "./logging.js" import { MachineDiagnosticsManager } from "./machine-diagnostics-manager.js" import { MachineDiagnosticsStore } from "./machine-diagnostics-store.js" import { ProviderJobs } from "./provider-jobs.js" @@ -136,6 +137,12 @@ function createWindow(): void { } app.whenReady().then(async () => { + const appSettingsStore = new AppSettingsStore( + join(app.getPath("userData"), "app-settings.json"), + ) + appSettingsStore.load() + setMainLogLevel(appSettingsStore.get().desktopLogLevel) + initAnalytics() trackEvent("app_open") @@ -167,6 +174,7 @@ app.whenReady().then(async () => { ? CliRunner.resolveBinaryPath(process.resourcesPath) : CliRunner.resolveBinaryPath(join(__dirname, "../../resources"))) const cli = new CliRunner(binaryPath) + cli.setDiagnosticLogLevel(appSettingsStore.get().cliCaptureLogLevel) // Initialize log store. Logs live under ~/.devsy/desktop/logs/ — inside the // shared ~/.devsy root (no artifact sprawl) but outside the CLI-managed @@ -182,9 +190,9 @@ app.whenReady().then(async () => { ) try { const pruned = logStore.prune(30) - if (pruned > 0) console.log(`Pruned ${pruned} old log files`) + if (pruned > 0) mainLog.info(`Pruned ${pruned} old log files`) } catch (e) { - console.error("Failed to prune old logs:", e) + mainLog.error("Failed to prune old logs:", e) } // Initialize PTY manager @@ -218,16 +226,14 @@ app.whenReady().then(async () => { const providerJobs = new ProviderJobs() const workspaceJobs = new WorkspaceJobs() - const appSettingsStore = new AppSettingsStore( - join(app.getPath("userData"), "app-settings.json"), - ) - appSettingsStore.load() const autostartEnv = detectAutostartEnvironment() const settingsService = new SettingsService({ store: appSettingsStore, applyAutostart: (settings) => applyAutostart(settings, autostartEnv), currentAutostartEnabled: () => readAutostartEnabled(autostartEnv), onChanged: (result) => { + setMainLogLevel(result.settings.desktopLogLevel) + cli.setDiagnosticLogLevel(result.settings.cliCaptureLogLevel) appTray?.rebuildMenu() const win = mainWindow if (win && !win.isDestroyed()) { diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 721d8c94f8..68bfd08238 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -12,7 +12,7 @@ import { parseCliEnvelope, } from "../shared/cli-error.js" import { hashWorkspaceRef, trackEvent } from "./analytics.js" -import type { CliRunner } from "./cli.js" +import type { CliRunner, StreamLine } from "./cli.js" import { loadCatalog } from "./image-catalog.js" import type { LogStore } from "./log-store.js" import type { MachineDiagnosticsStore } from "./machine-diagnostics-store.js" @@ -40,6 +40,7 @@ import { type ProviderEntry, parseProviderEntries } from "./watcher.js" import { normalizeWorkspaceStatus } from "./workspace-status.js" import type { WorkspaceActivity } from "../shared/workspace-operation.js" import type { WorkspaceJobs } from "./workspace-jobs.js" +import { mainLog } from "./logging.js" const execFileAsync = promisify(execFile) @@ -121,6 +122,12 @@ function formatLogLine(line: string, level: "INFO" | "ERROR" = "INFO"): string { return `${new Date().toISOString()}\t${level}\t${line}` } +function displayCliLine(line: string, meta?: StreamLine): string { + return meta?.cliError + ? formatLogLine(meta.cliError.message, "ERROR") + : formatLogLine(line) +} + interface ProgressSink { line(formatted: string): boolean done( @@ -257,7 +264,7 @@ function createLogSink( await flush?.() } catch (error) { // Log persistence must not change a command's actual outcome. - console.warn("[workspace-operation] log flush failed", error) + mainLog.warn("[workspace-operation] log flush failed", error) } post(true, { message: finalLine, ...extra }) }, @@ -704,7 +711,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { return } } - const formatted = redactSensitiveText(formatLogLine(line)) + const formatted = redactSensitiveText(displayCliLine(line, meta)) win?.webContents.send("command-progress", { commandId: cmdId, message: formatted, @@ -1245,7 +1252,6 @@ export function registerIpcHandlers(deps: IpcDependencies): { provider?: string ide?: string ideLaunch?: "auto" | "headless" | "skip" - debug?: boolean workspaceFolder?: string devcontainer?: string prebuildRepository?: string @@ -1262,7 +1268,6 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (args.provider) cliArgs.push("--provider", args.provider) if (args.ide) cliArgs.push("--ide", args.ide) if (args.ideLaunch) cliArgs.push("--ide-launch", args.ideLaunch) - if (args.debug) cliArgs.push("--debug") if (args.workspaceFolder) cliArgs.push("--workspace-folder", args.workspaceFolder) if (args.devcontainer) cliArgs.push("--devcontainer", args.devcontainer) @@ -1277,7 +1282,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { const generation = workspaceJobs.start(wsId, activity, cmdId) const started = performance.now() const timing = (stage: string) => - console.debug( + mainLog.debug( `[workspace-operation] ${cmdId} ${stage} ${Math.round(performance.now() - started)}ms`, ) timing("accepted") @@ -1347,7 +1352,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { try { child = await cli.runStreaming( ["workspace", "task", "logs", taskId, "--follow"], - (line, stream) => { + (line, stream, meta) => { if ( signalledDone || suppressCallbacks || @@ -1377,7 +1382,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { return } - const formatted = formatLogLine(line) + const formatted = displayCliLine(line, meta) if (envelope?.kind === "result") { signalledDone = true @@ -1504,7 +1509,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { await sink .done(formatLogLine("Completed"), { success: true }) .catch((error) => - console.warn("[workspace-operation] log flush failed", error), + mainLog.warn("[workspace-operation] log flush failed", error), ) await workspaceJobs.finish(wsId, generation) return @@ -1561,7 +1566,6 @@ export function registerIpcHandlers(deps: IpcDependencies): { type WorkspaceActionSource = "renderer" | "tray" type StopWorkspaceArgs = { workspaceId: string - debug?: boolean commandId?: string } @@ -1579,7 +1583,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { ) const started = performance.now() const timing = (stage: string) => - console.debug( + mainLog.debug( `[workspace-operation] ${commandId} ${stage} ${Math.round(performance.now() - started)}ms`, ) trackEvent( @@ -1603,7 +1607,6 @@ export function registerIpcHandlers(deps: IpcDependencies): { (line) => logStore.appendLog(logPath, line), () => logStore.closeLog(logPath), ) - if (args.debug) cliArgs.push("--debug") workspaceJobs.phase(args.workspaceId, commandId, "Launching command") timing("cli-launch") let firstProgress = true @@ -1611,7 +1614,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { void cli .runStreaming( cliArgs, - (line, stream) => { + (line, stream, meta) => { if (!workspaceJobs.owns(args.workspaceId, commandId)) return if (firstProgress) { timing("first-progress") @@ -1626,7 +1629,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { ) ) return - if (!sink!.line(formatLogLine(line))) + if (!sink!.line(displayCliLine(line, meta))) return logStore.onDrain(logPath) }, (code, cliError) => { @@ -1649,7 +1652,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { await sink .done(formatLogLine("Completed"), { success: true }) .catch((error) => - console.warn("[workspace-operation] log flush failed", error), + mainLog.warn("[workspace-operation] log flush failed", error), ) await workspaceJobs.finish(args.workspaceId, generation) timing("reconciled") diff --git a/desktop/src/main/log-store.ts b/desktop/src/main/log-store.ts index a73d44e248..9480dae7ee 100644 --- a/desktop/src/main/log-store.ts +++ b/desktop/src/main/log-store.ts @@ -10,6 +10,7 @@ import { writeFileSync, } from "node:fs" import { basename, join } from "node:path" +import { mainLog } from "./logging.js" // safeLogFilename strips any directory components from filename so a caller // can't traverse out of the per-workspace logs dir with "../" segments. @@ -76,7 +77,7 @@ export class LogStore { stream = createWriteStream(logPath, { flags: "a" }) stream.on("error", (err) => { if ((err as NodeJS.ErrnoException).code !== "ENOENT") { - console.error(`log write failed for ${logPath}:`, err) + mainLog.error(`log write failed for ${logPath}:`, err) } }) this.streams.set(logPath, stream) diff --git a/desktop/src/main/logging.ts b/desktop/src/main/logging.ts new file mode 100644 index 0000000000..4b8a320408 --- /dev/null +++ b/desktop/src/main/logging.ts @@ -0,0 +1,24 @@ +import type { LogLevel } from "../shared/app-settings.js" + +const rank: Record = { error: 0, warn: 1, info: 2, debug: 3, trace: 4 } +let current: LogLevel = "info" + +function enabled(level: LogLevel): boolean { + return rank[current] >= rank[level] +} + +export const mainLog = { + debug: (...args: unknown[]) => { if (enabled("debug")) console.debug(...args) }, + trace: (...args: unknown[]) => { if (enabled("trace")) console.debug(...args) }, + info: (...args: unknown[]) => { if (enabled("info")) console.info(...args) }, + warn: (...args: unknown[]) => { if (enabled("warn")) console.warn(...args) }, + error: (...args: unknown[]) => console.error(...args), +} + +export function setMainLogLevel(level: LogLevel | undefined): void { + current = level && level in rank ? level : "info" +} + +export function getMainLogLevel(): LogLevel { + return current +} diff --git a/desktop/src/main/state.ts b/desktop/src/main/state.ts index bb6a2ba9a8..69671c62ee 100644 --- a/desktop/src/main/state.ts +++ b/desktop/src/main/state.ts @@ -1,3 +1,5 @@ +import { mainLog } from "./logging.js" + export interface Workspace { id: string lastUsed?: string @@ -115,12 +117,12 @@ export class DaemonState { // watcher's first poll picks up the new workspace) the active context is // used instead of a hard-coded "default", so creation logs land alongside // the workspace they're describing rather than orphaned under `default/`. - // Both fallback branches emit a console.warn so log misrouting is visible. + // Both fallback branches emit a main warning so log misrouting is visible. workspaceContext(workspaceId: string): string { const ws = this.workspaces.get(workspaceId) if (!ws) { const active = this.currentContext() - console.warn( + mainLog.warn( `[state] workspaceContext: workspace ${workspaceId} not found in state; falling back to active context %q`, active, ) @@ -128,7 +130,7 @@ export class DaemonState { } if (typeof ws.context !== "string" || ws.context === "") { const active = this.currentContext() - console.warn( + mainLog.warn( `[state] workspaceContext: workspace ${workspaceId} has no context field; falling back to active context %q`, active, ) @@ -151,7 +153,7 @@ export class DaemonState { try { listener() } catch (error) { - console.error("[state] workspace listener failed:", error) + mainLog.error("[state] workspace listener failed:", error) } } } diff --git a/desktop/src/main/tray.ts b/desktop/src/main/tray.ts index 39f33f27dd..13fb63330b 100644 --- a/desktop/src/main/tray.ts +++ b/desktop/src/main/tray.ts @@ -7,6 +7,7 @@ import { workspaceJobLabel, } from "../shared/workspace-operation.js" import type { AppSettings } from "./app-settings.js" +import { mainLog } from "./logging.js" import type { DaemonState, Workspace } from "./state.js" import { getLastStatus, @@ -320,7 +321,7 @@ export class AppTray { toggleOpenToTray: () => this.deps.toggleOpenToTray(), installUpdate: () => void installUpdate().catch((error) => - console.warn("[tray] failed to install update:", error), + mainLog.warn("[tray] failed to install update:", error), ), quit: () => app.quit(), }, @@ -341,7 +342,7 @@ export class AppTray { try { await this.deps.startWorkspace(workspaceId) } catch (error) { - console.warn(`[tray] failed to start workspace ${workspaceId}:`, error) + mainLog.warn(`[tray] failed to start workspace ${workspaceId}:`, error) } finally { await this.refreshAfterAction(workspaceId) this.pendingStarts.delete(workspaceId) @@ -356,7 +357,7 @@ export class AppTray { try { await this.deps.stopWorkspace(workspaceId) } catch (error) { - console.warn(`[tray] failed to stop workspace ${workspaceId}:`, error) + mainLog.warn(`[tray] failed to stop workspace ${workspaceId}:`, error) } finally { await this.refreshAfterAction(workspaceId) this.pendingStops.delete(workspaceId) @@ -368,12 +369,12 @@ export class AppTray { try { await this.deps.refreshWorkspace(workspaceId) } catch (error) { - console.warn(`[tray] failed to refresh workspace ${workspaceId}:`, error) + mainLog.warn(`[tray] failed to refresh workspace ${workspaceId}:`, error) } try { await this.deps.refreshWorkspaces() } catch (error) { - console.warn("[tray] failed to refresh workspaces:", error) + mainLog.warn("[tray] failed to refresh workspaces:", error) } } diff --git a/desktop/src/main/updater.ts b/desktop/src/main/updater.ts index 29145bc47b..c1330a7251 100644 --- a/desktop/src/main/updater.ts +++ b/desktop/src/main/updater.ts @@ -5,6 +5,7 @@ import electronUpdater, { type AppUpdater } from "electron-updater" import semver from "semver" import { trackEvent } from "./analytics.js" import { clearAppQuitting, markAppQuitting } from "./app-lifecycle.js" +import { mainLog } from "./logging.js" export type ReleaseChannel = "stable" | "beta" @@ -93,7 +94,7 @@ export function logUpdateDecision(params: UpdateDecisionLog): void { `result=${params.result}`, params.error ? `error=${params.error}` : null, ].filter(Boolean) - console.info(parts.join(" ")) + mainLog.info(parts.join(" ")) } export interface UpdateProgress { @@ -173,7 +174,7 @@ function saveSettings(patch: PersistedSettings): void { writeFileSync(tmp, JSON.stringify({ ...current, ...patch })) renameSync(tmp, target) } catch (err) { - console.warn("[updater] failed to persist settings:", err) + mainLog.warn("[updater] failed to persist settings:", err) } } @@ -188,7 +189,7 @@ function getCurrentVersion(): string | null { const v = app.getVersion() return v || null } catch (err) { - console.error( + mainLog.error( "Auto-update: unable to read app version:", err instanceof Error ? err.message : String(err), ) @@ -229,7 +230,7 @@ function setStatus(status: UpdateStatus): void { try { listener(status) } catch (error) { - console.error("[updater] status listener failed:", error) + mainLog.error("[updater] status listener failed:", error) } } } @@ -515,7 +516,7 @@ export async function initAutoUpdater( currentVersion, code, }) - console.warn("Auto-update: channel manifest missing:", err.message) + mainLog.warn("Auto-update: channel manifest missing:", err.message) return } setStatus({ @@ -524,7 +525,7 @@ export async function initAutoUpdater( code, error: err.message, }) - console.error("Auto-update error:", err.message) + mainLog.error("Auto-update error:", err.message) }) const runBackgroundCheck = (): void => { @@ -534,7 +535,7 @@ export async function initAutoUpdater( checkInFlight = true void runUpdateCheck(autoUpdater) .catch((err: Error) => { - console.error("Update check failed:", err.message) + mainLog.error("Update check failed:", err.message) }) .finally(() => { checkInFlight = false @@ -687,7 +688,7 @@ export async function installUpdate(): Promise { code: "install-failed", error: message, }) - console.error("Update installation failed:", message) + mainLog.error("Update installation failed:", message) throw error } } diff --git a/desktop/src/main/watcher.ts b/desktop/src/main/watcher.ts index d51f9361e6..3ac3c71397 100644 --- a/desktop/src/main/watcher.ts +++ b/desktop/src/main/watcher.ts @@ -9,6 +9,7 @@ import type { ProviderJobs } from "./provider-jobs.js" import type { DaemonState } from "./state.js" import type { WorkspaceJobs } from "./workspace-jobs.js" import { normalizeWorkspaceStatus } from "./workspace-status.js" +import { mainLog } from "./logging.js" interface WatcherDeps { cli: CliRunner @@ -275,7 +276,7 @@ export class Watcher { } catch (error) { // Polling is best-effort, but retain diagnostics so a broken CLI or // daemon cannot make the desktop appear empty without explanation. - console.warn("[watcher] workspace poll failed", error) + mainLog.warn("[watcher] workspace poll failed", error) if (strict) throw error } } @@ -327,7 +328,7 @@ export class Watcher { this.broadcastProviders() } } catch (error) { - console.warn("[watcher] provider poll failed", error) + mainLog.warn("[watcher] provider poll failed", error) } } diff --git a/desktop/src/renderer/src/lib/components/workspace/WorkspaceWizard.svelte b/desktop/src/renderer/src/lib/components/workspace/WorkspaceWizard.svelte index d34fd1e5a1..640f5ccf25 100644 --- a/desktop/src/renderer/src/lib/components/workspace/WorkspaceWizard.svelte +++ b/desktop/src/renderer/src/lib/components/workspace/WorkspaceWizard.svelte @@ -665,7 +665,6 @@ async function handleLaunch(recovery = false) { imageIncompatible && emulationEnabled && emulationTarget ? emulationTarget : undefined, - debug: loadLocalOptions().debugFlag, recovery, }) diff --git a/desktop/src/renderer/src/lib/ipc/commands.ts b/desktop/src/renderer/src/lib/ipc/commands.ts index 010de7774f..cdab0d0dfe 100644 --- a/desktop/src/renderer/src/lib/ipc/commands.ts +++ b/desktop/src/renderer/src/lib/ipc/commands.ts @@ -54,7 +54,6 @@ export async function workspaceUp(params: { provider?: string ide?: string ideLaunch?: "auto" | "headless" | "skip" - debug?: boolean workspaceFolder?: string devcontainer?: string prebuildRepository?: string @@ -67,34 +66,30 @@ export async function workspaceUp(params: { export async function workspaceStop( workspaceId: string, - debug?: boolean, commandId?: string, ): Promise { - return invoke("workspace_stop", { workspaceId, debug, commandId }) + return invoke("workspace_stop", { workspaceId, commandId }) } export async function workspaceDelete( workspaceId: string, - debug?: boolean, commandId?: string, ): Promise { - return invoke("workspace_delete", { workspaceId, debug, commandId }) + return invoke("workspace_delete", { workspaceId, commandId }) } export async function workspaceRebuild( workspaceId: string, - debug?: boolean, commandId?: string, ): Promise { - return invoke("workspace_rebuild", { workspaceId, debug, commandId }) + return invoke("workspace_rebuild", { workspaceId, commandId }) } export async function workspaceReset( workspaceId: string, - debug?: boolean, commandId?: string, ): Promise { - return invoke("workspace_reset", { workspaceId, debug, commandId }) + return invoke("workspace_reset", { workspaceId, commandId }) } export async function workspaceStatus( diff --git a/desktop/src/renderer/src/lib/ipc/mock.ts b/desktop/src/renderer/src/lib/ipc/mock.ts index 1f978df90c..d415a0d971 100644 --- a/desktop/src/renderer/src/lib/ipc/mock.ts +++ b/desktop/src/renderer/src/lib/ipc/mock.ts @@ -112,6 +112,8 @@ let MOCK_APP_SETTINGS = { runAtStartup: false, openToTrayOnStartup: false, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }, startup: { applied: true, enabled: false, status: "disabled" }, } as { @@ -119,6 +121,8 @@ let MOCK_APP_SETTINGS = { runAtStartup: boolean openToTrayOnStartup: boolean trayNotifications: "off" | "failures" | "all" + desktopLogLevel: "error" | "warn" | "info" | "debug" | "trace" + cliCaptureLogLevel: "error" | "warn" | "info" | "debug" | "trace" } startup: { applied: boolean diff --git a/desktop/src/renderer/src/lib/stores/desktop-settings.test.ts b/desktop/src/renderer/src/lib/stores/desktop-settings.test.ts index 3d2ebc8135..31881c318c 100644 --- a/desktop/src/renderer/src/lib/stores/desktop-settings.test.ts +++ b/desktop/src/renderer/src/lib/stores/desktop-settings.test.ts @@ -33,6 +33,8 @@ function appState(enabled: boolean): AppSettingsState { runAtStartup: enabled, openToTrayOnStartup: true, trayNotifications: "failures", + desktopLogLevel: "info", + cliCaptureLogLevel: "info", }, startup: { applied: true, diff --git a/desktop/src/renderer/src/lib/stores/settings.ts b/desktop/src/renderer/src/lib/stores/settings.ts index 44bf0db272..b1f82dee21 100644 --- a/desktop/src/renderer/src/lib/stores/settings.ts +++ b/desktop/src/renderer/src/lib/stores/settings.ts @@ -9,6 +9,7 @@ import { onAppSettingsChanged } from "$lib/ipc/events.js" import type { AppSettings, AppSettingsState, + LogLevel, StartupStatus, TrayNotificationLevel, } from "$shared/app-settings.js" @@ -231,7 +232,6 @@ export interface ContextOptions { export type OnBuildFailure = "prompt" | "auto-recovery" | "nothing" export interface LocalOptions { - debugFlag: boolean sshKeyPath: string httpProxy: string httpsProxy: string @@ -263,7 +263,6 @@ export const DEFAULT_CONTEXT_OPTIONS: ContextOptions = { } export const DEFAULT_LOCAL_OPTIONS: LocalOptions = { - debugFlag: false, sshKeyPath: "", httpProxy: "", httpsProxy: "", @@ -366,6 +365,8 @@ export function parseContextOptions( export const runAtStartup = writable(false) export const openToTrayOnStartup = writable(false) export const trayNotifications = writable("failures") +export const desktopLogLevel = writable("info") +export const cliCaptureLogLevel = writable("info") export const startupStatus = writable(null) let desktopSettingsQueue: Promise = Promise.resolve() @@ -375,6 +376,8 @@ function applyAppSettingsState(state: AppSettingsState): void { runAtStartup.set(state.settings.runAtStartup) openToTrayOnStartup.set(state.settings.openToTrayOnStartup) trayNotifications.set(state.settings.trayNotifications) + desktopLogLevel.set(state.settings.desktopLogLevel) + cliCaptureLogLevel.set(state.settings.cliCaptureLogLevel) startupStatus.set(state.startup) } diff --git a/desktop/src/renderer/src/pages/SettingsPage.svelte b/desktop/src/renderer/src/pages/SettingsPage.svelte index 6bfc04690a..b9263ad641 100644 --- a/desktop/src/renderer/src/pages/SettingsPage.svelte +++ b/desktop/src/renderer/src/pages/SettingsPage.svelte @@ -22,6 +22,8 @@ import { runAtStartup, openToTrayOnStartup, trayNotifications, + desktopLogLevel, + cliCaptureLogLevel, startupStatus, syncDesktopSettingsFromMain, updateDesktopSettings, @@ -32,7 +34,7 @@ import type { LocalOptions, OnBuildFailure, } from "$lib/stores/settings.js" -import type { TrayNotificationLevel } from "$shared/app-settings.js" +import type { LogLevel, TrayNotificationLevel } from "$shared/app-settings.js" import * as Select from "$lib/components/ui/select/index.js" import UpdatesPanel from "$lib/components/update/UpdatesPanel.svelte" import { Skeleton } from "$lib/components/ui/skeleton/index.js" @@ -104,7 +106,6 @@ let filteredIdes = $derived( ) let local = $state({ - debugFlag: false, sshKeyPath: "", httpProxy: "", httpsProxy: "", @@ -135,6 +136,14 @@ const NOTIFICATION_OPTIONS: { value: TrayNotificationLevel; label: string }[] = { value: "all", label: "All terminal outcomes" }, ] +const LOG_LEVEL_OPTIONS: { value: LogLevel; label: string }[] = [ + { value: "error", label: "Error" }, + { value: "warn", label: "Warn" }, + { value: "info", label: "Info" }, + { value: "debug", label: "Debug" }, + { value: "trace", label: "Trace" }, +] + onMount(() => { local = loadLocalOptions() localOptionsStore.set(local) @@ -190,11 +199,34 @@ function toggleLocal(key: keyof LocalOptions) {
- -

Run all commands with --debug flag

+ +

Controls diagnostic detail emitted by the desktop application

+
+ { if (v) updateDesktopSettings({ desktopLogLevel: v as LogLevel }) }} + > + {LOG_LEVEL_OPTIONS.find((o) => o.value === $desktopLogLevel)?.label ?? "Info"} + + {#each LOG_LEVEL_OPTIONS as o (o.value)} + + {/each} + + +
+ +
+
+ +

Controls diagnostic detail captured from CLI commands launched by the desktop

- toggleLocal("debugFlag")} disabled={loading || saving} /> + { if (v) updateDesktopSettings({ cliCaptureLogLevel: v as LogLevel }) }}> + {LOG_LEVEL_OPTIONS.find((o) => o.value === $cliCaptureLogLevel)?.label ?? "Info"} + {#each LOG_LEVEL_OPTIONS as o (o.value)}{/each} +
+

Direct terminal CLI commands use their own context default, which is Warn unless overridden.

diff --git a/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte b/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte index e5314a7732..8bfc02529e 100644 --- a/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte +++ b/desktop/src/renderer/src/pages/WorkspaceDetailPage.svelte @@ -439,10 +439,6 @@ function handleGpgForwardFailed(reason: string) { ) } -function isDebug(): boolean { - return loadLocalOptions().debugFlag -} - function startStreamingOp(label: string): string { const newCmdId = crypto.randomUUID() commandId = newCmdId @@ -468,7 +464,6 @@ async function handleStart() { await workspaceUp({ source: id, ide, - debug: isDebug(), workspaceFolder: folder, commandId: cmdId, }) @@ -506,7 +501,6 @@ async function handleRecovery() { source: id, ide, recovery: true, - debug: isDebug(), workspaceFolder: folder, commandId: cmdId, }) @@ -528,7 +522,6 @@ async function handleOpenIde() { source: id, ide, ideLaunch: "auto", - debug: isDebug(), workspaceFolder: folder, commandId: cmdId, }) @@ -541,7 +534,7 @@ async function handleOpenIde() { async function handleStop() { const cmdId = startStreamingOp("Stop") try { - await workspaceStop(id, isDebug(), cmdId) + await workspaceStop(id, cmdId) } catch (err) { awaitingAcceptance = false toasts.error(`Failed to stop: ${extractErrorMessage(err)}`) @@ -552,7 +545,7 @@ async function handleRebuild() { confirmRebuildOpen = false const cmdId = startStreamingOp("Rebuild") try { - await workspaceRebuild(id, isDebug(), cmdId) + await workspaceRebuild(id, cmdId) } catch (err) { awaitingAcceptance = false toasts.error(`Failed to rebuild: ${extractErrorMessage(err)}`) @@ -563,7 +556,7 @@ async function handleReset() { confirmResetOpen = false const cmdId = startStreamingOp("Reset") try { - await workspaceReset(id, isDebug(), cmdId) + await workspaceReset(id, cmdId) } catch (err) { awaitingAcceptance = false toasts.error(`Failed to reset: ${extractErrorMessage(err)}`) @@ -575,7 +568,7 @@ async function handleDelete() { const cmdId = startStreamingOp("Delete") deleting = true try { - await workspaceDelete(id, isDebug(), cmdId) + await workspaceDelete(id, cmdId) } catch (err) { awaitingAcceptance = false deleting = false diff --git a/desktop/src/shared/app-settings.ts b/desktop/src/shared/app-settings.ts index bbc9642c5e..ad8443861c 100644 --- a/desktop/src/shared/app-settings.ts +++ b/desktop/src/shared/app-settings.ts @@ -1,9 +1,14 @@ export type TrayNotificationLevel = "off" | "failures" | "all" +export type LogLevel = "error" | "warn" | "info" | "debug" | "trace" export interface AppSettings { runAtStartup: boolean openToTrayOnStartup: boolean trayNotifications: TrayNotificationLevel + desktopLogLevel: LogLevel + cliCaptureLogLevel: LogLevel + /** Removed after migration; accepted only when reading older settings files. */ + logLevel?: LogLevel } export interface StartupStatus { diff --git a/desktop/src/shared/cli-error.ts b/desktop/src/shared/cli-error.ts index 306d2f2723..7401c6a85b 100644 --- a/desktop/src/shared/cli-error.ts +++ b/desktop/src/shared/cli-error.ts @@ -62,6 +62,46 @@ export interface CliErrorEnvelope { context?: Record } +export function cliErrorFromEnvelope(value: unknown): CLIError | undefined { + if (!value || typeof value !== "object" || Array.isArray(value)) return undefined + const candidate = value as Record + if (candidate.kind !== "error" || candidate.outcome !== "error") return undefined + if (typeof candidate.message !== "string" || candidate.message.length === 0) return undefined + if (candidate.code !== undefined && typeof candidate.code !== "string") return undefined + if (candidate.hint !== undefined && typeof candidate.hint !== "string") return undefined + if (candidate.context !== undefined && !isStringMap(candidate.context)) return undefined + return { + code: typeof candidate.code === "string" ? candidate.code : "UNKNOWN", + message: candidate.message, + hint: typeof candidate.hint === "string" ? candidate.hint : undefined, + context: candidate.context as Record | undefined, + } +} + +export function cliErrorFromLegacy(value: unknown): CLIError | undefined { + if (!value || typeof value !== "object" || Array.isArray(value)) return undefined + const candidate = value as Record + if (candidate.level !== "error" && candidate.level !== "fatal" && candidate.level !== "panic") return undefined + if (!isCLIError(candidate.cliError)) return undefined + // Legacy zap encoding flattens context to a string; drop it rather than discard the error. + const err = candidate.cliError + return isStringMap(err.context) || err.context === undefined ? err : { ...err, context: undefined } +} + +function isCLIError(value: unknown): value is CLIError { + if (!value || typeof value !== "object" || Array.isArray(value)) return false + const candidate = value as Record + return typeof candidate.code === "string" && typeof candidate.message === "string" && + (candidate.hint === undefined || typeof candidate.hint === "string") && + (candidate.context === undefined || typeof candidate.context === "string" || + isStringMap(candidate.context)) +} + +function isStringMap(value: unknown): value is Record { + return !!value && typeof value === "object" && !Array.isArray(value) && + Object.values(value).every((entry) => typeof entry === "string") +} + export type CliEnvelope = | CliStatusEnvelope | CliResultEnvelope @@ -85,6 +125,7 @@ export function parseCliEnvelope(line: string): CliEnvelope | undefined { return undefined } } + if ((obj as { kind: string }).kind === "error" && !cliErrorFromEnvelope(obj)) return undefined return obj as CliEnvelope } } catch { diff --git a/e2e/tests/build/build.go b/e2e/tests/build/build.go index 8c48082fc6..076b865069 100644 --- a/e2e/tests/build/build.go +++ b/e2e/tests/build/build.go @@ -223,6 +223,9 @@ var _ = ginkgo.Describe("devsy build test suite", ginkgo.Label("build"), ginkgo. ginkgo.SpecTimeout(framework.TimeoutShort()), func(ctx context.Context) { f := framework.NewDefaultFramework(initialDir + "/bin") + // This fixture uses a local no-op feature so it exercises feature + // installation without depending on distro package mirrors. Package + // upgrades are unrelated to the compose build-context behavior here. tempDir, err := framework.CopyToTempDir( "tests/build/testdata/docker-compose-features-context", ) diff --git a/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/devcontainer.json b/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/devcontainer.json index 88c60d6e6c..9f72e3fcad 100644 --- a/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/devcontainer.json +++ b/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/devcontainer.json @@ -4,21 +4,10 @@ "service": "example-dc", "workspaceFolder": "/workspace", "features": { - "ghcr.io/devcontainers/features/common-utils:2": { - "installZsh": true, - "configureZshAsDefaultShell": true, - "upgradePackages": false, - "installOhMyZsh": false, - "installOhMyZshConfig": false, - "username": "example", - "userUid": "automatic", - "userGid": "automatic" - } + "./features/context-build": {} }, "containerEnv": { "LANG": "en_US.UTF-8", "LANGUAGE": "en_US:en" - }, - "remoteUser": "example", - "containerUser": "example" + } } diff --git a/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/features/context-build/devcontainer-feature.json b/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/features/context-build/devcontainer-feature.json new file mode 100644 index 0000000000..7ee35c03ce --- /dev/null +++ b/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/features/context-build/devcontainer-feature.json @@ -0,0 +1,5 @@ +{ + "id": "context-build", + "version": "1.0.0", + "name": "Context Build Feature" +} diff --git a/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/features/context-build/install.sh b/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/features/context-build/install.sh new file mode 100755 index 0000000000..e67f952a8e --- /dev/null +++ b/e2e/tests/build/testdata/docker-compose-features-context/.devcontainer/features/context-build/install.sh @@ -0,0 +1,5 @@ +#!/bin/sh +set -eu + +printf '%s\n' 'context-build-feature-installed' >/usr/local/bin/context-build-feature +chmod +x /usr/local/bin/context-build-feature diff --git a/e2e/tests/build/testdata/docker-compose-features-context/Dockerfile b/e2e/tests/build/testdata/docker-compose-features-context/Dockerfile index 1137a4ff6d..7f6a8ad0c7 100644 --- a/e2e/tests/build/testdata/docker-compose-features-context/Dockerfile +++ b/e2e/tests/build/testdata/docker-compose-features-context/Dockerfile @@ -1,7 +1,4 @@ -FROM ghcr.io/devsy-org/test-images/python:latest - -ENV PYTHONDONTWRITEBYTECODE=1 \ - PYTHONUNBUFFERED=1 +FROM ghcr.io/devsy-org/test-images/base:alpine WORKDIR /example diff --git a/pkg/config/context.go b/pkg/config/context.go index 453c26a746..d63ef53321 100644 --- a/pkg/config/context.go +++ b/pkg/config/context.go @@ -26,6 +26,7 @@ const ( ContextOptionSSHTunnelMode = "SSH_TUNNEL_MODE" ContextOptionSecretsBackend = "SECRETS_BACKEND" ContextOptionSnapshotRegistry = "SNAPSHOT_REGISTRY" + ContextOptionLogLevel = "LOG_LEVEL" ) var ContextOptions = []ContextOption{ @@ -131,6 +132,12 @@ var ContextOptions = []ContextOption{ Description: "Specifies the default registry to push/pull workspace snapshots to, e.g. ghcr.io/acme/snapshots", Default: "", }, + { + Name: ContextOptionLogLevel, + Description: "Default log level for Devsy commands (error, warn, info, debug, or trace)", + Default: "warn", + Enum: []string{"error", "warn", "info", "debug", "trace"}, + }, } func MergeContextOptions(contextConfig *ContextConfig, environ []string) { diff --git a/pkg/config/context_test.go b/pkg/config/context_test.go index 769121c4b2..f9c13b2dfb 100644 --- a/pkg/config/context_test.go +++ b/pkg/config/context_test.go @@ -90,3 +90,18 @@ func TestSnapshotRegistryOption_Registered(t *testing.T) { } require.True(t, found, "SNAPSHOT_REGISTRY must be a registered context option") } + +func TestLogLevelOption_DefaultAndEnum(t *testing.T) { + c := &Config{Contexts: map[string]*ContextConfig{"default": {}}, DefaultContext: "default"} + require.Equal(t, "warn", c.ContextOption(ContextOptionLogLevel)) + + for _, option := range ContextOptions { + if option.Name == ContextOptionLogLevel { + require.Equal(t, []string{ + "error", "warn", "info", "debug", "trace", + }, option.Enum) + return + } + } + t.Fatal("LOG_LEVEL must be a registered context option") +} diff --git a/pkg/flags/names/names.go b/pkg/flags/names/names.go index 375472e02c..fd97ef6dd0 100644 --- a/pkg/flags/names/names.go +++ b/pkg/flags/names/names.go @@ -203,6 +203,7 @@ const ( Workdir = "workdir" Home = "home" LogOutput = "log-output" + LogLevel = "log-level" LogFormat = "log-format" ResultFormat = "result-format" SetupInfo = "setup-info" diff --git a/pkg/log/levels.go b/pkg/log/levels.go index 55359ae699..9231b01b83 100644 --- a/pkg/log/levels.go +++ b/pkg/log/levels.go @@ -2,6 +2,43 @@ package log import "go.uber.org/zap/zapcore" +const DefaultLevel = LevelWarnName + +const ( + LevelErrorName = "error" + LevelWarnName = "warn" + LevelInfoName = "info" + LevelDebugName = "debug" + LevelTraceName = "trace" +) + +var validLevels = [...]string{ + LevelErrorName, + LevelWarnName, + LevelInfoName, + LevelDebugName, + LevelTraceName, +} + +func LevelFromString(value string) (zapcore.Level, bool) { + switch value { + case LevelErrorName: + return zapcore.ErrorLevel, true + case LevelWarnName: + return zapcore.WarnLevel, true + case LevelInfoName: + return zapcore.InfoLevel, true + case LevelDebugName: + return zapcore.DebugLevel, true + case LevelTraceName: + return zapcore.DebugLevel - 1, true + default: + return zapcore.ErrorLevel, false + } +} + +func ValidLevels() []string { return append([]string(nil), validLevels[:]...) } + // Verbosity levels mapped from CLI flags. const ( LevelTrace = 3 @@ -21,6 +58,9 @@ func DebugEnabled() bool { // (e.g. "debug", "info", "warn", "error", "fatal"). func LevelString() string { core := sugar.Load().Desugar().Core() + if core.Enabled(zapcore.DebugLevel - 1) { + return LevelTraceName + } for _, l := range []zapcore.Level{ zapcore.DebugLevel, zapcore.InfoLevel, diff --git a/pkg/log/logger.go b/pkg/log/logger.go index 918c8d2046..c83b94440c 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -6,7 +6,6 @@ import ( "sync" "sync/atomic" - "github.com/devsy-org/devsy/pkg/clierr" "github.com/devsy-org/devsy/pkg/secrets" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -26,11 +25,14 @@ func init() { // Config holds logger configuration parsed from CLI flags. type Config struct { - Verbosity int // 0=error, 1=info+warn, 2=debug, 3=trace - Quiet bool // fatal only - Debug bool // backwards compat, equivalent to Verbosity=2 - Format string // "text", "json", "logfmt" - Redactor *secrets.Redactor + Verbosity int // 0=error, 1=info+warn, 2=debug, 3=trace + Quiet bool // error only + Debug bool // backwards compat, equivalent to Verbosity=2 + Level string // explicit --log-level override + DefaultLevel string // persisted default when no explicit verbosity flag is set + VerbositySet bool + Format string // "text", "json", "logfmt" + Redactor *secrets.Redactor } // Init configures the global logger. Called once in root command PersistentPreRunE. @@ -145,13 +147,39 @@ func (f *writerFanout) add(w io.Writer) (remove func()) { } func resolveLevel(cfg Config) zapcore.Level { + if level, ok := resolveExplicitLevel(cfg); ok { + return level + } + return VerbosityToLevel(cfg.Verbosity) +} + +// Precedence: --quiet > --debug > explicitly supplied -v > --log-level > persisted default. +// This ordering is compatibility-sensitive: earlier flags must keep winning over +// later configuration, so do not reorder it. +func resolveExplicitLevel(cfg Config) (zapcore.Level, bool) { if cfg.Quiet { - return zapcore.FatalLevel + return zapcore.ErrorLevel, true } if cfg.Debug { - return zapcore.DebugLevel + return zapcore.DebugLevel, true } - return VerbosityToLevel(cfg.Verbosity) + if cfg.VerbositySet || cfg.Verbosity > 0 { + return VerbosityToLevel(cfg.Verbosity), true + } + return resolveConfiguredLevel(cfg.Level, cfg.DefaultLevel) +} + +func resolveConfiguredLevel(explicit, fallback string) (zapcore.Level, bool) { + if explicit != "" { + return LevelFromString(explicit) + } + if fallback != "" { + if level, ok := LevelFromString(fallback); ok { + return level, true + } + } + level, _ := LevelFromString(DefaultLevel) + return level, true } func resolveEncoder(format string) zapcore.Encoder { @@ -221,19 +249,6 @@ func Warn(args ...any) { sugar.Load().Warn(args...) } func Error(args ...any) { sugar.Load().Error(args...) } func Fatal(args ...any) { sugar.Load().Fatal(args...) } -func JSONError(cliErr *clierr.CLIError) { - if cliErr == nil { - return - } - msg := cliErr.Message - if wrapped := cliErr.Unwrap(); wrapped != nil { - if s := wrapped.Error(); s != "" { - msg = s - } - } - sugar.Load().Desugar().Error(msg, zap.Object("cliError", cliErr)) -} - func Debugw(msg string, keysAndValues ...any) { sugar.Load().Debugw(msg, keysAndValues...) } func Infow(msg string, keysAndValues ...any) { sugar.Load().Infow(msg, keysAndValues...) } func Warnw(msg string, keysAndValues ...any) { sugar.Load().Warnw(msg, keysAndValues...) } diff --git a/pkg/log/logger_test.go b/pkg/log/logger_test.go index 13f2d0ab6a..4e9e16c1e2 100644 --- a/pkg/log/logger_test.go +++ b/pkg/log/logger_test.go @@ -25,6 +25,140 @@ func TestColorEnabledHonorsNoColor(t *testing.T) { } } +func TestResolveLevelPrecedence(t *testing.T) { + tests := []struct { + name string + cfg Config + want string + }{ + {name: "persisted default", cfg: Config{DefaultLevel: LevelWarnName}, want: LevelWarnName}, + { + name: "explicit log level", + cfg: Config{Level: LevelDebugName, DefaultLevel: LevelWarnName}, want: LevelDebugName, + }, + { + name: "verbosity beats log level", + cfg: Config{ + Verbosity: 1, + VerbositySet: true, + Level: LevelDebugName, + }, + want: LevelInfoName, + }, + { + name: "debug beats verbosity", + cfg: Config{Verbosity: 1, VerbositySet: true, Debug: true}, want: LevelDebugName, + }, + { + name: "quiet beats debug", + cfg: Config{Debug: true, Quiet: true}, want: LevelErrorName, + }, + { + name: "log level beats persisted default", + cfg: Config{Level: LevelWarnName, DefaultLevel: LevelInfoName}, want: LevelWarnName, + }, + { + name: "explicit verbosity beats log level", + cfg: Config{ + Verbosity: 1, + VerbositySet: true, + Level: LevelErrorName, + }, + want: LevelInfoName, + }, + { + name: "debug beats log level", + cfg: Config{Debug: true, Level: LevelErrorName}, want: LevelDebugName, + }, + { + name: "quiet beats log level", + cfg: Config{Quiet: true, Level: LevelTraceName}, want: LevelErrorName, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := resolveLevel(tt.cfg).String(); got != tt.want { + t.Fatalf("resolveLevel() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestResolveLevelExplicitFalseValues(t *testing.T) { + tests := []struct { + name string + cfg Config + want string + }{ + { + name: "explicit debug=false falls through to log level", + cfg: Config{ + Debug: false, + Level: LevelDebugName, + DefaultLevel: LevelWarnName, + }, + want: LevelDebugName, + }, + { + name: "explicit quiet=false falls through to log level", + cfg: Config{ + Quiet: false, + Level: LevelInfoName, + DefaultLevel: LevelWarnName, + }, + want: LevelInfoName, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := resolveLevel(tt.cfg).String(); got != tt.want { + t.Fatalf("resolveLevel() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestResolveLevelDefaultsToWarn(t *testing.T) { + if got := resolveLevel(Config{DefaultLevel: DefaultLevel}).String(); got != LevelWarnName { + t.Fatalf("resolveLevel() = %q, want %q", got, LevelWarnName) + } +} + +func TestResolveLevelInvalidPersistedDefaultFallsBackToWarn(t *testing.T) { + if got := resolveLevel(Config{DefaultLevel: "verbose"}).String(); got != LevelWarnName { + t.Fatalf("resolveLevel() = %q, want %q", got, LevelWarnName) + } +} + +func TestQuietKeepsErrorsVisible(t *testing.T) { + Init(Config{Quiet: true}) + var sink syncBuffer + remove := AddSink(&sink) + defer remove() + + Warn("hidden warning") + Error("visible error") + _ = Sync() + + if got := sink.String(); strings.Contains(got, "hidden warning") { + t.Fatalf("quiet logger emitted warning: %q", got) + } + if got := sink.String(); !strings.Contains(got, "visible error") { + t.Fatalf("quiet logger hid error: %q", got) + } +} + +func TestLevelFromString(t *testing.T) { + for _, level := range ValidLevels() { + if _, ok := LevelFromString(level); !ok { + t.Errorf("LevelFromString(%q) rejected valid level", level) + } + } + if _, ok := LevelFromString("verbose"); ok { + t.Fatal("LevelFromString accepted invalid level") + } +} + func TestAddSink_ForwardsLogLines(t *testing.T) { Init(Config{Verbosity: 2}) // info+ diff --git a/pkg/log/streamer.go b/pkg/log/streamer.go index 706cdce959..07108ee700 100644 --- a/pkg/log/streamer.go +++ b/pkg/log/streamer.go @@ -214,7 +214,7 @@ const infoLevelName = "info" func normalizeZapLevel(raw string) (zapcore.Level, bool) { switch strings.ToLower(raw) { - case "trace", "debug": + case LevelTraceName, LevelDebugName: return zapcore.DebugLevel, true case infoLevelName: return zapcore.InfoLevel, true