diff --git a/Taskfile.yml b/Taskfile.yml index b9363951a..16445559c 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -80,8 +80,8 @@ tasks: dir: pkg/agent/tunnel cmd: | # sudo apt install protobuf-compiler - go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.5.1 - go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.4 + go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.2 + go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.11 protoc -I . tunnel.proto --go_out=. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative cli:test: diff --git a/cmd/provider/add.go b/cmd/provider/add.go index 54769db41..b45a4b2e2 100644 --- a/cmd/provider/add.go +++ b/cmd/provider/add.go @@ -3,6 +3,7 @@ package provider import ( "context" "fmt" + "os" "strings" "github.com/devsy-org/devsy/cmd/flags" @@ -11,6 +12,7 @@ import ( "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/provider" + "github.com/devsy-org/devsy/pkg/status" "github.com/devsy-org/devsy/pkg/types" "github.com/devsy-org/devsy/pkg/workspace" "github.com/spf13/cobra" @@ -85,18 +87,31 @@ func (cmd *AddCmd) Run(ctx context.Context, devsyConfig *config.Config, args []s return err } + reporter, err := newStatusReporter(cmd.ResultFormat, os.Stdout) + if err != nil { + return err + } + + status.Enter(reporter, status.PhaseInstallingProvider, providerName) providerConfig, options, err := cmd.resolveProviderConfig(ctx, devsyConfig, providerName, args) if err != nil { + status.Fail(reporter, status.PhaseInstallingProvider, err) return err } + status.Leave(reporter, status.PhaseInstallingProvider, providerConfig.Name) log.Infof("installed provider: providerName=%s", providerConfig.Name) if !cmd.Use { log.Infof("To initialize the provider, run: devsy provider init %s", providerConfig.Name) + // No PhaseReady: installed but not initialized. return nil } - return cmd.useProvider(ctx, devsyConfig, providerConfig, options) + if err := cmd.useProvider(ctx, devsyConfig, providerConfig, options, reporter); err != nil { + return err + } + status.Leave(reporter, status.PhaseReady, providerConfig.Name) + return nil } func validateOptionalProviderName(providerName string) error { @@ -158,6 +173,7 @@ func (cmd *AddCmd) useProvider( devsyConfig *config.Config, providerConfig *provider.ProviderConfig, options []string, + reporter status.Reporter, ) error { // First add: there are no prior user values to merge, so // DiscardPriorValues is moot. Set it explicitly so future readers @@ -168,6 +184,7 @@ func (cmd *AddCmd) useProvider( UserOptions: options, DiscardPriorValues: true, SingleMachine: &cmd.SingleMachine, + Reporter: reporter, }) if configureErr != nil { devsyConfig, err := config.LoadConfig(cmd.Context, "") diff --git a/cmd/provider/configure_shared.go b/cmd/provider/configure_shared.go index 14539f7c1..95444122f 100644 --- a/cmd/provider/configure_shared.go +++ b/cmd/provider/configure_shared.go @@ -10,6 +10,7 @@ import ( "github.com/devsy-org/devsy/pkg/log" options2 "github.com/devsy-org/devsy/pkg/options" provider2 "github.com/devsy-org/devsy/pkg/provider" + "github.com/devsy-org/devsy/pkg/status" ) // ProviderOptionsConfig parameterizes ConfigureProvider. @@ -32,6 +33,15 @@ type ProviderOptionsConfig struct { SkipInit bool SkipSubOptions bool SingleMachine *bool + + Reporter status.Reporter +} + +func (cfg ProviderOptionsConfig) reporter() status.Reporter { + if cfg.Reporter == nil { + return status.Nop() + } + return cfg.Reporter } func ConfigureProvider(ctx context.Context, cfg ProviderOptionsConfig) error { @@ -91,13 +101,18 @@ func configureProviderOptions( } // fill defaults + reporter := cfg.reporter() + status.Enter(reporter, status.PhaseResolvingOptions, cfg.Provider.Name) devsyConfig, err = options2.ResolveOptions( ctx, devsyConfig, cfg.Provider, options, cfg.SkipRequired, cfg.SkipSubOptions, cfg.SingleMachine, ) if err != nil { - return nil, fmt.Errorf("resolve options: %w", err) + err = fmt.Errorf("resolve options: %w", err) + status.Fail(reporter, status.PhaseResolvingOptions, err) + return nil, err } + status.Leave(reporter, status.PhaseResolvingOptions, cfg.Provider.Name) // run init command if !cfg.SkipInit { @@ -107,10 +122,13 @@ func configureProviderOptions( stderr := log.Writer(log.LevelError) defer func() { _ = stderr.Close() }() + status.Enter(reporter, status.PhaseRunningInit, cfg.Provider.Name) err = initProvider(ctx, devsyConfig, cfg.Provider, initIO{stdout: stdout, stderr: stderr}) if err != nil { + status.Fail(reporter, status.PhaseRunningInit, err) return nil, err } + status.Leave(reporter, status.PhaseRunningInit, cfg.Provider.Name) } return devsyConfig, nil diff --git a/cmd/provider/init.go b/cmd/provider/init.go index 857cf687e..0e75d281f 100644 --- a/cmd/provider/init.go +++ b/cmd/provider/init.go @@ -1,11 +1,14 @@ package provider import ( + "os" + "github.com/devsy-org/devsy/cmd/completion" "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/config" cliflags "github.com/devsy-org/devsy/pkg/flags" "github.com/devsy-org/devsy/pkg/flags/names" + "github.com/devsy-org/devsy/pkg/status" "github.com/devsy-org/devsy/pkg/workspace" "github.com/spf13/cobra" ) @@ -39,14 +42,23 @@ func NewInitCmd(f *flags.GlobalFlags) *cobra.Command { if err != nil { return err } - return ConfigureProvider(cobraCmd.Context(), ProviderOptionsConfig{ + reporter, err := newStatusReporter(cmd.ResultFormat, os.Stdout) + if err != nil { + return err + } + if err := ConfigureProvider(cobraCmd.Context(), ProviderOptionsConfig{ Provider: p.Config, ContextName: devsyConfig.DefaultContext, UserOptions: cmd.Options, DiscardPriorValues: cmd.Reset, SkipInit: cmd.SkipInit, SingleMachine: &cmd.SingleMachine, - }) + Reporter: reporter, + }); err != nil { + return err + } + status.Leave(reporter, status.PhaseReady, name) + return nil }, ValidArgsFunction: func( rootCmd *cobra.Command, diff --git a/cmd/provider/set_source.go b/cmd/provider/set_source.go index 149e4bd6b..732defcc5 100644 --- a/cmd/provider/set_source.go +++ b/cmd/provider/set_source.go @@ -3,12 +3,14 @@ package provider import ( "context" "fmt" + "os" "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/config" cliflags "github.com/devsy-org/devsy/pkg/flags" "github.com/devsy-org/devsy/pkg/flags/names" "github.com/devsy-org/devsy/pkg/log" + "github.com/devsy-org/devsy/pkg/status" "github.com/devsy-org/devsy/pkg/workspace" "github.com/spf13/cobra" ) @@ -67,14 +69,23 @@ func (cmd *SetSourceCmd) Run(ctx context.Context, devsyConfig *config.Config, ar providerSource = args[1] } + reporter, err := newStatusReporter(cmd.ResultFormat, os.Stdout) + if err != nil { + return err + } + + status.Enter(reporter, status.PhaseInstallingProvider, args[0]) providerConfig, err := workspace.UpdateProvider(ctx, devsyConfig, args[0], providerSource) if err != nil { + status.Fail(reporter, status.PhaseInstallingProvider, err) return err } + status.Leave(reporter, status.PhaseInstallingProvider, providerConfig.Name) log.Infof("updated provider: providerName=%s", providerConfig.Name) if !cmd.Use { log.Infof("To initialize the provider, run: devsy provider init %s", providerConfig.Name) + // No PhaseReady: not ready until a following `provider init` runs. return nil } @@ -85,11 +96,16 @@ func (cmd *SetSourceCmd) Run(ctx context.Context, devsyConfig *config.Config, ar Provider: providerConfig, ContextName: devsyConfig.DefaultContext, UserOptions: cmd.Options, + Reporter: reporter, }); err != nil { return fmt.Errorf("configure provider: %w", err) } - return writeDefaultProvider(cmd.Context, providerConfig.Name) + if err := writeDefaultProvider(cmd.Context, providerConfig.Name); err != nil { + return err + } + status.Leave(reporter, status.PhaseReady, providerConfig.Name) + return nil } func (cmd *SetSourceCmd) runPinVersion( diff --git a/cmd/provider/status.go b/cmd/provider/status.go new file mode 100644 index 000000000..17f35f366 --- /dev/null +++ b/cmd/provider/status.go @@ -0,0 +1,58 @@ +package provider + +import ( + "io" + + config2 "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/log" + "github.com/devsy-org/devsy/pkg/output" + "github.com/devsy-org/devsy/pkg/status" +) + +// newStatusReporter drives provider progress output: one NDJSON status line +// per phase transition in JSON mode, human-readable info lines otherwise. +func newStatusReporter(resultFormat string, out io.Writer) (status.Reporter, error) { + mode, err := output.ResolveMode(resultFormat) + if err != nil { + return nil, err + } + + var r status.Reporter = plainStatusReporter{} + if mode == output.ModeJSON { + r = &jsonStatusReporter{out: out} + } + return status.ForPipeline(r, status.PipelineProvider), nil +} + +type jsonStatusReporter struct { + out io.Writer +} + +func (r *jsonStatusReporter) Report(e status.Event) { + _ = config2.WriteStatusJSON(r.out, e) +} + +type plainStatusReporter struct{} + +func (plainStatusReporter) Report(e status.Event) { + switch { + case e.Phase == status.PhaseFailed: + log.Errorf("provider: phase %q failed: %s", e.Step, e.Err) + case e.Started: + log.Infof("provider: %s", phaseLabel(e.Phase)) + } +} + +var phaseLabels = map[status.Phase]string{ + status.PhaseInstallingProvider: "installing provider", + status.PhaseResolvingOptions: "resolving options", + status.PhaseRunningInit: "running provider init", + status.PhaseReady: "ready", +} + +func phaseLabel(p status.Phase) string { + if label, ok := phaseLabels[p]; ok { + return label + } + return string(p) +} diff --git a/cmd/workspace/task.go b/cmd/workspace/task.go index 06636a5b3..80ed6ae5b 100644 --- a/cmd/workspace/task.go +++ b/cmd/workspace/task.go @@ -185,7 +185,12 @@ func followTask(ctx context.Context, store *task.Store, opts followTaskOptions) ticker := time.NewTicker(opts.interval) defer ticker.Stop() + tailer := newLogTailer(opts.id) + defer tailer.flush(os.Stderr) + for { + tailer.poll(os.Stderr) + state, err := store.Get(opts.id) if err != nil { return err diff --git a/cmd/workspace/task_tail.go b/cmd/workspace/task_tail.go new file mode 100644 index 000000000..20fb4e30f --- /dev/null +++ b/cmd/workspace/task_tail.go @@ -0,0 +1,101 @@ +package workspace + +import ( + "bytes" + "encoding/json" + "io" + "os" + + "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/task" +) + +// logTailer streams a detached worker's captured stdout/stderr as it's written. +type logTailer struct { + path string + file *os.File + buf []byte +} + +func newLogTailer(taskID string) *logTailer { + path, err := config.DefaultPathManager().ProcessStreamsFile(task.WorkerProcessName(taskID)) + if err != nil { + return &logTailer{} + } + return &logTailer{path: path} +} + +// poll emits any output appended since the last call. Safe to call before +// the worker has created its streams file; it just retries next time. +func (t *logTailer) poll(w io.Writer) { + if t.path == "" { + return + } + if t.file == nil { + f, err := os.Open(t.path) // #nosec G304 -- path derived from our own task ID + if err != nil { + return + } + t.file = f + } + + buf := make([]byte, 32*1024) + for { + n, err := t.file.Read(buf) + if n > 0 { + t.emit(w, buf[:n]) + } + if err != nil { + return + } + } +} + +// flush polls one last time and emits any trailing partial line, then +// releases the file handle. Call once the task has reached a terminal state. +func (t *logTailer) flush(w io.Writer) { + t.poll(w) + if len(t.buf) > 0 { + t.writeLine(w, t.buf) + t.buf = nil + } + if t.file != nil { + _ = t.file.Close() + t.file = nil + } +} + +func (t *logTailer) emit(w io.Writer, chunk []byte) { + t.buf = append(t.buf, chunk...) + for { + i := bytes.IndexByte(t.buf, '\n') + if i < 0 { + return + } + line := t.buf[:i] + t.buf = t.buf[i+1:] + t.writeLine(w, line) + } +} + +func (t *logTailer) writeLine(w io.Writer, line []byte) { + if isEnvelopeLine(line) { + return + } + _, _ = w.Write(line) + _, _ = w.Write([]byte("\n")) +} + +func isEnvelopeLine(line []byte) bool { + trimmed := bytes.TrimSpace(line) + if len(trimmed) == 0 || trimmed[0] != '{' { + return false + } + var probe struct { + Kind string `json:"kind"` + } + if err := json.Unmarshal(trimmed, &probe); err != nil { + return false + } + return probe.Kind != "" +} diff --git a/cmd/workspace/task_tail_test.go b/cmd/workspace/task_tail_test.go new file mode 100644 index 000000000..8bbace129 --- /dev/null +++ b/cmd/workspace/task_tail_test.go @@ -0,0 +1,187 @@ +package workspace + +import ( + "bytes" + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/task" +) + +func newTestTailer(t *testing.T, taskID string) (*logTailer, string) { + t.Helper() + dir := t.TempDir() + config.SetPathManager(fakeRuntimeDirPathManager{dir: dir}) + t.Cleanup(config.ResetPathManager) + + tailer := newLogTailer(taskID) + if tailer.path == "" { + t.Fatal("newLogTailer produced an empty path") + } + return tailer, tailer.path +} + +type fakeRuntimeDirPathManager struct { + config.PathManager + dir string +} + +func (f fakeRuntimeDirPathManager) RuntimeDir() (string, error) { return f.dir, nil } + +func (f fakeRuntimeDirPathManager) ProcessStreamsFile(name string) (string, error) { + return filepath.Join(f.dir, name+".streams"), nil +} + +func TestLogTailerEmitsLinesAsTheyAreAppended(t *testing.T) { + tailer, path := newTestTailer(t, "task1") + var out bytes.Buffer + + tailer.poll(&out) // file doesn't exist yet + if out.Len() != 0 { + t.Fatalf("expected no output before the file exists, got %q", out.String()) + } + + if err := os.WriteFile(path, []byte("first line\n"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + tailer.poll(&out) + if out.String() != "first line\n" { + t.Fatalf("got %q, want %q", out.String(), "first line\n") + } + + // #nosec G304 -- path is t.TempDir()-derived + f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0o600) + if err != nil { + t.Fatalf("open for append: %v", err) + } + if _, err := f.WriteString("second line\n"); err != nil { + t.Fatalf("append: %v", err) + } + _ = f.Close() + + tailer.poll(&out) + if out.String() != "first line\nsecond line\n" { + t.Fatalf("got %q, want both lines", out.String()) + } +} + +func TestLogTailerSkipsStructuredEnvelopeLines(t *testing.T) { + tailer, path := newTestTailer(t, "task2") + content := `{"level":"debug","ts":"2026-01-01T00:00:00.000-0500","msg":"a debug line"} +{"kind":"status","phase":"building_image","started":true} +{"kind":"result","outcome":"success","containerId":"abc"} +plain unstructured line +` + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + var out bytes.Buffer + tailer.poll(&out) + + got := out.String() + if !bytes.Contains([]byte(got), []byte("a debug line")) { + t.Errorf("expected the debug line to pass through, got %q", got) + } + if !bytes.Contains([]byte(got), []byte("plain unstructured line")) { + t.Errorf("expected the unstructured line to pass through, got %q", got) + } + if bytes.Contains([]byte(got), []byte(`"kind"`)) { + t.Errorf("expected structured envelope lines to be filtered out, got %q", got) + } +} + +func TestLogTailerFlushEmitsTrailingPartialLine(t *testing.T) { + tailer, path := newTestTailer(t, "task3") + // No trailing newline: simulates reading mid-write, or the worker's + // final line never getting a newline before it exits. + if err := os.WriteFile(path, []byte("unterminated"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + var out bytes.Buffer + tailer.poll(&out) + if out.Len() != 0 { + t.Fatalf("expected the partial line to be buffered, not emitted yet, got %q", out.String()) + } + + tailer.flush(&out) + if out.String() != "unterminated\n" { + t.Fatalf("got %q, want %q", out.String(), "unterminated\n") + } +} + +func TestFollowTaskStreamsWorkerLogOutput(t *testing.T) { + dir := t.TempDir() + config.SetPathManager(fakeRuntimeDirPathManager{dir: dir}) + t.Cleanup(config.ResetPathManager) + + store, err := task.NewStoreAt(t.TempDir()) + if err != nil { + t.Fatalf("NewStoreAt: %v", err) + } + tk, err := store.Create(task.CreateOptions{Command: "up"}) + if err != nil { + t.Fatalf("Create: %v", err) + } + + streamsPath, err := config.DefaultPathManager(). + ProcessStreamsFile(task.WorkerProcessName(tk.ID())) + if err != nil { + t.Fatalf("ProcessStreamsFile: %v", err) + } + line := `{"level":"debug","ts":"2026-01-01T00:00:00.000-0500","msg":"worker debug line"}` + "\n" + if err := os.WriteFile(streamsPath, []byte(line), 0o600); err != nil { + t.Fatalf("write streams file: %v", err) + } + + go func() { + time.Sleep(30 * time.Millisecond) + _ = tk.Succeed(nil) + }() + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("Pipe: %v", err) + } + origStderr := os.Stderr + os.Stderr = w + followErr := followTask(context.Background(), store, followTaskOptions{ + id: tk.ID(), + interval: 10 * time.Millisecond, + emitJSON: true, + }) + os.Stderr = origStderr + _ = w.Close() + + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + + if followErr != nil { + t.Fatalf("followTask: %v", followErr) + } + if !strings.Contains(buf.String(), "worker debug line") { + t.Errorf("expected the worker's debug line on stderr, got %q", buf.String()) + } +} + +func TestLogTailerToleratesMissingRuntimeDir(t *testing.T) { + config.SetPathManager( + fakeRuntimeDirPathManager{dir: filepath.Join(t.TempDir(), "does-not-exist")}, + ) + t.Cleanup(config.ResetPathManager) + + tailer := newLogTailer("task4") + var out bytes.Buffer + tailer.poll(&out) + tailer.flush(&out) + if out.Len() != 0 { + t.Errorf("expected no output when the streams file never appears, got %q", out.String()) + } +} diff --git a/cmd/workspace/up/detach.go b/cmd/workspace/up/detach.go index 1ec04ea44..4f03ef550 100644 --- a/cmd/workspace/up/detach.go +++ b/cmd/workspace/up/detach.go @@ -53,7 +53,7 @@ func launchDetached(taskID string) error { } args := append(detachedArgs(os.Args[1:]), names.Flag(names.TaskID), taskID) - return command.StartBackground("devsy-up-"+taskID, func() (*exec.Cmd, error) { + return command.StartBackground(task.WorkerProcessName(taskID), func() (*exec.Cmd, error) { return &exec.Cmd{ Path: execPath, Args: append([]string{execPath}, args...), diff --git a/cmd/workspace/up/status.go b/cmd/workspace/up/status.go index c325aa70a..e0c38e9cb 100644 --- a/cmd/workspace/up/status.go +++ b/cmd/workspace/up/status.go @@ -11,10 +11,11 @@ import ( // newStatusReporter drives `up`'s progress output. func newStatusReporter(emitJSON bool, out io.Writer) status.Reporter { + var r status.Reporter = plainStatusReporter{} if emitJSON { - return &jsonStatusReporter{out: out} + r = &jsonStatusReporter{out: out} } - return plainStatusReporter{} + return status.ForPipeline(r, status.PipelineWorkspaceUp) } // jsonStatusReporter serializes write events. diff --git a/desktop/e2e/app.e2e.ts b/desktop/e2e/app.e2e.ts index 43ac75523..c0676021f 100644 --- a/desktop/e2e/app.e2e.ts +++ b/desktop/e2e/app.e2e.ts @@ -1,11 +1,12 @@ import type { ElectronApplication, Page } from "@playwright/test" import { expect, test } from "@playwright/test" -import { launchApp } from "./electron-app.js" +import { launchApp, resetMockState } from "./electron-app.js" let app: ElectronApplication let page: Page test.beforeAll(async () => { + resetMockState() ;({ app, page } = await launchApp()) }) diff --git a/desktop/e2e/fixtures/mock-devsy.cjs b/desktop/e2e/fixtures/mock-devsy.cjs index 58b826938..cd7200652 100755 --- a/desktop/e2e/fixtures/mock-devsy.cjs +++ b/desktop/e2e/fixtures/mock-devsy.cjs @@ -110,6 +110,18 @@ function out(data) { ) } +function providerStatus(phase, started, step) { + out( + JSON.stringify({ + kind: "status", + pipeline: "provider", + phase, + ...(step ? { step } : {}), + started, + }), + ) +} + // Parse a slice of args (positional + recognized flags) into a result object. function parseArgs(args) { const positional = [] @@ -395,14 +407,19 @@ function handleStop(args) { function handleDelete(args) { const { positional } = parseArgs(args) const wsId = positional[0] - out("Deleting workspace...") - out("Workspace deleted.") - const idx = state.workspaces.findIndex((w) => w.id === wsId) - if (idx !== -1) { - state.workspaces.splice(idx, 1) - saveState(state) - } - process.exit(0) + // *probe suffix delays deletion so lifecycle tests can observe in-flight state. + const deleteMs = /probe$/.test(wsId) ? 1500 : 0 + setTimeout(() => { + out("Deleting workspace...") + out("Workspace deleted.") + const latest = loadState() + const idx = latest.workspaces.findIndex((w) => w.id === wsId) + if (idx !== -1) { + latest.workspaces.splice(idx, 1) + saveState(latest) + } + process.exit(0) + }, deleteMs) } function handleRename(args) { @@ -489,8 +506,10 @@ if (cmd === "workspace") { process.stderr.write(`mock-devsy: unknown workspace subcommand '${verb}'\n`) process.exit(2) } + // No unconditional exit(0) here: handleDelete's setTimeout would get killed + // before firing. `return` still skips the non-workspace switch below. handler(rawArgs.slice(2)) - process.exit(0) + return } // Non-workspace top-level commands (preserved verbatim). @@ -530,8 +549,11 @@ switch (cmd) { case "add": { const provName = extra if (provName) { - for (const key of Object.keys(state.providers)) { - state.providers[key].default = false + const takesDefault = !rawArgs.includes("--use=false") + if (takesDefault) { + for (const key of Object.keys(state.providers)) { + state.providers[key].default = false + } } state.providers[provName] = { config: { @@ -544,11 +566,33 @@ switch (cmd) { optionGroups: [], }, state: { initialized: false }, - default: true, + default: takesDefault, } saveState(state) } - out("") + // No ready phase: install finished, init has not run. + providerStatus("installing_provider", true) + providerStatus("installing_provider", false, provName) + process.exit(0) + break + } + case "init": { + const provName = extra || providerFlag + providerStatus("resolving_options", true, provName) + providerStatus("resolving_options", false, provName) + providerStatus("running_init", true, provName) + // *probe suffix delays init so lifecycle tests can observe in-flight state. + const initMs = /probe$/.test(provName) ? 5000 : 150 + setTimeout(() => { + const latest = loadState() + if (provName && latest.providers[provName]) { + latest.providers[provName].state.initialized = true + saveState(latest) + } + providerStatus("running_init", false, provName) + providerStatus("ready", false, provName) + process.exit(0) + }, initMs) break } case "delete": { @@ -594,6 +638,17 @@ switch (cmd) { out("") break } + case "set-source": { + const provName = extra + if (provName && state.providers[provName]) { + state.providers[provName].state.initialized = false + saveState(state) + } + providerStatus("installing_provider", true, provName) + providerStatus("installing_provider", false, provName) + process.exit(0) + break + } case "versions": out([]) break diff --git a/desktop/e2e/providers.e2e.ts b/desktop/e2e/providers.e2e.ts index aed7e25b0..0ab4f029c 100644 --- a/desktop/e2e/providers.e2e.ts +++ b/desktop/e2e/providers.e2e.ts @@ -82,3 +82,95 @@ test.describe("Providers Page", () => { await expect(sheet).toContainText("docker") }) }) + +test.describe("Provider lifecycle badges", () => { + test("never shows 'not initialized' while add+init is in flight", async () => { + const main = page.locator('[data-slot="sidebar-inset"] main') + + await page.evaluate(async () => { + const api = ( + window as unknown as { + electronAPI: { + invoke: (c: string, a?: Record) => Promise + } + } + ).electronAPI + await api.invoke("provider_delete", { name: "lifecycleprobe" }) + await api.invoke("provider_add", { name: "lifecycleprobe" }) + void api.invoke("provider_init", { name: "lifecycleprobe" }) + }) + + try { + const card = main.locator("button", { hasText: "lifecycleprobe" }).first() + await expect(card).toBeVisible({ timeout: 10000 }) + + let sawBusy = false + let settled = false + const deadline = Date.now() + 8000 + while (Date.now() < deadline) { + const text = ((await card.textContent()) ?? "").toLowerCase() + expect(text).not.toContain("not initialized") + if (/installing|initializing/.test(text)) sawBusy = true + // Reached only once the busy label is replaced by the settled badge. + if (sawBusy && /(^|\s)initialized/.test(text)) { + settled = true + break + } + await page.waitForTimeout(100) + } + + expect(sawBusy, "expected a busy badge during install/init").toBe(true) + expect(settled, "expected the card to settle as initialized").toBe(true) + } finally { + // Mock CLI state is shared across specs, so don't leave this behind. + await page.evaluate(async () => { + await ( + window as unknown as { + electronAPI: { + invoke: (c: string, a?: Record) => Promise + } + } + ).electronAPI.invoke("provider_delete", { name: "lifecycleprobe" }) + }) + } + }) + + // An abandoned wizard (skip init, or close mid-flow) leaves the install's + // job open. Without an explicit release the card spins on "installing…" + // forever, since nothing else will ever finish that job. + test("releases the job when a provider is added but never initialized", async () => { + const main = page.locator('[data-slot="sidebar-inset"] main') + + const api = async (channel: string, args: Record) => + page.evaluate( + ([c, a]) => + ( + window as unknown as { + electronAPI: { + invoke: ( + c: string, + a?: Record, + ) => Promise + } + } + ).electronAPI.invoke(c as string, a as Record), + [channel, args] as const, + ) + + await api("provider_delete", { name: "skipprobe" }) + await api("provider_add", { name: "skipprobe" }) + + try { + const card = main.locator("button", { hasText: "skipprobe" }).first() + await expect(card).toContainText(/installing/i, { timeout: 10000 }) + + // What the wizard does on skip/close. + await api("provider_release_job", { name: "skipprobe" }) + + // Settles to the uninitialized state rather than staying busy. + await expect(card).toContainText(/not initialized/i, { timeout: 10000 }) + } finally { + await api("provider_delete", { name: "skipprobe" }) + } + }) +}) diff --git a/desktop/e2e/workspaces.e2e.ts b/desktop/e2e/workspaces.e2e.ts index f8198f4d4..c5ebc555a 100644 --- a/desktop/e2e/workspaces.e2e.ts +++ b/desktop/e2e/workspaces.e2e.ts @@ -44,6 +44,51 @@ test.describe("Workspaces Page", () => { }) }) +test.describe("Workspace lifecycle badges", () => { + const api = async (channel: string, args: Record) => + page.evaluate( + ([c, a]) => + ( + window as unknown as { + electronAPI: { + invoke: ( + c: string, + a?: Record, + ) => Promise + } + } + ).electronAPI.invoke(c as string, a as Record), + [channel, args] as const, + ) + + test("shows a Deleting badge while removal is in flight, then removes the row", async () => { + const main = page.locator('[data-slot="sidebar-inset"] main') + + try { + await api("workspace_up", { + source: "https://example.com/deleteprobe.git", + workspaceId: "deleteprobe", + }) + await expect(main.locator("text=deleteprobe")).toBeVisible({ + timeout: 5000, + }) + + // Not awaited: the assertions below run while the delete is in flight. + void api("workspace_delete", { workspaceId: "deleteprobe" }).catch( + () => undefined, + ) + + 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" }) + } + }) +}) + test.describe.serial("Create Workspace Wizard", () => { test("should open the wizard and show step 1 (provider)", async () => { await page.getByRole("button", { name: /create workspace/i }).click() diff --git a/desktop/src/main/__tests__/cli.test.ts b/desktop/src/main/__tests__/cli.test.ts index 7cdb68908..d9391770f 100644 --- a/desktop/src/main/__tests__/cli.test.ts +++ b/desktop/src/main/__tests__/cli.test.ts @@ -1,6 +1,7 @@ // @vitest-environment node import { execFile, spawn } from "node:child_process" import { EventEmitter } from "node:events" +import { Readable } from "node:stream" import { beforeEach, describe, expect, it, vi } from "vitest" import { CliRunner } from "../cli.js" @@ -24,6 +25,17 @@ function fakeChild() { return child } +/** A child whose stdout/stderr are real streams, as readline requires. */ +function fakeStreamingChild() { + const child = new EventEmitter() as EventEmitter & { + stdout: Readable + stderr: Readable + } + child.stdout = new Readable({ read() {} }) + child.stderr = new Readable({ read() {} }) + return child +} + vi.mock("node:child_process", async (importOriginal) => { const actual = await importOriginal() return { @@ -213,6 +225,36 @@ describe("CliRunner", () => { }) }) + describe("runStreaming", () => { + it("reports an exit when the child fails to spawn", async () => { + const child = fakeStreamingChild() + const mockSpawn = vi.mocked(spawn) as unknown as ReturnType + mockSpawn.mockReturnValue(child) + + const onExit = vi.fn() + await cli.runStreaming(["provider", "init", "docker"], () => {}, onExit) + child.emit("error", new Error("spawn ENOENT")) + + await vi.waitFor(() => expect(onExit).toHaveBeenCalledTimes(1)) + const [code, cliError] = onExit.mock.calls[0] + expect(code).toBe(-1) + expect(cliError?.message).toContain("spawn ENOENT") + }) + + it("reports the exit once when error and close both fire", async () => { + const child = fakeStreamingChild() + const mockSpawn = vi.mocked(spawn) as unknown as ReturnType + mockSpawn.mockReturnValue(child) + + const onExit = vi.fn() + await cli.runStreaming(["provider", "init", "docker"], () => {}, onExit) + child.emit("error", new Error("boom")) + child.emit("close", 1) + + await vi.waitFor(() => expect(onExit).toHaveBeenCalledTimes(1)) + }) + }) + describe("stripAnsi", () => { it("removes ANSI escape sequences", () => { const result = CliRunner.stripAnsi( diff --git a/desktop/src/main/__tests__/ipc-provider-jobs.test.ts b/desktop/src/main/__tests__/ipc-provider-jobs.test.ts new file mode 100644 index 000000000..025e4f0c7 --- /dev/null +++ b/desktop/src/main/__tests__/ipc-provider-jobs.test.ts @@ -0,0 +1,192 @@ +// @vitest-environment node +import { EventEmitter } from "node:events" +import { beforeEach, describe, expect, it, vi } from "vitest" +import { ProviderJobs } from "../provider-jobs.js" + +const handlers = new Map unknown>() + +vi.mock("electron", () => ({ + app: { getPath: () => "/tmp", getVersion: () => "0.0.0" }, + dialog: {}, + ipcMain: { + handle: (channel: string, fn: (...args: unknown[]) => unknown) => { + handlers.set(channel, fn) + }, + on: () => undefined, + }, +})) + +vi.mock("../analytics.js", () => ({ + hashWorkspaceRef: (v: string) => v, + trackEvent: () => undefined, +})) + +const { registerIpcHandlers } = await import("../ipc.js") + +/** A child that emits the given stdout lines, then exits with `code`. */ +function fakeChild(lines: string[], code: number) { + const child = new EventEmitter() as EventEmitter & { + exitCode: number | null + signalCode: string | null + kill: () => void + } + child.exitCode = null + child.signalCode = null + child.kill = () => undefined + return { child, lines, code } +} + +function setup( + script: (cliArgs: string[]) => { lines: string[]; code: number } = () => ({ + lines: [], + code: 0, + }), +) { + const providerJobs = new ProviderJobs() + const cli = { + run: vi.fn(async () => ({})), + runRaw: vi.fn(async () => ""), + runStreaming: vi.fn( + async ( + cliArgs: string[], + onLine: (line: string, stream: "stdout" | "stderr") => void, + onExit: (code: number, cliError?: unknown) => void, + ) => { + const { lines, code } = script(cliArgs) + const { child } = fakeChild(lines, code) + // Deliver lines then exit asynchronously, as a real child does. + setTimeout(() => { + for (const line of lines) onLine(line, "stdout") + onExit(code, code === 0 ? undefined : { message: "boom" }) + }, 0) + return child + }, + ), + cancelFor: vi.fn(async () => undefined), + } + const deps = { + cli, + state: { workspaceContext: () => "ctx", providerList: () => [] }, + logStore: { + createLogFile: () => "/tmp/log.txt", + appendLog: () => true, + closeLog: async () => undefined, + onDrain: async () => undefined, + }, + pty: { cancelFor: vi.fn(async () => undefined) }, + getMainWindow: () => null, + providerJobs, + } + // biome-ignore lint/suspicious/noExplicitAny: partial test doubles + registerIpcHandlers(deps as any) + return { providerJobs, cli } +} + +function invoke(channel: string, args: Record) { + const handler = handlers.get(channel) + if (!handler) throw new Error(`${channel} not registered`) + return handler({}, args) +} + +function statusLine(phase: string) { + return JSON.stringify({ kind: "status", pipeline: "provider", phase }) +} + +describe("provider job lifecycle over IPC", () => { + beforeEach(() => { + handlers.clear() + vi.clearAllMocks() + }) + + it("clears the job when init succeeds", async () => { + const { providerJobs } = setup(() => ({ + lines: [statusLine("running_init"), statusLine("ready")], + code: 0, + })) + + await invoke("provider_init", { name: "docker" }) + + expect(providerJobs.get("docker")).toBeUndefined() + }) + + it("records the failure when init fails", async () => { + const { providerJobs } = setup(() => ({ lines: [], code: 1 })) + + const result = (await invoke("provider_init", { name: "docker" })) as { + ok: boolean + } + + expect(result.ok).toBe(false) + expect(providerJobs.get("docker")?.error).toBeTruthy() + }) + + it("leaves the job open after add, for the chained init to close", async () => { + // Closing it here would flash the red badge between add and init. + const { providerJobs } = setup(() => ({ + lines: [statusLine("installing_provider")], + code: 0, + })) + + await invoke("provider_add", { name: "docker" }) + + expect(providerJobs.get("docker")?.activity).toBe("installing") + }) + + it("releases an abandoned job so the card stops spinning", async () => { + const { providerJobs } = setup(() => ({ lines: [], code: 0 })) + + await invoke("provider_add", { name: "docker" }) + expect(providerJobs.get("docker")).toBeDefined() + + await invoke("provider_release_job", { name: "docker" }) + + expect(providerJobs.get("docker")).toBeUndefined() + }) + + it("keeps a failed job's error when released", async () => { + const { providerJobs } = setup(() => ({ lines: [], code: 1 })) + + await invoke("provider_init", { name: "docker" }) + await invoke("provider_release_job", { name: "docker" }) + + expect(providerJobs.get("docker")?.error).toBeTruthy() + }) + + it("runs set-source then init on update, clearing the job once", async () => { + const seen: string[][] = [] + const { providerJobs } = setup((cliArgs) => { + seen.push(cliArgs) + return { lines: [], code: 0 } + }) + + await invoke("provider_update", { name: "docker" }) + + expect(seen.map((a) => a[1])).toEqual(["set-source", "init"]) + expect(providerJobs.get("docker")).toBeUndefined() + }) + + it("records the failure when the update's chained init fails", async () => { + const { providerJobs } = setup((cliArgs) => ({ + lines: [], + code: cliArgs[1] === "init" ? 1 : 0, + })) + + await expect( + invoke("provider_update", { name: "docker" }), + ).rejects.toThrow() + + expect(providerJobs.get("docker")?.error).toBeTruthy() + }) + + it("does not blame a successful init for a refresh failure afterward", async () => { + const { providerJobs } = setup(() => ({ + lines: [statusLine("running_init"), statusLine("ready")], + code: 0, + })) + providerJobs.setRefresh(() => Promise.reject(new Error("refresh boom"))) + + await invoke("provider_init", { name: "docker" }) + + expect(providerJobs.get("docker")?.error).not.toBe("refresh boom") + }) +}) diff --git a/desktop/src/main/__tests__/ipc-workspace-jobs.test.ts b/desktop/src/main/__tests__/ipc-workspace-jobs.test.ts new file mode 100644 index 000000000..f9e73bc05 --- /dev/null +++ b/desktop/src/main/__tests__/ipc-workspace-jobs.test.ts @@ -0,0 +1,118 @@ +// @vitest-environment node +import { EventEmitter } from "node:events" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { ProviderJobs } from "../provider-jobs.js" +import { WorkspaceJobs } from "../workspace-jobs.js" + +const handlers = new Map unknown>() + +vi.mock("electron", () => ({ + app: { getPath: () => "/tmp", getVersion: () => "0.0.0" }, + dialog: {}, + ipcMain: { + handle: (channel: string, fn: (...args: unknown[]) => unknown) => { + handlers.set(channel, fn) + }, + on: () => undefined, + }, +})) + +vi.mock("../analytics.js", () => ({ + hashWorkspaceRef: (v: string) => v, + trackEvent: () => undefined, +})) + +const { registerIpcHandlers } = await import("../ipc.js") + +function fakeChild() { + const child = new EventEmitter() as EventEmitter & { + exitCode: number | null + signalCode: string | null + kill: () => void + } + child.exitCode = null + child.signalCode = null + child.kill = () => undefined + return child +} + +function setup(exitCode = 0) { + const workspaceJobs = new WorkspaceJobs() + const cli = { + run: vi.fn(async () => []), + runRaw: vi.fn(async () => ""), + runStreaming: vi.fn( + async ( + _cliArgs: string[], + _onLine: (line: string, stream: "stdout" | "stderr") => void, + onExit: (code: number, cliError?: unknown) => void, + ) => { + const child = fakeChild() + setTimeout(() => { + onExit(exitCode, exitCode === 0 ? undefined : { message: "boom" }) + }, 0) + return child + }, + ), + cancelFor: vi.fn(async () => undefined), + } + const deps = { + cli, + state: { workspaceContext: () => "ctx", providerList: () => [] }, + logStore: { + createLogFile: () => "/tmp/log.txt", + appendLog: () => true, + closeLog: async () => undefined, + onDrain: async () => undefined, + }, + pty: { cancelFor: vi.fn(async () => undefined) }, + getMainWindow: () => null, + providerJobs: new ProviderJobs(), + workspaceJobs, + } + // biome-ignore lint/suspicious/noExplicitAny: partial test doubles + registerIpcHandlers(deps as any) + return { workspaceJobs } +} + +function invoke(channel: string, args: Record) { + const handler = handlers.get(channel) + if (!handler) throw new Error(`${channel} not registered`) + return handler({}, args) +} + +describe("workspace delete job lifecycle over IPC", () => { + beforeEach(() => { + handlers.clear() + vi.clearAllMocks() + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("shows a deleting job while the command runs, then clears it", async () => { + const { workspaceJobs } = setup(0) + + // workspace_delete resolves as soon as the CLI command is launched, well + // before it exits — the job must already be visible at that point. + await invoke("workspace_delete", { workspaceId: "ws1" }) + expect(workspaceJobs.get("ws1")).toEqual({ activity: "deleting" }) + + // Deterministically flush the mock's exit callback instead of racing it + // against a fixed wall-clock wait. + await vi.runAllTimersAsync() + + expect(workspaceJobs.get("ws1")).toBeUndefined() + }) + + it("retains the failure when the delete command fails", async () => { + const { workspaceJobs } = setup(1) + + await invoke("workspace_delete", { workspaceId: "ws1" }) + await vi.runAllTimersAsync() + + expect(workspaceJobs.get("ws1")?.error).toBe("boom") + }) +}) diff --git a/desktop/src/main/__tests__/provider-jobs.test.ts b/desktop/src/main/__tests__/provider-jobs.test.ts new file mode 100644 index 000000000..bf3b2b142 --- /dev/null +++ b/desktop/src/main/__tests__/provider-jobs.test.ts @@ -0,0 +1,132 @@ +// @vitest-environment node +import { beforeEach, describe, expect, it, vi } from "vitest" +import { ProviderJobs } from "../provider-jobs.js" + +describe("ProviderJobs", () => { + let jobs: ProviderJobs + + beforeEach(() => { + jobs = new ProviderJobs() + }) + + it("tracks activity and phase transitions", async () => { + jobs.start("docker", "installing") + expect(jobs.get("docker")).toEqual({ activity: "installing" }) + + jobs.report("docker", "installing_provider") + expect(jobs.get("docker")).toEqual({ + activity: "installing", + phase: "installing_provider", + }) + + await jobs.finish("docker") + expect(jobs.get("docker")).toBeUndefined() + }) + + it("ignores phase reports for a provider with no active job", () => { + jobs.report("docker", "running_init") + expect(jobs.get("docker")).toBeUndefined() + }) + + it("retains the failure so the UI can explain it", async () => { + jobs.start("docker", "initializing") + await jobs.finish("docker", "init: boom") + + expect(jobs.get("docker")).toEqual({ + activity: "initializing", + phase: "failed", + error: "init: boom", + }) + }) + + it("does not let a later release erase a recorded failure", async () => { + jobs.start("docker", "initializing") + await jobs.finish("docker", "init: boom") + + // The wizard closing after a failed init releases the job; that must not + // turn the failure into a clean success. + await jobs.finish("docker") + + expect(jobs.get("docker")?.error).toBe("init: boom") + }) + + it("refreshes provider state before clearing a finished job", async () => { + const order: string[] = [] + jobs.setRefresh(async () => { + order.push(`refresh(job=${jobs.get("docker") ? "present" : "gone"})`) + }) + + jobs.start("docker", "installing") + await jobs.finish("docker") + + expect(order).toEqual(["refresh(job=present)"]) + expect(jobs.get("docker")).toBeUndefined() + }) + + it("does not clear a newer job started while refresh was in flight", async () => { + let releaseRefresh: (() => void) | undefined + jobs.setRefresh( + () => + new Promise((resolve) => { + releaseRefresh = resolve + }), + ) + + jobs.start("docker", "installing") + const finishing = jobs.finish("docker") + + jobs.start("docker", "initializing") + releaseRefresh?.() + await finishing + + expect(jobs.get("docker")).toEqual({ activity: "initializing" }) + }) + + it("still tracks phases for the superseding job", async () => { + let releaseRefresh: (() => void) | undefined + jobs.setRefresh( + () => + new Promise((resolve) => { + releaseRefresh = resolve + }), + ) + + jobs.start("docker", "installing") + const finishing = jobs.finish("docker") + jobs.start("docker", "initializing") + releaseRefresh?.() + await finishing + + jobs.report("docker", "running_init") + + expect(jobs.get("docker")?.phase).toBe("running_init") + }) + + it("notifies listeners on every mutation", () => { + const listener = vi.fn() + jobs.onChange(listener) + + jobs.start("docker", "installing") + jobs.report("docker", "installing_provider") + jobs.clear("docker") + + expect(listener).toHaveBeenCalledTimes(3) + }) + + it("does not notify when clearing an untracked provider", () => { + const listener = vi.fn() + jobs.onChange(listener) + + jobs.clear("nonexistent") + + expect(listener).not.toHaveBeenCalled() + }) + + it("clears a retained failure so a re-added provider starts clean", () => { + jobs.start("docker", "installing") + jobs.report("docker", "failed", "boom") + jobs.clear("docker") + + expect(jobs.get("docker")).toBeUndefined() + }) +}) diff --git a/desktop/src/main/__tests__/watcher.test.ts b/desktop/src/main/__tests__/watcher.test.ts new file mode 100644 index 000000000..f2db9f684 --- /dev/null +++ b/desktop/src/main/__tests__/watcher.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from "vitest" +import { Watcher } from "../watcher.js" + +function makeWatcher(runProviderList: () => Promise>) { + const state = { + updateProviders: vi.fn().mockReturnValue(false), + providerList: vi.fn().mockReturnValue([]), + } + const cli = { + run: vi.fn((args: string[]) => { + if (args[0] === "provider" && args[1] === "list") { + return runProviderList() + } + return Promise.resolve([]) + }), + } + const providerJobs = { snapshot: vi.fn().mockReturnValue({}) } + const workspaceJobs = { snapshot: vi.fn().mockReturnValue({}) } + const watcher = new Watcher({ + cli: cli as never, + state: state as never, + getMainWindow: () => null, + providerJobs: providerJobs as never, + workspaceJobs: workspaceJobs as never, + }) + return { watcher, cli, state } +} + +describe("Watcher.refreshProviders", () => { + it("does not run concurrently with another in-flight provider query", async () => { + let inFlight = 0 + let concurrentCalls = 0 + const { watcher } = makeWatcher(async () => { + inFlight++ + if (inFlight > 1) concurrentCalls++ + await new Promise((r) => setTimeout(r, 20)) + inFlight-- + return {} + }) + + const first = watcher.refreshProviders() + const second = watcher.refreshProviders() + await Promise.all([first, second]) + + expect(concurrentCalls).toBe(0) + }) + + it("does not start the second query until the first has finished", async () => { + const events: string[] = [] + let releaseFirst!: () => void + const { watcher } = makeWatcher(async () => { + const id = events.filter((e) => e.startsWith("start")).length + 1 + events.push(`start-${id}`) + if (id === 1) { + await new Promise((resolve) => { + releaseFirst = resolve + }) + } + events.push(`end-${id}`) + return {} + }) + + const first = watcher.refreshProviders() + await Promise.resolve() // let the first query actually start + const second = watcher.refreshProviders() + await Promise.resolve() + releaseFirst() + await Promise.all([first, second]) + + expect(events).toEqual(["start-1", "end-1", "start-2", "end-2"]) + }) +}) diff --git a/desktop/src/main/__tests__/workspace-jobs.test.ts b/desktop/src/main/__tests__/workspace-jobs.test.ts new file mode 100644 index 000000000..7ce9caba7 --- /dev/null +++ b/desktop/src/main/__tests__/workspace-jobs.test.ts @@ -0,0 +1,104 @@ +// @vitest-environment node +import { beforeEach, describe, expect, it, vi } from "vitest" +import { WorkspaceJobs } from "../workspace-jobs.js" + +describe("WorkspaceJobs", () => { + let jobs: WorkspaceJobs + + beforeEach(() => { + jobs = new WorkspaceJobs() + }) + + it("tracks a delete until it finishes", async () => { + const generation = jobs.start("ws1") + expect(jobs.get("ws1")).toEqual({ activity: "deleting" }) + + await jobs.finish("ws1", generation) + expect(jobs.get("ws1")).toBeUndefined() + }) + + it("does not let a stale failure land on a newer job", async () => { + const first = jobs.start("ws1") + const second = jobs.start("ws1") + + await jobs.finish("ws1", first, "boom") + + expect(jobs.get("ws1")).toEqual({ activity: "deleting" }) + expect(second).not.toBe(first) + }) + + it("retains the failure so the UI can explain it", async () => { + const generation = jobs.start("ws1") + await jobs.finish("ws1", generation, "delete exited with code 1") + + expect(jobs.get("ws1")).toEqual({ + activity: "deleting", + error: "delete exited with code 1", + }) + }) + + it("does not let a later release erase a recorded failure", async () => { + const generation = jobs.start("ws1") + await jobs.finish("ws1", generation, "boom") + + await jobs.finish("ws1", generation) + + expect(jobs.get("ws1")?.error).toBe("boom") + }) + + it("ignores a failure for a workspace with no active job", async () => { + await jobs.finish("ws1", 1, "boom") + expect(jobs.get("ws1")).toBeUndefined() + }) + + it("refreshes the workspace list before clearing a finished job", async () => { + const order: string[] = [] + jobs.setRefresh(async () => { + order.push(`refresh(job=${jobs.get("ws1") ? "present" : "gone"})`) + }) + + const generation = jobs.start("ws1") + await jobs.finish("ws1", generation) + + expect(order).toEqual(["refresh(job=present)"]) + expect(jobs.get("ws1")).toBeUndefined() + }) + + it("does not clear a newer job started while refresh was in flight", async () => { + let releaseRefresh: (() => void) | undefined + jobs.setRefresh( + () => + new Promise((resolve) => { + releaseRefresh = resolve + }), + ) + + const generation = jobs.start("ws1") + const finishing = jobs.finish("ws1", generation) + + jobs.start("ws1") + releaseRefresh?.() + await finishing + + expect(jobs.get("ws1")).toEqual({ activity: "deleting" }) + }) + + it("notifies listeners on every mutation", () => { + const listener = vi.fn() + jobs.onChange(listener) + + jobs.start("ws1") + jobs.clear("ws1") + + expect(listener).toHaveBeenCalledTimes(2) + }) + + it("does not notify when clearing an untracked workspace", () => { + const listener = vi.fn() + jobs.onChange(listener) + + jobs.clear("nonexistent") + + expect(listener).not.toHaveBeenCalled() + }) +}) diff --git a/desktop/src/main/cli.ts b/desktop/src/main/cli.ts index 367919421..7d98ae5e6 100644 --- a/desktop/src/main/cli.ts +++ b/desktop/src/main/cli.ts @@ -297,7 +297,10 @@ export class CliRunner { }) } - child.on("close", (code) => { + let settled = false + const finish = (code: number, cliError?: CLIError): void => { + if (settled) return + settled = true this.activeChildren.delete(child) if (workspaceId) { const bucket = this.childrenByWorkspace.get(workspaceId) @@ -307,7 +310,18 @@ export class CliRunner { } } this.release() - onExit(code ?? -1, lastCliError) + onExit(code, cliError) + } + + // A spawn failure (missing binary, EACCES) emits "error" and never + // "close". Without this, onExit never fires: callers that wrap this in a + // promise hang forever, and the concurrency slot is never released. + child.on("error", (err) => { + finish(-1, { code: "spawn_failed", message: err.message }) + }) + + child.on("close", (code) => { + finish(code ?? -1, lastCliError) }) return child diff --git a/desktop/src/main/index.ts b/desktop/src/main/index.ts index 7cf9fafa2..0906a51c6 100644 --- a/desktop/src/main/index.ts +++ b/desktop/src/main/index.ts @@ -6,11 +6,13 @@ import { CliRunner } from "./cli.js" import { DaemonManager } from "./daemon-manager.js" import { registerIpcHandlers } from "./ipc.js" import { LogStore } from "./log-store.js" +import { ProviderJobs } from "./provider-jobs.js" import { PtyManager } from "./pty.js" import { DaemonState } from "./state.js" import { AppTray } from "./tray.js" import { initAutoUpdater, stopAutoUpdater } from "./updater.js" import { Watcher } from "./watcher.js" +import { WorkspaceJobs } from "./workspace-jobs.js" const PROTOCOL = "devsy" @@ -160,6 +162,9 @@ app.whenReady().then(() => { stopAutoUpdater() }) + const providerJobs = new ProviderJobs() + const workspaceJobs = new WorkspaceJobs() + // Register IPC handlers const { tunnelProcesses, @@ -171,6 +176,8 @@ app.whenReady().then(() => { logStore, pty: ptyManager, getMainWindow: () => mainWindow, + providerJobs, + workspaceJobs, }) // Start state watcher @@ -179,7 +186,14 @@ app.whenReady().then(() => { daemon: daemonManager.daemonClient, state, getMainWindow: () => mainWindow, + providerJobs, + workspaceJobs, }) + providerJobs.onChange(() => watcher.broadcastProviders()) + providerJobs.setRefresh(() => watcher.refreshProviders()) + workspaceJobs.onChange(() => watcher.broadcastWorkspaces()) + workspaceJobs.setRefresh(() => watcher.refreshWorkspaces()) + void watcher.start().then(runInitialProviderUpdateCheck) scheduleProviderUpdateCheck() diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 191867971..fec4a2a89 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -10,6 +10,11 @@ import type { CLIError } from "../shared/cli-error.js" import { parseCliEnvelope } from "../shared/cli-error.js" import { hashWorkspaceRef, trackEvent } from "./analytics.js" import { loadCatalog } from "./image-catalog.js" +import type { + ProviderActivity, + ProviderJobs, + ProviderPhase, +} from "./provider-jobs.js" import type { CliRunner } from "./cli.js" import type { LogStore } from "./log-store.js" import type { PtyManager } from "./pty.js" @@ -27,6 +32,7 @@ import { setReleaseChannel, } from "./updater.js" import { type ProviderEntry, parseProviderEntries } from "./watcher.js" +import type { WorkspaceJobs } from "./workspace-jobs.js" const execFileAsync = promisify(execFile) @@ -90,6 +96,8 @@ interface IpcDependencies { logStore: LogStore pty: PtyManager getMainWindow: () => BrowserWindow | null + providerJobs: ProviderJobs + workspaceJobs: WorkspaceJobs } /** Format a line in zap console format so log-parser.ts can parse it. */ @@ -101,7 +109,11 @@ interface ProgressSink { line(formatted: string): boolean done( finalLine: string, - extra?: { level?: "info" | "warn" | "error"; cliError?: CLIError }, + extra?: { + level?: "info" | "warn" | "error" + cliError?: CLIError + success?: boolean + }, ): Promise } @@ -122,6 +134,7 @@ function createLogSink( message?: string level?: "info" | "warn" | "error" cliError?: CLIError + success?: boolean }, ): void { if (timer) { @@ -161,7 +174,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { scheduleProviderUpdateCheck: () => void runInitialProviderUpdateCheck: () => void } { - const { cli, state, logStore, pty } = deps + const { cli, state, logStore, pty, providerJobs, workspaceJobs } = deps const tunnelProcesses = new Map< string, import("node:child_process").ChildProcess @@ -243,6 +256,74 @@ export function registerIpcHandlers(deps: IpcDependencies): { }) } + function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error) + } + + /** + * Track a provider job for the duration of fn, so the job cannot outlive the + * work it describes. Opening a job in one place and closing it in another + * leaves the card spinning forever on any path that forgets. + * + * Errors are recorded on the job and rethrown, leaving the caller's own + * error handling intact. + */ + async function withProviderJob( + name: string, + activity: ProviderActivity, + fn: () => Promise, + ): Promise { + providerJobs.start(name, activity) + let result: T + try { + result = await fn() + } catch (error) { + const cliError = (error as { cliError?: CLIError }).cliError + await providerJobs.finish(name, cliError?.message ?? errorMessage(error)) + throw error + } + await providerJobs.finish(name) + return result + } + + /** + * Run a provider CLI command, feeding its NDJSON status lines into the job + * registry so the UI tracks phases as they happen instead of only learning + * the outcome at exit. + */ + function runProviderWithStatus( + name: string, + cliArgs: string[], + ): Promise { + return new Promise((resolve, reject) => { + cli + .runStreaming( + cliArgs, + (line, stream) => { + if (stream !== "stdout") return + const envelope = parseCliEnvelope(line) + if (envelope?.kind === "status") { + providerJobs.report( + name, + envelope.phase as ProviderPhase, + envelope.error, + ) + } + }, + (code, cliError) => { + if (code === 0) { + resolve() + return + } + const message = + cliError?.message ?? `${cliArgs.join(" ")} exited with ${code}` + reject(Object.assign(new Error(message), { cliError })) + }, + ) + .catch(reject) + }) + } + /** * Compute provider update information by querying the CLI for all installed providers. */ @@ -361,15 +442,37 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (args.singleMachine) { cliArgs.push("--single-machine") } - await cli.runRaw(cliArgs) + + // Not withProviderJob: the job outlives this call so the badge stays busy + // until the wizard's chained provider_init finishes. The opener closes it, + // via provider_release_job on any path that abandons the install. + providerJobs.start(args.name, "installing") + try { + await runProviderWithStatus(args.name, cliArgs) + } catch (error) { + await providerJobs.finish(args.name, errorMessage(error)) + throw error + } }, ) ipcMain.handle("provider_delete", async (_event, args: { name: string }) => { trackEvent("provider_delete") await cli.runRaw(["provider", "delete", args.name]) + // Drop any retained failure so a re-added provider starts clean. + providerJobs.clear(args.name) }) + // Releases a job the caller opened but will not finish — e.g. the wizard + // installs a provider, then the user skips initialization or closes the + // dialog. Without this the card would spin on "installing…" indefinitely. + ipcMain.handle( + "provider_release_job", + async (_event, args: { name: string }) => { + await providerJobs.finish(args.name) + }, + ) + ipcMain.handle("provider_use", async (_event, args: { name: string }) => { await cli.runRaw(["provider", "use", args.name]) }) @@ -380,12 +483,13 @@ export function registerIpcHandlers(deps: IpcDependencies): { // own-properties, so a thrown Error with a .cliError attached would lose it. ipcMain.handle("provider_init", async (_event, args: { name: string }) => { try { - await cli.runRaw(["provider", "init", args.name]) + await withProviderJob(args.name, "initializing", () => + runProviderWithStatus(args.name, ["provider", "init", args.name]), + ) return { ok: true } as const } catch (err) { const cliError = (err as { cliError?: CLIError }).cliError - const message = err instanceof Error ? err.message : String(err) - return { ok: false, message, cliError } as const + return { ok: false, message: errorMessage(err), cliError } as const } }) @@ -395,9 +499,24 @@ export function registerIpcHandlers(deps: IpcDependencies): { const cmdId = crypto.randomUUID() const win = deps.getMainWindow() + // Not withProviderJob: the job outlives the handler, closed from onExit. + providerJobs.start(args.name, "initializing") await cli.runStreaming( ["provider", "init", args.name], - (line, _stream, meta) => { + (line, stream, meta) => { + // Status envelopes drive the job registry; everything else is log + // text for the wizard's output pane. + if (stream === "stdout") { + const envelope = parseCliEnvelope(line) + if (envelope?.kind === "status") { + providerJobs.report( + args.name, + envelope.phase as ProviderPhase, + envelope.error, + ) + return + } + } const formatted = formatLogLine(line) win?.webContents.send("command-progress", { commandId: cmdId, @@ -406,7 +525,15 @@ export function registerIpcHandlers(deps: IpcDependencies): { done: false, }) }, - (code, cliError) => { + async (code, cliError) => { + // Finish first: it refreshes the provider list, so the wizard's + // done signal can't arrive while the card still reads uninitialized. + await providerJobs.finish( + args.name, + code === 0 + ? undefined + : (cliError?.message ?? `provider init exited with ${code}`), + ) const exitMsg = formatLogLine( `Exit code: ${code}`, code === 0 ? "INFO" : "ERROR", @@ -415,6 +542,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { commandId: cmdId, message: exitMsg, level: code === 0 ? "info" : "error", + success: code === 0, cliError, done: true, }) @@ -425,8 +553,19 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ) + // set-source installs replacement binaries and clears Initialized, since + // the new binary has not run its init. Chain init so the provider ends up + // usable rather than sitting in a needs-re-init state the user must notice. ipcMain.handle("provider_update", async (_event, args: { name: string }) => { - await cli.runRaw(["provider", "set-source", args.name, "--use=false"]) + await withProviderJob(args.name, "updating", async () => { + await runProviderWithStatus(args.name, [ + "provider", + "set-source", + args.name, + "--use=false", + ]) + await runProviderWithStatus(args.name, ["provider", "init", args.name]) + }) }) ipcMain.handle("provider_options", async (_event, args: { name: string }) => { @@ -485,13 +624,18 @@ export function registerIpcHandlers(deps: IpcDependencies): { ipcMain.handle( "provider_set_version", async (_event, args: { name: string; tag: string }) => { - await cli.runRaw([ - "provider", - "set-source", - args.name, - "--version", - args.tag, - ]) + // Pinning a version swaps binaries via the same update path, so it + // clears Initialized and needs the same re-init as a source change. + await withProviderJob(args.name, "updating", async () => { + await runProviderWithStatus(args.name, [ + "provider", + "set-source", + args.name, + "--version", + args.tag, + ]) + await runProviderWithStatus(args.name, ["provider", "init", args.name]) + }) }, ) @@ -813,6 +957,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { const err = error as Error & { cliError?: CLIError } void sink.done(formatLogLine(err.message, "ERROR"), { level: "error", + success: false, cliError: err.cliError ?? { code: "up_failed", message: err.message, @@ -859,7 +1004,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (envelope?.kind === "result") { signalledDone = true releaseTask() - void sink.done(formatted) + void sink.done(formatted, { success: true }) return } @@ -868,6 +1013,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { releaseTask() void sink.done(formatted, { level: "error", + success: false, cliError: { code: "up_failed", message: envelope.message }, }) return @@ -887,7 +1033,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { `Exit code: ${code}`, code === 0 ? "INFO" : "ERROR", ), - code === 0 ? undefined : { level: "error", cliError }, + code === 0 + ? { success: true } + : { level: "error", success: false, cliError }, ) }, wsId, @@ -899,6 +1047,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { const err = error as Error & { cliError?: CLIError } void sink.done(formatLogLine(err.message, "ERROR"), { level: "error", + success: false, cliError: err.cliError ?? { code: "up_follow_failed", message: err.message, @@ -943,6 +1092,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { (code) => { void sink.done( formatLogLine(`Exit code: ${code}`, code === 0 ? "INFO" : "ERROR"), + { success: code === 0 }, ) }, args.workspaceId, @@ -980,14 +1130,25 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (args.debug) cliArgs.push("--debug") cliArgs.push("--force") + // The card shows "Deleting" until finish() below + const jobGeneration = workspaceJobs.start(args.workspaceId) + cli.runStreaming( cliArgs, (line) => { if (!sink.line(formatLogLine(line))) return logStore.onDrain(logPath) }, - (code) => { + (code, cliError) => { void sink.done( formatLogLine(`Exit code: ${code}`, code === 0 ? "INFO" : "ERROR"), + { success: code === 0 }, + ) + void workspaceJobs.finish( + args.workspaceId, + jobGeneration, + code === 0 + ? undefined + : cliError?.message ?? `delete exited with code ${code}`, ) }, args.workspaceId, @@ -1026,7 +1187,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { (code, cliError) => { void sink.done( formatLogLine(`Exit code: ${code}`, code === 0 ? "INFO" : "ERROR"), - code === 0 ? undefined : { level: "error", cliError }, + code === 0 + ? { success: true } + : { level: "error", success: false, cliError }, ) }, args.workspaceId, @@ -1065,7 +1228,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { (code, cliError) => { void sink.done( formatLogLine(`Exit code: ${code}`, code === 0 ? "INFO" : "ERROR"), - code === 0 ? undefined : { level: "error", cliError }, + code === 0 + ? { success: true } + : { level: "error", success: false, cliError }, ) }, args.workspaceId, diff --git a/desktop/src/main/provider-jobs.ts b/desktop/src/main/provider-jobs.ts new file mode 100644 index 000000000..d430b9ef7 --- /dev/null +++ b/desktop/src/main/provider-jobs.ts @@ -0,0 +1,133 @@ +/** + * Tracks in-flight provider install/init/update work in the main process. + * + * The persisted `initialized` flag answers "is this provider usable", which + * is false both for a provider the user never initialized and for one being + * installed right now. Those render very differently, so the transient half + * of the lifecycle lives here rather than being inferred from disk state. + * + * Main-process ownership is deliberate: a renderer-local pending set is lost + * when the wizard closes, the user navigates away, or the window reloads, + * which is exactly when a multi-second install is still running. + */ + +/** Phases emitted by the Go provider pipeline. */ +export type ProviderPhase = + | "installing_provider" + | "resolving_options" + | "running_init" + | "ready" + | "failed" + +export type ProviderActivity = "installing" | "initializing" | "updating" + +export interface ProviderJob { + activity: ProviderActivity + phase?: ProviderPhase + /** Set when the job ended in failure; the job is retained so the UI can show why. */ + error?: string +} + +export class ProviderJobs { + private jobs = new Map() + // Lets an in-flight finish() tell whether it still owns the entry. + private generations = new Map() + private lastGeneration = 0 + private listeners = new Set<() => void>() + + /** + * Register a callback fired after every mutation, so a phase transition + * reaches the renderer without waiting for the next 3s disk poll. + */ + onChange(listener: () => void): () => void { + this.listeners.add(listener) + return () => this.listeners.delete(listener) + } + + private emit(): void { + for (const listener of this.listeners) listener() + } + + /** Begin tracking work on a provider, clearing any previous failure. */ + start(name: string, activity: ProviderActivity): void { + this.jobs.set(name, { activity }) + this.generations.set(name, ++this.lastGeneration) + this.emit() + } + + /** + * Record a phase transition. Ignored when no job is active, so a stray + * event can't resurrect a provider the UI already considers settled. + */ + report(name: string, phase: ProviderPhase, error?: string): void { + const job = this.jobs.get(name) + if (!job) return + if (phase === "failed") { + this.jobs.set(name, { ...job, phase, error: error ?? "failed" }) + } else { + this.jobs.set(name, { ...job, phase }) + } + this.emit() + } + + /** Stop tracking a provider, discarding any recorded failure. */ + clear(name: string): void { + this.generations.delete(name) + if (this.jobs.delete(name)) this.emit() + } + + /** + * Finish a job. A failure is retained so the UI can explain it; success + * drops the entry and lets the persisted `initialized` flag speak. + * + * On success the caller must have refreshed the provider list first: + * clearing the job while the list still shows the pre-init `initialized: + * false` would expose the same red badge this class exists to prevent, + * until the next poll caught up. + */ + async finish(name: string, error?: string): Promise { + if (error) { + const job = this.jobs.get(name) + if (!job) return + this.jobs.set(name, { + activity: job.activity, + phase: "failed", + error, + }) + this.emit() + return + } + const job = this.jobs.get(name) + if (!job) return + // A recorded failure survives a later success-shaped finish. + if (job.error) return + + const generation = this.generations.get(name) + await this.refresh?.() + // refresh is a CLI round-trip; a newer job may own the entry by now. + if (this.generations.get(name) !== generation) return + + this.jobs.delete(name) + this.generations.delete(name) + this.emit() + } + + /** + * Supplies a way to re-read provider state from disk, so a finished job + * isn't cleared before the list reflects what the command just wrote. + */ + setRefresh(refresh: () => Promise): void { + this.refresh = refresh + } + + private refresh?: () => Promise + + get(name: string): ProviderJob | undefined { + return this.jobs.get(name) + } + + /** Snapshot for IPC, keyed by provider name. */ + snapshot(): Record { + return Object.fromEntries(this.jobs) + } +} diff --git a/desktop/src/main/watcher.ts b/desktop/src/main/watcher.ts index b698f9db7..eb4395415 100644 --- a/desktop/src/main/watcher.ts +++ b/desktop/src/main/watcher.ts @@ -5,13 +5,17 @@ import { watch } from "chokidar" import type { BrowserWindow } from "electron" import type { CliRunner } from "./cli.js" import type { DaemonClient } from "./daemon-client.js" +import type { ProviderJobs } from "./provider-jobs.js" import type { DaemonState } from "./state.js" +import type { WorkspaceJobs } from "./workspace-jobs.js" interface WatcherDeps { cli: CliRunner daemon?: DaemonClient state: DaemonState getMainWindow: () => BrowserWindow | null + providerJobs: ProviderJobs + workspaceJobs: WorkspaceJobs } interface ContextEntry { @@ -55,6 +59,13 @@ export class Watcher { private fsWatcher: ReturnType | null = null private polling = false private pollQueued = false + // Serializes pollProviders so a manual refreshProviders() can never + // overlap a scheduled poll; each queued call is guaranteed a fresh read + // that starts after it was requested. + private providerPollChain: Promise = Promise.resolve() + // Same serialization for pollWorkspaces, so a manual refreshWorkspaces() + // (e.g. after a delete finishes) can't overlap a scheduled poll. + private workspacePollChain: Promise = Promise.resolve() constructor(private deps: WatcherDeps) {} @@ -97,8 +108,8 @@ export class Watcher { this.polling = true try { await Promise.allSettled([ - this.pollWorkspaces(), - this.pollProviders(), + this.queueWorkspacePoll(), + this.queueProviderPoll(), this.pollMachines(), this.pollContexts(), ]) @@ -125,6 +136,17 @@ export class Watcher { return cliFn() } + /** Re-read the workspace list from disk now, without waiting for the next scheduled poll. */ + async refreshWorkspaces(): Promise { + await this.queueWorkspacePoll() + } + + private queueWorkspacePoll(): Promise { + const run = this.workspacePollChain.then(() => this.pollWorkspaces()) + this.workspacePollChain = run + return run + } + private async pollWorkspaces(): Promise { try { const workspaces = await this.queryWithFallback( @@ -135,15 +157,36 @@ export class Watcher { ) const changed = this.deps.state.updateWorkspaces(workspaces as any[]) if (changed) { - this.send("workspaces-changed", { - workspaces: this.deps.state.workspaceList(), - }) + this.broadcastWorkspaces() } } catch { // Silently ignore poll failures } } + /** + * Push the workspace list plus in-flight job state. Sent on one channel so + * the two can't arrive out of order and show a deleted-but-still-listed + * workspace as idle between the delete finishing and the list catching up. + */ + broadcastWorkspaces(): void { + this.send("workspaces-changed", { + workspaces: this.deps.state.workspaceList(), + jobs: this.deps.workspaceJobs.snapshot(), + }) + } + + /** Re-read provider state from disk now, without waiting for the next scheduled poll. */ + async refreshProviders(): Promise { + await this.queueProviderPoll() + } + + private queueProviderPoll(): Promise { + const run = this.providerPollChain.then(() => this.pollProviders()) + this.providerPollChain = run + return run + } + private async pollProviders(): Promise { try { const raw = await this.queryWithFallback( @@ -160,15 +203,25 @@ export class Watcher { const providers = parseProviderEntries(raw) const changed = this.deps.state.updateProviders(providers as any[]) if (changed) { - this.send("providers-changed", { - providers: this.deps.state.providerList(), - }) + this.broadcastProviders() } } catch { // Silently ignore poll failures } } + /** + * Push the provider list plus in-flight job state. Sent on one channel so + * the two can't arrive out of order and render a provider as idle-and- + * uninitialized between an install finishing and its job clearing. + */ + broadcastProviders(): void { + this.send("providers-changed", { + providers: this.deps.state.providerList(), + jobs: this.deps.providerJobs.snapshot(), + }) + } + private async pollMachines(): Promise { try { const machines = await this.queryWithFallback( diff --git a/desktop/src/main/workspace-jobs.ts b/desktop/src/main/workspace-jobs.ts new file mode 100644 index 000000000..141dfdd60 --- /dev/null +++ b/desktop/src/main/workspace-jobs.ts @@ -0,0 +1,103 @@ +/** + * Tracks in-flight workspace delete operations in the main process, the + * same way ProviderJobs tracks provider install/init work: workspace_delete + * is fire-and-forget from the IPC handler's perspective (it returns as soon + * as the CLI command is launched, so the log-streaming UI isn't blocked), + * so nothing else records that a delete is running. Main-process ownership + * means the "Deleting" badge survives navigating away from the list or + * reloading the window while the delete is still in flight. + */ + +export interface WorkspaceJob { + activity: "deleting" + /** Set when the job ended in failure; the job is retained so the UI can show why. */ + error?: string +} + +export class WorkspaceJobs { + private jobs = new Map() + // Lets an in-flight finish() tell whether it still owns the entry. + private generations = new Map() + private lastGeneration = 0 + private listeners = new Set<() => void>() + private refresh?: () => Promise + + /** + * Register a callback fired after every mutation, so starting or + * finishing a delete reaches the renderer without waiting for the next + * disk poll. + */ + onChange(listener: () => void): () => void { + this.listeners.add(listener) + return () => this.listeners.delete(listener) + } + + private emit(): void { + for (const listener of this.listeners) listener() + } + + /** + * Begin tracking a delete, clearing any previous failure. Returns a + * generation token the caller must pass to finish() so a stale exit + * callback from an earlier delete of the same workspace can't land on + * a retry that has already superseded it. + */ + start(id: string): number { + this.jobs.set(id, { activity: "deleting" }) + const generation = ++this.lastGeneration + this.generations.set(id, generation) + this.emit() + return generation + } + + /** Stop tracking a workspace, discarding any recorded failure. */ + clear(id: string): void { + this.generations.delete(id) + if (this.jobs.delete(id)) this.emit() + } + + /** + * Finish the job started under `generation`. A failure is retained so the + * UI can explain it; success drops the entry once the workspace list has + * caught up, so the card doesn't flash back to its pre-delete state for + * one poll cycle. Either way, a generation mismatch means a retry has + * already superseded this job, so the call is a no-op. + */ + async finish(id: string, generation: number, error?: string): Promise { + if (this.generations.get(id) !== generation) return + + if (error) { + this.jobs.set(id, { activity: "deleting", error }) + this.emit() + return + } + const job = this.jobs.get(id) + if (!job) return + if (job.error) return + + await this.refresh?.() + // refresh is a CLI round-trip; a newer job may own the entry by now. + if (this.generations.get(id) !== generation) return + + this.jobs.delete(id) + this.generations.delete(id) + this.emit() + } + + /** + * Supplies a way to re-read the workspace list from disk, so a finished + * job isn't cleared before the list reflects the deletion. + */ + setRefresh(refresh: () => Promise): void { + this.refresh = refresh + } + + get(id: string): WorkspaceJob | undefined { + return this.jobs.get(id) + } + + /** Snapshot for IPC, keyed by workspace id. */ + snapshot(): Record { + return Object.fromEntries(this.jobs) + } +} diff --git a/desktop/src/renderer/src/lib/components/provider/ProviderCard.svelte b/desktop/src/renderer/src/lib/components/provider/ProviderCard.svelte index aa7a06572..af5a04c9a 100644 --- a/desktop/src/renderer/src/lib/components/provider/ProviderCard.svelte +++ b/desktop/src/renderer/src/lib/components/provider/ProviderCard.svelte @@ -3,12 +3,13 @@ import { badgeVariants } from "$lib/components/ui/badge/index.js" import { Star, Loader2 } from "@lucide/svelte" import ProviderIcon from "./ProviderIcon.svelte" import { providerVersions } from "$lib/stores/providerVersions.js" -import { initializingProviders } from "$lib/stores/providers.js" +import { providerJobs } from "$lib/stores/providers.js" +import { providerStatus } from "$lib/utils/provider-status.js" import type { Provider } from "$lib/types/index.js" let { provider, onopen }: { provider: Provider; onopen?: () => void } = $props() -let isInitializing = $derived($initializingProviders.has(provider.name)) +let status = $derived(providerStatus(provider, $providerJobs[provider.name])) function sourceDisplay(p: Provider): string { if (p.source?.github) return p.source.github @@ -41,15 +42,19 @@ function sourceDisplay(p: Provider): string { {/if}
- {#if provider.state?.initialized} - initialized - {:else if isInitializing} + {#if status.kind === "ready"} + {status.label} + {:else if status.kind === "busy"} - initializing… + {status.label} + + {:else if status.kind === "failed"} + + {status.label} {:else} - not initialized + {status.label} {/if} {#if provider.version} {provider.version} diff --git a/desktop/src/renderer/src/lib/components/provider/ProviderCard.test.ts b/desktop/src/renderer/src/lib/components/provider/ProviderCard.test.ts index 6e47be242..828953dab 100644 --- a/desktop/src/renderer/src/lib/components/provider/ProviderCard.test.ts +++ b/desktop/src/renderer/src/lib/components/provider/ProviderCard.test.ts @@ -14,11 +14,7 @@ vi.mock("$lib/stores/providerVersions.js", async () => { }) import ProviderCard from "./ProviderCard.svelte" -import { - initializingProviders, - markInitializing, - clearInitializing, -} from "$lib/stores/providers.js" +import { providerJobs } from "$lib/stores/providers.js" function makeProvider(name: string, extras: Partial = {}): Provider { return { @@ -56,8 +52,7 @@ describe("ProviderCard", () => { }) it("shows the initializing badge while an uninitialized provider is in flight", () => { - initializingProviders.set(new Set()) - markInitializing("ssh") + providerJobs.set({ ssh: { activity: "initializing", phase: "running_init" } }) const { container, unmount } = render(ProviderCard, { props: { provider: makeProvider("ssh", { state: { initialized: false } }) }, }) @@ -65,12 +60,42 @@ describe("ProviderCard", () => { const text = container.textContent ?? "" expect(text.toLowerCase()).toContain("initializing") expect(text.toLowerCase()).not.toContain("not initialized") - clearInitializing("ssh") + providerJobs.set({}) unmount() }) - it("shows not initialized when no init is in flight", () => { - initializingProviders.set(new Set()) + it("shows installing rather than not initialized during install", () => { + providerJobs.set({ + ssh: { activity: "installing", phase: "installing_provider" }, + }) + const { container, unmount } = render(ProviderCard, { + props: { provider: makeProvider("ssh", { state: { initialized: false } }) }, + }) + + const text = (container.textContent ?? "").toLowerCase() + expect(text).toContain("installing") + expect(text).not.toContain("not initialized") + providerJobs.set({}) + unmount() + }) + + it("shows a failure badge when the job recorded an error", () => { + providerJobs.set({ + ssh: { activity: "initializing", phase: "failed", error: "init: boom" }, + }) + const { container, unmount } = render(ProviderCard, { + props: { provider: makeProvider("ssh", { state: { initialized: false } }) }, + }) + + const text = (container.textContent ?? "").toLowerCase() + expect(text).toContain("failed") + expect(text).not.toContain("not initialized") + providerJobs.set({}) + unmount() + }) + + it("shows not initialized when no job is in flight", () => { + providerJobs.set({}) const { container, unmount } = render(ProviderCard, { props: { provider: makeProvider("ssh", { state: { initialized: false } }) }, }) diff --git a/desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte b/desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte index 5ae33b6da..9f8b8afe9 100644 --- a/desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte +++ b/desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte @@ -28,11 +28,7 @@ import { providerRename, providerSetVersion, } from "$lib/ipc/commands.js" -import { - providers, - markInitializing, - clearInitializing, -} from "$lib/stores/providers.js" +import { providers, providerJobs } from "$lib/stores/providers.js" import { providerVersions, loadVersionsFor, @@ -40,6 +36,7 @@ import { } from "$lib/stores/providerVersions.js" import { toasts } from "$lib/stores/toasts.js" import { extractErrorMessage } from "$lib/utils/error.js" +import { providerStatus } from "$lib/utils/provider-status.js" import type { Provider, ProviderOption } from "$lib/types/index.js" let { @@ -70,6 +67,7 @@ let updating = $state(false) let confirmSwitchOpen = $state(false) let targetTag = $state("") let switching = $state(false) +let status = $derived(providerStatus(provider, $providerJobs[provider.name])) function openVersionSwitch(tag: string) { targetTag = tag @@ -181,8 +179,11 @@ function handleUpdate() { async function runUpdate() { updating = true try { + // Also re-initializes: the new binaries have not run their init, and + // set-source clears the initialized flag accordingly. await providerUpdate(provider.name) toasts.success(`Updated ${provider.name}`) + providers.set(await providerList()) await loadVersionsFor(provider.name) await refreshUpdates() } catch (err) { @@ -198,6 +199,7 @@ async function runSwitch() { try { await providerSetVersion(provider.name, targetTag) toasts.success(`Switched ${provider.name} to ${targetTag}`) + providers.set(await providerList()) await loadVersionsFor(provider.name) await refreshUpdates() } catch (err) { @@ -227,7 +229,6 @@ function extractCliError(err: unknown): CLIError | null { async function handleInitialize() { initializing = true initError = null - markInitializing(provider.name) try { await providerInit(provider.name) const updated = await providerList() @@ -248,7 +249,6 @@ async function handleInitialize() { } } finally { initializing = false - clearInitializing(provider.name) } } @@ -367,8 +367,19 @@ async function handleSaveOptions() { Default {/if} - {#if provider.state?.initialized} - initialized + {#if status.kind === "ready"} + {status.label} + {:else if status.kind === "busy"} + + + {status.label} + + {:else if status.kind === "failed"} + + {status.label} + + {:else} + {status.label} {/if} {#if provider.description} @@ -377,7 +388,7 @@ async function handleSaveOptions() {
- {#if provider.state?.initialized !== true} + {#if status.kind !== "ready" && status.kind !== "busy"}