Merge remote-tracking branch 'origin/main'
8 files · +189 -27
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 +15 -7
| @@ -229,7 +229,7 @@ func (m *Manager) Discover(ctx context.Context) []error { | |||
| 229 | 229 | go func(extDir string) { | |
| 230 | 230 | defer wg.Done() | |
| 231 | 231 | if err := m.loadOne(ctx, extDir); err != nil { | |
| 232 | - | errCh <- fmt.Errorf("%s: %w", extDir, err) | |
| 232 | + | errCh <- err | |
| 233 | 233 | } | |
| 234 | 234 | }(j.dir) | |
| 235 | 235 | } | |
| @@ -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) }) | |
| @@ -506,7 +514,7 @@ func (m *Manager) LoadExplicit(ctx context.Context, paths []string) []error { | |||
| 506 | 514 | go func(extDir string) { | |
| 507 | 515 | defer wg.Done() | |
| 508 | 516 | if err := m.loadOne(ctx, extDir); err != nil { | |
| 509 | - | errCh <- fmt.Errorf("%s: %w", extDir, err) | |
| 517 | + | errCh <- err | |
| 510 | 518 | } | |
| 511 | 519 | }(abs) | |
| 512 | 520 | } | |
| @@ -727,7 +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 | - | return fmt.Errorf("spawn (stderr log: %s): %w", logPath, err) | |
| 738 | + | return err | |
| 731 | 739 | } | |
| 732 | 740 | started = true | |
| 733 | 741 | ext.cmd = cmd | |
| @@ -229,7 +229,7 @@ func (m *Manager) Discover(ctx context.Context) []error { | |||||
| 229 | go func(extDir string) { | 229 | go func(extDir string) { | ||
| 230 | defer wg.Done() | 230 | defer wg.Done() | ||
| 231 | if err := m.loadOne(ctx, extDir); err != nil { | 231 | if err := m.loadOne(ctx, extDir); err != nil { | ||
| 232 | - | errCh <- fmt.Errorf("%s: %w", extDir, err) | 232 | + | errCh <- err |
| 233 | } | 233 | } | ||
| 234 | }(j.dir) | 234 | }(j.dir) | ||
| 235 | } | 235 | } | ||
| @@ -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) }) | ||
| @@ -506,7 +514,7 @@ func (m *Manager) LoadExplicit(ctx context.Context, paths []string) []error { | |||||
| 506 | go func(extDir string) { | 514 | go func(extDir string) { | ||
| 507 | defer wg.Done() | 515 | defer wg.Done() | ||
| 508 | if err := m.loadOne(ctx, extDir); err != nil { | 516 | if err := m.loadOne(ctx, extDir); err != nil { | ||
| 509 | - | errCh <- fmt.Errorf("%s: %w", extDir, err) | 517 | + | errCh <- err |
| 510 | } | 518 | } | ||
| 511 | }(abs) | 519 | }(abs) | ||
| 512 | } | 520 | } | ||
| @@ -727,7 +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 | - | return fmt.Errorf("spawn (stderr log: %s): %w", logPath, err) | 738 | + | return err |
| 731 | } | 739 | } | ||
| 732 | started = true | 740 | started = true | ||
| 733 | 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/agent/extensions/manager_test.go +31 -0
| @@ -160,6 +160,37 @@ func TestDiscoverReportsExitStatusAndLogWhenExtensionExitsBeforeHello(t *testing | |||
| 160 | 160 | } | |
| 161 | 161 | } | |
| 162 | 162 | ||
| 163 | + | func TestDiscoverReportsMissingRuntimeWhenExtensionCannotStart(t *testing.T) { | |
| 164 | + | tmp := t.TempDir() | |
| 165 | + | extDir := filepath.Join(tmp, "extensions", "missing-runtime") | |
| 166 | + | if err := os.MkdirAll(extDir, 0o755); err != nil { | |
| 167 | + | t.Fatal(err) | |
| 168 | + | } | |
| 169 | + | if err := os.WriteFile(filepath.Join(extDir, "extension.json"), []byte(`{"name":"missing-runtime","exec":"python3","language":"python"}`), 0o644); err != nil { | |
| 170 | + | t.Fatal(err) | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | // Isolate PATH so this tests a missing language runtime rather than | |
| 174 | + | // relying on a made-up executable name that could theoretically exist. | |
| 175 | + | t.Setenv("PATH", filepath.Join(tmp, "empty-bin")) | |
| 176 | + | mgr := New(tmp, "", "0.0.0-test", "", "", nil) | |
| 177 | + | errs := mgr.Discover(context.Background()) | |
| 178 | + | if len(errs) != 1 { | |
| 179 | + | t.Fatalf("discover errors = %v, want one", errs) | |
| 180 | + | } | |
| 181 | + | got := errs[0].Error() | |
| 182 | + | for _, want := range []string{ | |
| 183 | + | `Extension missing-runtime failed to start.`, | |
| 184 | + | `exec: "python3"`, | |
| 185 | + | `declared language: "python"`, | |
| 186 | + | "executable file not found", | |
| 187 | + | } { | |
| 188 | + | if !strings.Contains(got, want) { | |
| 189 | + | t.Fatalf("error = %q, want it to contain %q", got, want) | |
| 190 | + | } | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | ||
| 163 | 194 | func TestDiscoverReportsSignalWhenExtensionExitsBeforeHello(t *testing.T) { | |
| 164 | 195 | if runtime.GOOS == "windows" { | |
| 165 | 196 | t.Skip("mock extension uses Unix signals; skip on windows") | |
| @@ -160,6 +160,37 @@ func TestDiscoverReportsExitStatusAndLogWhenExtensionExitsBeforeHello(t *testing | |||||
| 160 | } | 160 | } | ||
| 161 | } | 161 | } | ||
| 162 | 162 | ||||
| 163 | + | func TestDiscoverReportsMissingRuntimeWhenExtensionCannotStart(t *testing.T) { | |||
| 164 | + | tmp := t.TempDir() | |||
| 165 | + | extDir := filepath.Join(tmp, "extensions", "missing-runtime") | |||
| 166 | + | if err := os.MkdirAll(extDir, 0o755); err != nil { | |||
| 167 | + | t.Fatal(err) | |||
| 168 | + | } | |||
| 169 | + | if err := os.WriteFile(filepath.Join(extDir, "extension.json"), []byte(`{"name":"missing-runtime","exec":"python3","language":"python"}`), 0o644); err != nil { | |||
| 170 | + | t.Fatal(err) | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | // Isolate PATH so this tests a missing language runtime rather than | |||
| 174 | + | // relying on a made-up executable name that could theoretically exist. | |||
| 175 | + | t.Setenv("PATH", filepath.Join(tmp, "empty-bin")) | |||
| 176 | + | mgr := New(tmp, "", "0.0.0-test", "", "", nil) | |||
| 177 | + | errs := mgr.Discover(context.Background()) | |||
| 178 | + | if len(errs) != 1 { | |||
| 179 | + | t.Fatalf("discover errors = %v, want one", errs) | |||
| 180 | + | } | |||
| 181 | + | got := errs[0].Error() | |||
| 182 | + | for _, want := range []string{ | |||
| 183 | + | `Extension missing-runtime failed to start.`, | |||
| 184 | + | `exec: "python3"`, | |||
| 185 | + | `declared language: "python"`, | |||
| 186 | + | "executable file not found", | |||
| 187 | + | } { | |||
| 188 | + | if !strings.Contains(got, want) { | |||
| 189 | + | t.Fatalf("error = %q, want it to contain %q", got, want) | |||
| 190 | + | } | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | ||||
| 163 | func TestDiscoverReportsSignalWhenExtensionExitsBeforeHello(t *testing.T) { | 194 | func TestDiscoverReportsSignalWhenExtensionExitsBeforeHello(t *testing.T) { | ||
| 164 | if runtime.GOOS == "windows" { | 195 | if runtime.GOOS == "windows" { | ||
| 165 | t.Skip("mock extension uses Unix signals; skip on windows") | 196 | t.Skip("mock extension uses Unix signals; skip on windows") | ||
M packages/agent/modes/reload_ext.go +14 -9
| @@ -55,16 +55,21 @@ func renderReloadErrors(theme tui.Theme, errors []string, width int) []string { | |||
| 55 | 55 | } | |
| 56 | 56 | var out []string | |
| 57 | 57 | for _, msg := range errors { | |
| 58 | - | wrapped := tui.WrapANSILine(msg, wrapWidth) | |
| 59 | - | if len(wrapped) == 0 { | |
| 60 | - | wrapped = []string{""} | |
| 61 | - | } | |
| 62 | - | for idx, line := range wrapped { | |
| 63 | - | prefix := marker | |
| 64 | - | if idx > 0 { | |
| 65 | - | prefix = indent | |
| 58 | + | // Split explicit newlines before wrapping. Passing a multiline string | |
| 59 | + | // to WrapANSILine would leave the terminal to interpret the embedded | |
| 60 | + | // newline, which can misalign the continuation text. | |
| 61 | + | for lineIndex, paragraph := range strings.Split(msg, "\n") { | |
| 62 | + | wrapped := tui.WrapANSILine(paragraph, wrapWidth) | |
| 63 | + | if len(wrapped) == 0 { | |
| 64 | + | wrapped = []string{""} | |
| 65 | + | } | |
| 66 | + | for wrapIndex, line := range wrapped { | |
| 67 | + | prefix := indent | |
| 68 | + | if lineIndex == 0 && wrapIndex == 0 { | |
| 69 | + | prefix = marker | |
| 70 | + | } | |
| 71 | + | out = append(out, theme.FG256(theme.Error, prefix+line)) | |
| 66 | 72 | } | |
| 67 | - | out = append(out, theme.FG256(theme.Error, prefix+line)) | |
| 68 | 73 | } | |
| 69 | 74 | } | |
| 70 | 75 | return out | |
| @@ -55,16 +55,21 @@ func renderReloadErrors(theme tui.Theme, errors []string, width int) []string { | |||||
| 55 | } | 55 | } | ||
| 56 | var out []string | 56 | var out []string | ||
| 57 | for _, msg := range errors { | 57 | for _, msg := range errors { | ||
| 58 | - | wrapped := tui.WrapANSILine(msg, wrapWidth) | 58 | + | // Split explicit newlines before wrapping. Passing a multiline string |
| 59 | - | if len(wrapped) == 0 { | 59 | + | // to WrapANSILine would leave the terminal to interpret the embedded |
| 60 | - | wrapped = []string{""} | 60 | + | // newline, which can misalign the continuation text. |
| 61 | - | } | 61 | + | for lineIndex, paragraph := range strings.Split(msg, "\n") { |
| 62 | - | for idx, line := range wrapped { | 62 | + | wrapped := tui.WrapANSILine(paragraph, wrapWidth) |
| 63 | - | prefix := marker | 63 | + | if len(wrapped) == 0 { |
| 64 | - | if idx > 0 { | 64 | + | wrapped = []string{""} |
| 65 | - | prefix = indent | 65 | + | } |
| 66 | + | for wrapIndex, line := range wrapped { | |||
| 67 | + | prefix := indent | |||
| 68 | + | if lineIndex == 0 && wrapIndex == 0 { | |||
| 69 | + | prefix = marker | |||
| 70 | + | } | |||
| 71 | + | out = append(out, theme.FG256(theme.Error, prefix+line)) | |||
| 66 | } | 72 | } | ||
| 67 | - | out = append(out, theme.FG256(theme.Error, prefix+line)) | |||
| 68 | } | 73 | } | ||
| 69 | } | 74 | } | ||
| 70 | return out | 75 | return out | ||
M packages/agent/modes/reload_ext_test.go +15 -0
| @@ -35,6 +35,21 @@ func TestFormatReloadStatusIncludesErrorDetails(t *testing.T) { | |||
| 35 | 35 | } | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | + | func TestRenderReloadErrorsPreservesMultilineAlignment(t *testing.T) { | |
| 39 | + | lines := renderReloadErrors(tui.Theme{Error: 1}, []string{"failed to start\nextension directory: /tmp/ext\nstderr log: /tmp/ext.log"}, 80) | |
| 40 | + | if len(lines) != 3 { | |
| 41 | + | t.Fatalf("rendered %d lines, want 3: %q", len(lines), lines) | |
| 42 | + | } | |
| 43 | + | if !strings.Contains(lines[0], "✖ failed to start") { | |
| 44 | + | t.Fatalf("first line = %q, want error marker", lines[0]) | |
| 45 | + | } | |
| 46 | + | for _, line := range lines[1:] { | |
| 47 | + | if !strings.Contains(line, " ") { | |
| 48 | + | t.Fatalf("continuation line = %q, want indentation", line) | |
| 49 | + | } | |
| 50 | + | } | |
| 51 | + | } | |
| 52 | + | ||
| 38 | 53 | func TestFormatReloadStatusSuccess(t *testing.T) { | |
| 39 | 54 | msg, failed := formatReloadStatus(extensions.ReloadStats{Stopped: 2, Loaded: 3, Ready: 3}) | |
| 40 | 55 | if failed { | |
| @@ -35,6 +35,21 @@ func TestFormatReloadStatusIncludesErrorDetails(t *testing.T) { | |||||
| 35 | } | 35 | } | ||
| 36 | } | 36 | } | ||
| 37 | 37 | ||||
| 38 | + | func TestRenderReloadErrorsPreservesMultilineAlignment(t *testing.T) { | |||
| 39 | + | lines := renderReloadErrors(tui.Theme{Error: 1}, []string{"failed to start\nextension directory: /tmp/ext\nstderr log: /tmp/ext.log"}, 80) | |||
| 40 | + | if len(lines) != 3 { | |||
| 41 | + | t.Fatalf("rendered %d lines, want 3: %q", len(lines), lines) | |||
| 42 | + | } | |||
| 43 | + | if !strings.Contains(lines[0], "✖ failed to start") { | |||
| 44 | + | t.Fatalf("first line = %q, want error marker", lines[0]) | |||
| 45 | + | } | |||
| 46 | + | for _, line := range lines[1:] { | |||
| 47 | + | if !strings.Contains(line, " ") { | |||
| 48 | + | t.Fatalf("continuation line = %q, want indentation", line) | |||
| 49 | + | } | |||
| 50 | + | } | |||
| 51 | + | } | |||
| 52 | + | ||||
| 38 | func TestFormatReloadStatusSuccess(t *testing.T) { | 53 | func TestFormatReloadStatusSuccess(t *testing.T) { | ||
| 39 | msg, failed := formatReloadStatus(extensions.ReloadStats{Stopped: 2, Loaded: 3, Ready: 3}) | 54 | msg, failed := formatReloadStatus(extensions.ReloadStats{Stopped: 2, Loaded: 3, Ready: 3}) | ||
| 40 | if failed { | 55 | if failed { | ||
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 | + | } | |||