fix(extensions): preserve startup error context and multiline status
- commit
- 7843f4cbc591e4f4ced8a64720857186f5d298a0
- author
- patriceckhart · · 2026-09-10 05:54 UTC
- parent
- 962dca9
- signature
- unsigned
5 files · +127 -20
M docs/extensions.md +5 -1
| @@ -880,7 +880,11 @@ error in red, including its message, and dismisses itself after five | |||
| 880 | 880 | seconds. Extension failures from startup and `/reload-ext` remain in the | |
| 881 | 881 | scrolling chat until a successful reload or `/clear`, without being sent to | |
| 882 | 882 | the model or saved in the session transcript. Subprocess startup and | |
| 883 | - | handshake errors include the exact stderr log path for the failed extension. | |
| 883 | + | handshake errors identify the extension and its directory, configured executable, | |
| 884 | + | and declared language (if present), along with the original error and stderr log | |
| 885 | + | path when the log was opened. Manifest errors also identify the extension | |
| 886 | + | directory. Multiline diagnostics preserve their line breaks in both the temporary | |
| 887 | + | status and scrolling chat. | |
| 884 | 888 | If a subprocess exits before sending `hello`, the error also reports its exit | |
| 885 | 889 | status or terminating signal. | |
| 886 | 890 | ||
| @@ -880,7 +880,11 @@ error in red, including its message, and dismisses itself after five | |||||
| 880 | seconds. Extension failures from startup and `/reload-ext` remain in the | 880 | seconds. Extension failures from startup and `/reload-ext` remain in the | ||
| 881 | scrolling chat until a successful reload or `/clear`, without being sent to | 881 | scrolling chat until a successful reload or `/clear`, without being sent to | ||
| 882 | the model or saved in the session transcript. Subprocess startup and | 882 | the model or saved in the session transcript. Subprocess startup and | ||
| 883 | - | handshake errors include the exact stderr log path for the failed extension. | 883 | + | handshake errors identify the extension and its directory, configured executable, |
| 884 | + | and declared language (if present), along with the original error and stderr log | |||
| 885 | + | path when the log was opened. Manifest errors also identify the extension | |||
| 886 | + | directory. Multiline diagnostics preserve their line breaks in both the temporary | |||
| 887 | + | status and scrolling chat. | |||
| 884 | If a subprocess exits before sending `hello`, the error also reports its exit | 888 | If a subprocess exits before sending `hello`, the error also reports its exit | ||
| 885 | status or terminating signal. | 889 | status or terminating signal. | ||
| 886 | 890 | ||||
M packages/agent/extensions/manager.go +13 -9
| @@ -396,14 +396,14 @@ func skillEntryPath(dir, entry string) (string, error) { | |||
| 396 | 396 | func (m *Manager) loadOne(ctx context.Context, dir string) error { | |
| 397 | 397 | mf, err := readManifest(dir) | |
| 398 | 398 | if err != nil { | |
| 399 | - | return err | |
| 399 | + | return fmt.Errorf("%s: %w", dir, err) | |
| 400 | 400 | } | |
| 401 | 401 | if mf.Name == "" { | |
| 402 | - | return errors.New("manifest: name is required") | |
| 402 | + | return fmt.Errorf("%s: manifest: name is required", dir) | |
| 403 | 403 | } | |
| 404 | 404 | hasTheme := HasExtensionTheme(dir) | |
| 405 | 405 | if mf.Exec == "" && !hasTheme && len(mf.Skills) == 0 { | |
| 406 | - | return errors.New("manifest: exec, theme, or skills is required") | |
| 406 | + | return fmt.Errorf("%s: manifest: exec, theme, or skills is required", dir) | |
| 407 | 407 | } | |
| 408 | 408 | if !mf.IsEnabled() { | |
| 409 | 409 | // Quietly skip disabled extensions; zot ext list will show them. | |
| @@ -430,7 +430,15 @@ func (m *Manager) loadOne(ctx context.Context, dir string) error { | |||
| 430 | 430 | } | |
| 431 | 431 | if mf.Exec != "" { | |
| 432 | 432 | if err := m.spawn(ctx, ext); err != nil { | |
| 433 | - | return err | |
| 433 | + | language := "" | |
| 434 | + | if mf.Language != "" { | |
| 435 | + | language = fmt.Sprintf(" (declared language: %q)", mf.Language) | |
| 436 | + | } | |
| 437 | + | logDetail := "" | |
| 438 | + | if ext.LogPath != "" { | |
| 439 | + | logDetail = "\n stderr log: " + ext.LogPath | |
| 440 | + | } | |
| 441 | + | return fmt.Errorf("Extension %s failed to start.\n\n exec: %q%s\n error: %w\n extension directory: %s%s", mf.Name, mf.Exec, language, err, dir, logDetail) | |
| 434 | 442 | } | |
| 435 | 443 | } else { | |
| 436 | 444 | ext.readyOnce.Do(func() { close(ext.readyCh) }) | |
| @@ -727,11 +735,7 @@ func (m *Manager) spawn(ctx context.Context, ext *Extension) error { | |||
| 727 | 735 | return fmt.Errorf("stdout pipe (stderr log: %s): %w", logPath, err) | |
| 728 | 736 | } | |
| 729 | 737 | if err := cmd.Start(); err != nil { | |
| 730 | - | language := "" | |
| 731 | - | if ext.Manifest.Language != "" { | |
| 732 | - | language = fmt.Sprintf(" (declared language: %q)", ext.Manifest.Language) | |
| 733 | - | } | |
| 734 | - | return fmt.Errorf("Extension %s failed to start.\n\n exec: %q%s\n error: %w\n extension directory: %s\n stderr log: %s", ext.Manifest.Name, ext.Manifest.Exec, language, err, ext.Dir, logPath) | |
| 738 | + | return err | |
| 735 | 739 | } | |
| 736 | 740 | started = true | |
| 737 | 741 | ext.cmd = cmd | |
| @@ -396,14 +396,14 @@ func skillEntryPath(dir, entry string) (string, error) { | |||||
| 396 | func (m *Manager) loadOne(ctx context.Context, dir string) error { | 396 | func (m *Manager) loadOne(ctx context.Context, dir string) error { | ||
| 397 | mf, err := readManifest(dir) | 397 | mf, err := readManifest(dir) | ||
| 398 | if err != nil { | 398 | if err != nil { | ||
| 399 | - | return err | 399 | + | return fmt.Errorf("%s: %w", dir, err) |
| 400 | } | 400 | } | ||
| 401 | if mf.Name == "" { | 401 | if mf.Name == "" { | ||
| 402 | - | return errors.New("manifest: name is required") | 402 | + | return fmt.Errorf("%s: manifest: name is required", dir) |
| 403 | } | 403 | } | ||
| 404 | hasTheme := HasExtensionTheme(dir) | 404 | hasTheme := HasExtensionTheme(dir) | ||
| 405 | if mf.Exec == "" && !hasTheme && len(mf.Skills) == 0 { | 405 | if mf.Exec == "" && !hasTheme && len(mf.Skills) == 0 { | ||
| 406 | - | return errors.New("manifest: exec, theme, or skills is required") | 406 | + | return fmt.Errorf("%s: manifest: exec, theme, or skills is required", dir) |
| 407 | } | 407 | } | ||
| 408 | if !mf.IsEnabled() { | 408 | if !mf.IsEnabled() { | ||
| 409 | // Quietly skip disabled extensions; zot ext list will show them. | 409 | // Quietly skip disabled extensions; zot ext list will show them. | ||
| @@ -430,7 +430,15 @@ func (m *Manager) loadOne(ctx context.Context, dir string) error { | |||||
| 430 | } | 430 | } | ||
| 431 | if mf.Exec != "" { | 431 | if mf.Exec != "" { | ||
| 432 | if err := m.spawn(ctx, ext); err != nil { | 432 | if err := m.spawn(ctx, ext); err != nil { | ||
| 433 | - | return err | 433 | + | language := "" |
| 434 | + | if mf.Language != "" { | |||
| 435 | + | language = fmt.Sprintf(" (declared language: %q)", mf.Language) | |||
| 436 | + | } | |||
| 437 | + | logDetail := "" | |||
| 438 | + | if ext.LogPath != "" { | |||
| 439 | + | logDetail = "\n stderr log: " + ext.LogPath | |||
| 440 | + | } | |||
| 441 | + | return fmt.Errorf("Extension %s failed to start.\n\n exec: %q%s\n error: %w\n extension directory: %s%s", mf.Name, mf.Exec, language, err, dir, logDetail) | |||
| 434 | } | 442 | } | ||
| 435 | } else { | 443 | } else { | ||
| 436 | ext.readyOnce.Do(func() { close(ext.readyCh) }) | 444 | ext.readyOnce.Do(func() { close(ext.readyCh) }) | ||
| @@ -727,11 +735,7 @@ func (m *Manager) spawn(ctx context.Context, ext *Extension) error { | |||||
| 727 | return fmt.Errorf("stdout pipe (stderr log: %s): %w", logPath, err) | 735 | return fmt.Errorf("stdout pipe (stderr log: %s): %w", logPath, err) | ||
| 728 | } | 736 | } | ||
| 729 | if err := cmd.Start(); err != nil { | 737 | if err := cmd.Start(); err != nil { | ||
| 730 | - | language := "" | 738 | + | return err |
| 731 | - | if ext.Manifest.Language != "" { | |||
| 732 | - | language = fmt.Sprintf(" (declared language: %q)", ext.Manifest.Language) | |||
| 733 | - | } | |||
| 734 | - | return fmt.Errorf("Extension %s failed to start.\n\n exec: %q%s\n error: %w\n extension directory: %s\n stderr log: %s", ext.Manifest.Name, ext.Manifest.Exec, language, err, ext.Dir, logPath) | |||
| 735 | } | 739 | } | ||
| 736 | started = true | 740 | started = true | ||
| 737 | ext.cmd = cmd | 741 | ext.cmd = cmd | ||
A packages/agent/extensions/manager_diagnostics_test.go +71 -0
| @@ -0,0 +1,71 @@ | |||
| 1 | + | package extensions | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 6 | + | "os" | |
| 7 | + | "os/exec" | |
| 8 | + | "path/filepath" | |
| 9 | + | "strings" | |
| 10 | + | "testing" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | func TestLoadErrorsIdentifyExtensionDirectory(t *testing.T) { | |
| 14 | + | for _, explicit := range []bool{false, true} { | |
| 15 | + | mode := "discover" | |
| 16 | + | if explicit { | |
| 17 | + | mode = "explicit" | |
| 18 | + | } | |
| 19 | + | for _, tc := range []struct { | |
| 20 | + | name string | |
| 21 | + | manifest string | |
| 22 | + | want string | |
| 23 | + | }{ | |
| 24 | + | {"invalid-json", `{`, "parse manifest"}, | |
| 25 | + | {"missing-name", `{"exec":"missing"}`, "name is required"}, | |
| 26 | + | {"missing-exec", `{"name":"broken"}`, "exec, theme, or skills is required"}, | |
| 27 | + | {"open-log", `{"name":"broken","exec":"missing"}`, "open log"}, | |
| 28 | + | {"missing-runtime", `{"name":"broken","exec":"zot-test-missing-runtime"}`, "failed to start"}, | |
| 29 | + | } { | |
| 30 | + | t.Run(mode+"/"+tc.name, func(t *testing.T) { | |
| 31 | + | home := t.TempDir() | |
| 32 | + | dir := filepath.Join(home, "extensions", tc.name) | |
| 33 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |
| 34 | + | t.Fatal(err) | |
| 35 | + | } | |
| 36 | + | if err := os.WriteFile(filepath.Join(dir, "extension.json"), []byte(tc.manifest), 0o644); err != nil { | |
| 37 | + | t.Fatal(err) | |
| 38 | + | } | |
| 39 | + | if tc.name == "open-log" { | |
| 40 | + | if err := os.WriteFile(filepath.Join(home, "logs"), nil, 0o644); err != nil { | |
| 41 | + | t.Fatal(err) | |
| 42 | + | } | |
| 43 | + | } | |
| 44 | + | if tc.name == "missing-runtime" { | |
| 45 | + | t.Setenv("PATH", t.TempDir()) | |
| 46 | + | } | |
| 47 | + | mgr := New(home, "", "test", "", "", nil) | |
| 48 | + | var errs []error | |
| 49 | + | if explicit { | |
| 50 | + | errs = mgr.LoadExplicit(context.Background(), []string{dir}) | |
| 51 | + | } else { | |
| 52 | + | errs = mgr.Discover(context.Background()) | |
| 53 | + | } | |
| 54 | + | if len(errs) != 1 { | |
| 55 | + | t.Fatalf("errors = %v, want one", errs) | |
| 56 | + | } | |
| 57 | + | if got := errs[0].Error(); !strings.Contains(got, dir) || !strings.Contains(got, tc.want) { | |
| 58 | + | t.Fatalf("error = %q, want directory %q and %q", got, dir, tc.want) | |
| 59 | + | } | |
| 60 | + | if tc.name == "missing-runtime" { | |
| 61 | + | if !errors.Is(errs[0], exec.ErrNotFound) { | |
| 62 | + | t.Fatalf("error does not wrap exec.ErrNotFound: %v", errs[0]) | |
| 63 | + | } | |
| 64 | + | if strings.Count(errs[0].Error(), dir) != 1 { | |
| 65 | + | t.Fatalf("directory should appear once: %v", errs[0]) | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | }) | |
| 69 | + | } | |
| 70 | + | } | |
| 71 | + | } | |
| @@ -0,0 +1,71 @@ | |||||
| 1 | + | package extensions | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "errors" | |||
| 6 | + | "os" | |||
| 7 | + | "os/exec" | |||
| 8 | + | "path/filepath" | |||
| 9 | + | "strings" | |||
| 10 | + | "testing" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | func TestLoadErrorsIdentifyExtensionDirectory(t *testing.T) { | |||
| 14 | + | for _, explicit := range []bool{false, true} { | |||
| 15 | + | mode := "discover" | |||
| 16 | + | if explicit { | |||
| 17 | + | mode = "explicit" | |||
| 18 | + | } | |||
| 19 | + | for _, tc := range []struct { | |||
| 20 | + | name string | |||
| 21 | + | manifest string | |||
| 22 | + | want string | |||
| 23 | + | }{ | |||
| 24 | + | {"invalid-json", `{`, "parse manifest"}, | |||
| 25 | + | {"missing-name", `{"exec":"missing"}`, "name is required"}, | |||
| 26 | + | {"missing-exec", `{"name":"broken"}`, "exec, theme, or skills is required"}, | |||
| 27 | + | {"open-log", `{"name":"broken","exec":"missing"}`, "open log"}, | |||
| 28 | + | {"missing-runtime", `{"name":"broken","exec":"zot-test-missing-runtime"}`, "failed to start"}, | |||
| 29 | + | } { | |||
| 30 | + | t.Run(mode+"/"+tc.name, func(t *testing.T) { | |||
| 31 | + | home := t.TempDir() | |||
| 32 | + | dir := filepath.Join(home, "extensions", tc.name) | |||
| 33 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |||
| 34 | + | t.Fatal(err) | |||
| 35 | + | } | |||
| 36 | + | if err := os.WriteFile(filepath.Join(dir, "extension.json"), []byte(tc.manifest), 0o644); err != nil { | |||
| 37 | + | t.Fatal(err) | |||
| 38 | + | } | |||
| 39 | + | if tc.name == "open-log" { | |||
| 40 | + | if err := os.WriteFile(filepath.Join(home, "logs"), nil, 0o644); err != nil { | |||
| 41 | + | t.Fatal(err) | |||
| 42 | + | } | |||
| 43 | + | } | |||
| 44 | + | if tc.name == "missing-runtime" { | |||
| 45 | + | t.Setenv("PATH", t.TempDir()) | |||
| 46 | + | } | |||
| 47 | + | mgr := New(home, "", "test", "", "", nil) | |||
| 48 | + | var errs []error | |||
| 49 | + | if explicit { | |||
| 50 | + | errs = mgr.LoadExplicit(context.Background(), []string{dir}) | |||
| 51 | + | } else { | |||
| 52 | + | errs = mgr.Discover(context.Background()) | |||
| 53 | + | } | |||
| 54 | + | if len(errs) != 1 { | |||
| 55 | + | t.Fatalf("errors = %v, want one", errs) | |||
| 56 | + | } | |||
| 57 | + | if got := errs[0].Error(); !strings.Contains(got, dir) || !strings.Contains(got, tc.want) { | |||
| 58 | + | t.Fatalf("error = %q, want directory %q and %q", got, dir, tc.want) | |||
| 59 | + | } | |||
| 60 | + | if tc.name == "missing-runtime" { | |||
| 61 | + | if !errors.Is(errs[0], exec.ErrNotFound) { | |||
| 62 | + | t.Fatalf("error does not wrap exec.ErrNotFound: %v", errs[0]) | |||
| 63 | + | } | |||
| 64 | + | if strings.Count(errs[0].Error(), dir) != 1 { | |||
| 65 | + | t.Fatalf("directory should appear once: %v", errs[0]) | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | }) | |||
| 69 | + | } | |||
| 70 | + | } | |||
| 71 | + | } | |||
M packages/tui/view.go +10 -10
| @@ -319,17 +319,17 @@ func (v *View) renderErr(width int) []string { | |||
| 319 | 319 | if wrapWidth < 8 { | |
| 320 | 320 | wrapWidth = 8 | |
| 321 | 321 | } | |
| 322 | - | wrapped := wrapLine(v.Err, wrapWidth, "") | |
| 323 | - | if len(wrapped) == 0 { | |
| 324 | - | wrapped = []string{""} | |
| 325 | - | } | |
| 326 | - | out := make([]string, 0, len(wrapped)) | |
| 327 | - | for idx, line := range wrapped { | |
| 328 | - | prefix := marker | |
| 329 | - | if idx > 0 { | |
| 330 | - | prefix = indent | |
| 322 | + | var out []string | |
| 323 | + | // Each returned row must represent one terminal line, including when | |
| 324 | + | // the error contains explicit newlines or blank separator lines. | |
| 325 | + | for _, paragraph := range strings.Split(v.Err, "\n") { | |
| 326 | + | for _, line := range wrapLine(paragraph, wrapWidth, "") { | |
| 327 | + | prefix := indent | |
| 328 | + | if len(out) == 0 { | |
| 329 | + | prefix = marker | |
| 330 | + | } | |
| 331 | + | out = append(out, v.Theme.FG256(v.Theme.Error, prefix+line)) | |
| 331 | 332 | } | |
| 332 | - | out = append(out, v.Theme.FG256(v.Theme.Error, prefix+line)) | |
| 333 | 333 | } | |
| 334 | 334 | return out | |
| 335 | 335 | } | |
| @@ -319,17 +319,17 @@ func (v *View) renderErr(width int) []string { | |||||
| 319 | if wrapWidth < 8 { | 319 | if wrapWidth < 8 { | ||
| 320 | wrapWidth = 8 | 320 | wrapWidth = 8 | ||
| 321 | } | 321 | } | ||
| 322 | - | wrapped := wrapLine(v.Err, wrapWidth, "") | 322 | + | var out []string |
| 323 | - | if len(wrapped) == 0 { | 323 | + | // Each returned row must represent one terminal line, including when |
| 324 | - | wrapped = []string{""} | 324 | + | // the error contains explicit newlines or blank separator lines. |
| 325 | - | } | 325 | + | for _, paragraph := range strings.Split(v.Err, "\n") { |
| 326 | - | out := make([]string, 0, len(wrapped)) | 326 | + | for _, line := range wrapLine(paragraph, wrapWidth, "") { |
| 327 | - | for idx, line := range wrapped { | 327 | + | prefix := indent |
| 328 | - | prefix := marker | 328 | + | if len(out) == 0 { |
| 329 | - | if idx > 0 { | 329 | + | prefix = marker |
| 330 | - | prefix = indent | 330 | + | } |
| 331 | + | out = append(out, v.Theme.FG256(v.Theme.Error, prefix+line)) | |||
| 331 | } | 332 | } | ||
| 332 | - | out = append(out, v.Theme.FG256(v.Theme.Error, prefix+line)) | |||
| 333 | } | 333 | } | ||
| 334 | return out | 334 | return out | ||
| 335 | } | 335 | } | ||
A packages/tui/view_error_test.go +28 -0
| @@ -0,0 +1,28 @@ | |||
| 1 | + | package tui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "strings" | |
| 5 | + | "testing" | |
| 6 | + | ) | |
| 7 | + | ||
| 8 | + | func TestRenderErrSplitsExplicitNewlines(t *testing.T) { | |
| 9 | + | v := &View{Theme: Theme{Error: 1}, Err: "failed to start\n\n exec: python3\nstderr log: /tmp/ext.log"} | |
| 10 | + | want := []string{"✖ failed to start", " ", " exec: python3", " stderr log: /tmp/ext.log"} | |
| 11 | + | rows := v.renderErr(80) | |
| 12 | + | if len(rows) != len(want) { | |
| 13 | + | t.Fatalf("rows = %q, want %d rows", rows, len(want)) | |
| 14 | + | } | |
| 15 | + | for n, row := range rows { | |
| 16 | + | if expected := v.Theme.FG256(v.Theme.Error, want[n]); row != expected { | |
| 17 | + | t.Errorf("row %d = %q, want %q", n, row, expected) | |
| 18 | + | } | |
| 19 | + | } | |
| 20 | + | for _, row := range v.renderErr(20) { | |
| 21 | + | if strings.ContainsAny(row, "\r\n") { | |
| 22 | + | t.Errorf("row contains a newline: %q", row) | |
| 23 | + | } | |
| 24 | + | if width := visibleWidth(row); width > 20 { | |
| 25 | + | t.Errorf("row width = %d, want <= 20: %q", width, row) | |
| 26 | + | } | |
| 27 | + | } | |
| 28 | + | } | |
| @@ -0,0 +1,28 @@ | |||||
| 1 | + | package tui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "strings" | |||
| 5 | + | "testing" | |||
| 6 | + | ) | |||
| 7 | + | ||||
| 8 | + | func TestRenderErrSplitsExplicitNewlines(t *testing.T) { | |||
| 9 | + | v := &View{Theme: Theme{Error: 1}, Err: "failed to start\n\n exec: python3\nstderr log: /tmp/ext.log"} | |||
| 10 | + | want := []string{"✖ failed to start", " ", " exec: python3", " stderr log: /tmp/ext.log"} | |||
| 11 | + | rows := v.renderErr(80) | |||
| 12 | + | if len(rows) != len(want) { | |||
| 13 | + | t.Fatalf("rows = %q, want %d rows", rows, len(want)) | |||
| 14 | + | } | |||
| 15 | + | for n, row := range rows { | |||
| 16 | + | if expected := v.Theme.FG256(v.Theme.Error, want[n]); row != expected { | |||
| 17 | + | t.Errorf("row %d = %q, want %q", n, row, expected) | |||
| 18 | + | } | |||
| 19 | + | } | |||
| 20 | + | for _, row := range v.renderErr(20) { | |||
| 21 | + | if strings.ContainsAny(row, "\r\n") { | |||
| 22 | + | t.Errorf("row contains a newline: %q", row) | |||
| 23 | + | } | |||
| 24 | + | if width := visibleWidth(row); width > 20 { | |||
| 25 | + | t.Errorf("row width = %d, want <= 20: %q", width, row) | |||
| 26 | + | } | |||
| 27 | + | } | |||
| 28 | + | } | |||