fix: show extension reload errors
Fixes #101
- commit
- 82dd8e41c88ec5999ab22e043bb8b00bd015d3f7
- author
- patriceckhart · · 2026-07-24 07:59 UTC
- parent
- 0341161
- signature
- unsigned
4 files · +170 -10
M docs/extensions.md +3 -1
| @@ -640,7 +640,9 @@ The agent's tool registry is rebuilt automatically, so freshly- | |||
| 640 | 640 | registered extension tools become callable without restarting zot. | |
| 641 | 641 | Useful while developing an extension: edit, save, `/reload-ext`, | |
| 642 | 642 | done. Explicit `--ext` paths are remembered and reloaded alongside | |
| 643 | - | discovered extensions. | |
| 643 | + | discovered extensions. The temporary reload status reports each load | |
| 644 | + | error in red, including its message, and dismisses itself after five | |
| 645 | + | seconds. | |
| 644 | 646 | ||
| 645 | 647 | ### TypeScript / Python | |
| 646 | 648 | ||
| @@ -640,7 +640,9 @@ The agent's tool registry is rebuilt automatically, so freshly- | |||||
| 640 | registered extension tools become callable without restarting zot. | 640 | registered extension tools become callable without restarting zot. | ||
| 641 | Useful while developing an extension: edit, save, `/reload-ext`, | 641 | Useful while developing an extension: edit, save, `/reload-ext`, | ||
| 642 | done. Explicit `--ext` paths are remembered and reloaded alongside | 642 | done. Explicit `--ext` paths are remembered and reloaded alongside | ||
| 643 | - | discovered extensions. | 643 | + | discovered extensions. The temporary reload status reports each load |
| 644 | + | error in red, including its message, and dismisses itself after five | |||
| 645 | + | seconds. | |||
| 644 | 646 | ||||
| 645 | ### TypeScript / Python | 647 | ### TypeScript / Python | ||
| 646 | 648 | ||||
M packages/agent/modes/interactive.go +5 -9
| @@ -359,6 +359,7 @@ type Interactive struct { | |||
| 359 | 359 | toolGate map[string]int | |
| 360 | 360 | statusErr string | |
| 361 | 361 | statusOK string | |
| 362 | + | reloadStatusSeq uint64 | |
| 362 | 363 | liveBlock []string // live streaming/tool progress rendered outside scrollback | |
| 363 | 364 | helpBlock []string // rendered above the chat when /help was typed | |
| 364 | 365 | cumUsage provider.Usage | |
| @@ -5726,7 +5727,7 @@ var _ = fmt.Sprintf | |||
| 5726 | 5727 | // runReloadExt triggers a live reload of every extension (discovered | |
| 5727 | 5728 | // + explicit). Runs on a goroutine so the TUI stays responsive; the | |
| 5728 | 5729 | // Manager.Reload takes a couple of hundred ms to shut down subprocs | |
| 5729 | - | // and respawn them. Shows a status line throughout. | |
| 5730 | + | // and respawn them. Shows a temporary status line throughout. | |
| 5730 | 5731 | func (i *Interactive) runReloadExt(ctx context.Context) { | |
| 5731 | 5732 | if i.cfg.Extensions == nil { | |
| 5732 | 5733 | i.mu.Lock() | |
| @@ -5743,15 +5744,10 @@ func (i *Interactive) runReloadExt(ctx context.Context) { | |||
| 5743 | 5744 | ||
| 5744 | 5745 | go func() { | |
| 5745 | 5746 | stats := i.cfg.Extensions.Reload(ctx, 2*time.Second) | |
| 5746 | - | msg := fmt.Sprintf("reloaded: %d stopped, %d loaded (%d ready)", stats.Stopped, stats.Loaded, stats.Ready) | |
| 5747 | - | if len(stats.Errors) > 0 { | |
| 5748 | - | msg += fmt.Sprintf(", %d error(s)", len(stats.Errors)) | |
| 5749 | - | } | |
| 5750 | - | i.mu.Lock() | |
| 5751 | - | i.statusOK = msg | |
| 5752 | - | i.statusErr = "" | |
| 5753 | - | i.mu.Unlock() | |
| 5747 | + | msg, failed := formatReloadStatus(stats) | |
| 5748 | + | seq := i.setReloadStatus(msg, failed) | |
| 5754 | 5749 | i.invalidate() | |
| 5750 | + | go i.dismissReloadStatus(ctx, seq, msg, failed) | |
| 5755 | 5751 | }() | |
| 5756 | 5752 | } | |
| 5757 | 5753 | ||
| @@ -359,6 +359,7 @@ type Interactive struct { | |||||
| 359 | toolGate map[string]int | 359 | toolGate map[string]int | ||
| 360 | statusErr string | 360 | statusErr string | ||
| 361 | statusOK string | 361 | statusOK string | ||
| 362 | + | reloadStatusSeq uint64 | |||
| 362 | liveBlock []string // live streaming/tool progress rendered outside scrollback | 363 | liveBlock []string // live streaming/tool progress rendered outside scrollback | ||
| 363 | helpBlock []string // rendered above the chat when /help was typed | 364 | helpBlock []string // rendered above the chat when /help was typed | ||
| 364 | cumUsage provider.Usage | 365 | cumUsage provider.Usage | ||
| @@ -5726,7 +5727,7 @@ var _ = fmt.Sprintf | |||||
| 5726 | // runReloadExt triggers a live reload of every extension (discovered | 5727 | // runReloadExt triggers a live reload of every extension (discovered | ||
| 5727 | // + explicit). Runs on a goroutine so the TUI stays responsive; the | 5728 | // + explicit). Runs on a goroutine so the TUI stays responsive; the | ||
| 5728 | // Manager.Reload takes a couple of hundred ms to shut down subprocs | 5729 | // Manager.Reload takes a couple of hundred ms to shut down subprocs | ||
| 5729 | - | // and respawn them. Shows a status line throughout. | 5730 | + | // and respawn them. Shows a temporary status line throughout. |
| 5730 | func (i *Interactive) runReloadExt(ctx context.Context) { | 5731 | func (i *Interactive) runReloadExt(ctx context.Context) { | ||
| 5731 | if i.cfg.Extensions == nil { | 5732 | if i.cfg.Extensions == nil { | ||
| 5732 | i.mu.Lock() | 5733 | i.mu.Lock() | ||
| @@ -5743,15 +5744,10 @@ func (i *Interactive) runReloadExt(ctx context.Context) { | |||||
| 5743 | 5744 | ||||
| 5744 | go func() { | 5745 | go func() { | ||
| 5745 | stats := i.cfg.Extensions.Reload(ctx, 2*time.Second) | 5746 | stats := i.cfg.Extensions.Reload(ctx, 2*time.Second) | ||
| 5746 | - | msg := fmt.Sprintf("reloaded: %d stopped, %d loaded (%d ready)", stats.Stopped, stats.Loaded, stats.Ready) | 5747 | + | msg, failed := formatReloadStatus(stats) |
| 5747 | - | if len(stats.Errors) > 0 { | 5748 | + | seq := i.setReloadStatus(msg, failed) |
| 5748 | - | msg += fmt.Sprintf(", %d error(s)", len(stats.Errors)) | |||
| 5749 | - | } | |||
| 5750 | - | i.mu.Lock() | |||
| 5751 | - | i.statusOK = msg | |||
| 5752 | - | i.statusErr = "" | |||
| 5753 | - | i.mu.Unlock() | |||
| 5754 | i.invalidate() | 5749 | i.invalidate() | ||
| 5750 | + | go i.dismissReloadStatus(ctx, seq, msg, failed) | |||
| 5755 | }() | 5751 | }() | ||
| 5756 | } | 5752 | } | ||
| 5757 | 5753 | ||||
A packages/agent/modes/reload_ext.go +77 -0
| @@ -0,0 +1,77 @@ | |||
| 1 | + | package modes | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "fmt" | |
| 6 | + | "strings" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/patriceckhart/zot/packages/agent/extensions" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | const reloadStatusDuration = 5 * time.Second | |
| 13 | + | ||
| 14 | + | func formatReloadStatus(stats extensions.ReloadStats) (string, bool) { | |
| 15 | + | msg := fmt.Sprintf("reloaded: %d stopped, %d loaded (%d ready)", stats.Stopped, stats.Loaded, stats.Ready) | |
| 16 | + | if len(stats.Errors) == 0 { | |
| 17 | + | return msg, false | |
| 18 | + | } | |
| 19 | + | details := make([]string, 0, len(stats.Errors)) | |
| 20 | + | for _, err := range stats.Errors { | |
| 21 | + | if err != nil { | |
| 22 | + | details = append(details, err.Error()) | |
| 23 | + | } | |
| 24 | + | } | |
| 25 | + | msg += fmt.Sprintf(", %d error(s)", len(stats.Errors)) | |
| 26 | + | if len(details) > 0 { | |
| 27 | + | msg += ": " + strings.Join(details, "; ") | |
| 28 | + | } | |
| 29 | + | return msg, true | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | func (i *Interactive) setReloadStatus(msg string, failed bool) uint64 { | |
| 33 | + | i.mu.Lock() | |
| 34 | + | defer i.mu.Unlock() | |
| 35 | + | i.reloadStatusSeq++ | |
| 36 | + | if failed { | |
| 37 | + | i.statusOK = "" | |
| 38 | + | i.statusErr = msg | |
| 39 | + | return i.reloadStatusSeq | |
| 40 | + | } | |
| 41 | + | i.statusOK = msg | |
| 42 | + | i.statusErr = "" | |
| 43 | + | return i.reloadStatusSeq | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func (i *Interactive) clearReloadStatus(seq uint64, msg string, failed bool) bool { | |
| 47 | + | i.mu.Lock() | |
| 48 | + | defer i.mu.Unlock() | |
| 49 | + | if i.reloadStatusSeq != seq { | |
| 50 | + | return false | |
| 51 | + | } | |
| 52 | + | if failed { | |
| 53 | + | if i.statusErr != msg { | |
| 54 | + | return false | |
| 55 | + | } | |
| 56 | + | i.statusErr = "" | |
| 57 | + | return true | |
| 58 | + | } | |
| 59 | + | if i.statusOK != msg { | |
| 60 | + | return false | |
| 61 | + | } | |
| 62 | + | i.statusOK = "" | |
| 63 | + | return true | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | func (i *Interactive) dismissReloadStatus(ctx context.Context, seq uint64, msg string, failed bool) { | |
| 67 | + | timer := time.NewTimer(reloadStatusDuration) | |
| 68 | + | defer timer.Stop() | |
| 69 | + | select { | |
| 70 | + | case <-ctx.Done(): | |
| 71 | + | return | |
| 72 | + | case <-timer.C: | |
| 73 | + | if i.clearReloadStatus(seq, msg, failed) { | |
| 74 | + | i.invalidate() | |
| 75 | + | } | |
| 76 | + | } | |
| 77 | + | } | |
| @@ -0,0 +1,77 @@ | |||||
| 1 | + | package modes | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "fmt" | |||
| 6 | + | "strings" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/patriceckhart/zot/packages/agent/extensions" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | const reloadStatusDuration = 5 * time.Second | |||
| 13 | + | ||||
| 14 | + | func formatReloadStatus(stats extensions.ReloadStats) (string, bool) { | |||
| 15 | + | msg := fmt.Sprintf("reloaded: %d stopped, %d loaded (%d ready)", stats.Stopped, stats.Loaded, stats.Ready) | |||
| 16 | + | if len(stats.Errors) == 0 { | |||
| 17 | + | return msg, false | |||
| 18 | + | } | |||
| 19 | + | details := make([]string, 0, len(stats.Errors)) | |||
| 20 | + | for _, err := range stats.Errors { | |||
| 21 | + | if err != nil { | |||
| 22 | + | details = append(details, err.Error()) | |||
| 23 | + | } | |||
| 24 | + | } | |||
| 25 | + | msg += fmt.Sprintf(", %d error(s)", len(stats.Errors)) | |||
| 26 | + | if len(details) > 0 { | |||
| 27 | + | msg += ": " + strings.Join(details, "; ") | |||
| 28 | + | } | |||
| 29 | + | return msg, true | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | func (i *Interactive) setReloadStatus(msg string, failed bool) uint64 { | |||
| 33 | + | i.mu.Lock() | |||
| 34 | + | defer i.mu.Unlock() | |||
| 35 | + | i.reloadStatusSeq++ | |||
| 36 | + | if failed { | |||
| 37 | + | i.statusOK = "" | |||
| 38 | + | i.statusErr = msg | |||
| 39 | + | return i.reloadStatusSeq | |||
| 40 | + | } | |||
| 41 | + | i.statusOK = msg | |||
| 42 | + | i.statusErr = "" | |||
| 43 | + | return i.reloadStatusSeq | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func (i *Interactive) clearReloadStatus(seq uint64, msg string, failed bool) bool { | |||
| 47 | + | i.mu.Lock() | |||
| 48 | + | defer i.mu.Unlock() | |||
| 49 | + | if i.reloadStatusSeq != seq { | |||
| 50 | + | return false | |||
| 51 | + | } | |||
| 52 | + | if failed { | |||
| 53 | + | if i.statusErr != msg { | |||
| 54 | + | return false | |||
| 55 | + | } | |||
| 56 | + | i.statusErr = "" | |||
| 57 | + | return true | |||
| 58 | + | } | |||
| 59 | + | if i.statusOK != msg { | |||
| 60 | + | return false | |||
| 61 | + | } | |||
| 62 | + | i.statusOK = "" | |||
| 63 | + | return true | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | func (i *Interactive) dismissReloadStatus(ctx context.Context, seq uint64, msg string, failed bool) { | |||
| 67 | + | timer := time.NewTimer(reloadStatusDuration) | |||
| 68 | + | defer timer.Stop() | |||
| 69 | + | select { | |||
| 70 | + | case <-ctx.Done(): | |||
| 71 | + | return | |||
| 72 | + | case <-timer.C: | |||
| 73 | + | if i.clearReloadStatus(seq, msg, failed) { | |||
| 74 | + | i.invalidate() | |||
| 75 | + | } | |||
| 76 | + | } | |||
| 77 | + | } | |||
A packages/agent/modes/reload_ext_test.go +85 -0
| @@ -0,0 +1,85 @@ | |||
| 1 | + | package modes | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "errors" | |
| 5 | + | "strings" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/patriceckhart/zot/packages/agent/extensions" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestFormatReloadStatusIncludesErrorDetails(t *testing.T) { | |
| 12 | + | msg, failed := formatReloadStatus(extensions.ReloadStats{ | |
| 13 | + | Stopped: 2, | |
| 14 | + | Loaded: 1, | |
| 15 | + | Ready: 1, | |
| 16 | + | Errors: []error{ | |
| 17 | + | errors.New("broken manifest"), | |
| 18 | + | errors.New("spawn worker: executable not found"), | |
| 19 | + | }, | |
| 20 | + | }) | |
| 21 | + | if !failed { | |
| 22 | + | t.Fatal("reload with errors was not marked failed") | |
| 23 | + | } | |
| 24 | + | for _, want := range []string{ | |
| 25 | + | "2 stopped, 1 loaded (1 ready)", | |
| 26 | + | "2 error(s)", | |
| 27 | + | "broken manifest", | |
| 28 | + | "spawn worker: executable not found", | |
| 29 | + | } { | |
| 30 | + | if !strings.Contains(msg, want) { | |
| 31 | + | t.Fatalf("reload status missing %q: %s", want, msg) | |
| 32 | + | } | |
| 33 | + | } | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | func TestFormatReloadStatusSuccess(t *testing.T) { | |
| 37 | + | msg, failed := formatReloadStatus(extensions.ReloadStats{Stopped: 2, Loaded: 3, Ready: 3}) | |
| 38 | + | if failed { | |
| 39 | + | t.Fatal("successful reload was marked failed") | |
| 40 | + | } | |
| 41 | + | if want := "reloaded: 2 stopped, 3 loaded (3 ready)"; msg != want { | |
| 42 | + | t.Fatalf("reload status = %q, want %q", msg, want) | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func TestSetReloadStatusUsesErrorChannelForFailures(t *testing.T) { | |
| 47 | + | i := &Interactive{} | |
| 48 | + | i.setReloadStatus("broken extension", true) | |
| 49 | + | if i.statusErr != "broken extension" || i.statusOK != "" { | |
| 50 | + | t.Fatalf("failed reload status used wrong channel: err=%q ok=%q", i.statusErr, i.statusOK) | |
| 51 | + | } | |
| 52 | + | i.setReloadStatus("reload complete", false) | |
| 53 | + | if i.statusOK != "reload complete" || i.statusErr != "" { | |
| 54 | + | t.Fatalf("successful reload status used wrong channel: err=%q ok=%q", i.statusErr, i.statusOK) | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func TestClearReloadStatusDoesNotClearNewerStatus(t *testing.T) { | |
| 59 | + | i := &Interactive{statusErr: "newer error", statusOK: "newer success"} | |
| 60 | + | if i.clearReloadStatus(0, "old error", true) { | |
| 61 | + | t.Fatal("stale error dismissal reported a clear") | |
| 62 | + | } | |
| 63 | + | if i.clearReloadStatus(0, "old success", false) { | |
| 64 | + | t.Fatal("stale success dismissal reported a clear") | |
| 65 | + | } | |
| 66 | + | if i.statusErr != "newer error" || i.statusOK != "newer success" { | |
| 67 | + | t.Fatalf("stale dismissal changed status: err=%q ok=%q", i.statusErr, i.statusOK) | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | if !i.clearReloadStatus(0, "newer error", true) || i.statusErr != "" { | |
| 71 | + | t.Fatalf("matching error status was not cleared: %q", i.statusErr) | |
| 72 | + | } | |
| 73 | + | if !i.clearReloadStatus(0, "newer success", false) || i.statusOK != "" { | |
| 74 | + | t.Fatalf("matching success status was not cleared: %q", i.statusOK) | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | first := i.setReloadStatus("same result", false) | |
| 78 | + | second := i.setReloadStatus("same result", false) | |
| 79 | + | if first == second { | |
| 80 | + | t.Fatal("reload status generation did not advance") | |
| 81 | + | } | |
| 82 | + | if i.clearReloadStatus(first, "same result", false) { | |
| 83 | + | t.Fatal("older identical reload status cleared the newer result") | |
| 84 | + | } | |
| 85 | + | } | |
| @@ -0,0 +1,85 @@ | |||||
| 1 | + | package modes | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "errors" | |||
| 5 | + | "strings" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/patriceckhart/zot/packages/agent/extensions" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestFormatReloadStatusIncludesErrorDetails(t *testing.T) { | |||
| 12 | + | msg, failed := formatReloadStatus(extensions.ReloadStats{ | |||
| 13 | + | Stopped: 2, | |||
| 14 | + | Loaded: 1, | |||
| 15 | + | Ready: 1, | |||
| 16 | + | Errors: []error{ | |||
| 17 | + | errors.New("broken manifest"), | |||
| 18 | + | errors.New("spawn worker: executable not found"), | |||
| 19 | + | }, | |||
| 20 | + | }) | |||
| 21 | + | if !failed { | |||
| 22 | + | t.Fatal("reload with errors was not marked failed") | |||
| 23 | + | } | |||
| 24 | + | for _, want := range []string{ | |||
| 25 | + | "2 stopped, 1 loaded (1 ready)", | |||
| 26 | + | "2 error(s)", | |||
| 27 | + | "broken manifest", | |||
| 28 | + | "spawn worker: executable not found", | |||
| 29 | + | } { | |||
| 30 | + | if !strings.Contains(msg, want) { | |||
| 31 | + | t.Fatalf("reload status missing %q: %s", want, msg) | |||
| 32 | + | } | |||
| 33 | + | } | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | func TestFormatReloadStatusSuccess(t *testing.T) { | |||
| 37 | + | msg, failed := formatReloadStatus(extensions.ReloadStats{Stopped: 2, Loaded: 3, Ready: 3}) | |||
| 38 | + | if failed { | |||
| 39 | + | t.Fatal("successful reload was marked failed") | |||
| 40 | + | } | |||
| 41 | + | if want := "reloaded: 2 stopped, 3 loaded (3 ready)"; msg != want { | |||
| 42 | + | t.Fatalf("reload status = %q, want %q", msg, want) | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func TestSetReloadStatusUsesErrorChannelForFailures(t *testing.T) { | |||
| 47 | + | i := &Interactive{} | |||
| 48 | + | i.setReloadStatus("broken extension", true) | |||
| 49 | + | if i.statusErr != "broken extension" || i.statusOK != "" { | |||
| 50 | + | t.Fatalf("failed reload status used wrong channel: err=%q ok=%q", i.statusErr, i.statusOK) | |||
| 51 | + | } | |||
| 52 | + | i.setReloadStatus("reload complete", false) | |||
| 53 | + | if i.statusOK != "reload complete" || i.statusErr != "" { | |||
| 54 | + | t.Fatalf("successful reload status used wrong channel: err=%q ok=%q", i.statusErr, i.statusOK) | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | func TestClearReloadStatusDoesNotClearNewerStatus(t *testing.T) { | |||
| 59 | + | i := &Interactive{statusErr: "newer error", statusOK: "newer success"} | |||
| 60 | + | if i.clearReloadStatus(0, "old error", true) { | |||
| 61 | + | t.Fatal("stale error dismissal reported a clear") | |||
| 62 | + | } | |||
| 63 | + | if i.clearReloadStatus(0, "old success", false) { | |||
| 64 | + | t.Fatal("stale success dismissal reported a clear") | |||
| 65 | + | } | |||
| 66 | + | if i.statusErr != "newer error" || i.statusOK != "newer success" { | |||
| 67 | + | t.Fatalf("stale dismissal changed status: err=%q ok=%q", i.statusErr, i.statusOK) | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | if !i.clearReloadStatus(0, "newer error", true) || i.statusErr != "" { | |||
| 71 | + | t.Fatalf("matching error status was not cleared: %q", i.statusErr) | |||
| 72 | + | } | |||
| 73 | + | if !i.clearReloadStatus(0, "newer success", false) || i.statusOK != "" { | |||
| 74 | + | t.Fatalf("matching success status was not cleared: %q", i.statusOK) | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | first := i.setReloadStatus("same result", false) | |||
| 78 | + | second := i.setReloadStatus("same result", false) | |||
| 79 | + | if first == second { | |||
| 80 | + | t.Fatal("reload status generation did not advance") | |||
| 81 | + | } | |||
| 82 | + | if i.clearReloadStatus(first, "same result", false) { | |||
| 83 | + | t.Fatal("older identical reload status cleared the newer result") | |||
| 84 | + | } | |||
| 85 | + | } | |||