Merge branch 'main' into openrouter-live-models
16 files · +751 -23
M docs/extensions.md +17 -2
| @@ -337,7 +337,22 @@ Closes a previously-open panel. | |||
| 337 | 337 | ``` | |
| 338 | 338 | ||
| 339 | 339 | `level` is one of `info`, `success`, `warn`, `error`. The note shows | |
| 340 | - | up below the transcript with the extension's name in brackets. | |
| 340 | + | up below the transcript with the extension's name in brackets. Notes | |
| 341 | + | are one-shot: they clear automatically when the user sends their next | |
| 342 | + | prompt (and on `esc` / `/clear`). | |
| 343 | + | ||
| 344 | + | #### `clear_notes` (one-way, any time) | |
| 345 | + | ||
| 346 | + | Removes every note this extension previously pushed via `notify` / | |
| 347 | + | `display`. Use it for transient status lines (e.g. an approval prompt) | |
| 348 | + | so they do not stack up; notes from other extensions are untouched. | |
| 349 | + | ||
| 350 | + | ```json | |
| 351 | + | {"type":"clear_notes"} | |
| 352 | + | ``` | |
| 353 | + | ||
| 354 | + | In `--mode rpc`, this surfaces to the host as an `ext_clear_notes` | |
| 355 | + | event (alongside `ext_notify` / `ext_display`). | |
| 341 | 356 | ||
| 342 | 357 | #### `shutdown_ack` | |
| 343 | 358 | ||
| @@ -593,7 +608,7 @@ trust or run zot under your platform's sandboxing tool (`bwrap` / | |||
| 593 | 608 | Phase 1 (shipped): | |
| 594 | 609 | - [x] subprocess lifecycle + hello handshake | |
| 595 | 610 | - [x] `register_command` + `command_invoked` | |
| 596 | - | - [x] `notify` | |
| 611 | + | - [x] `notify` + `clear_notes` | |
| 597 | 612 | - [x] `zot ext` CLI | |
| 598 | 613 | ||
| 599 | 614 | Phase 2 (shipped): | |
| @@ -337,7 +337,22 @@ Closes a previously-open panel. | |||||
| 337 | ``` | 337 | ``` | ||
| 338 | 338 | ||||
| 339 | `level` is one of `info`, `success`, `warn`, `error`. The note shows | 339 | `level` is one of `info`, `success`, `warn`, `error`. The note shows | ||
| 340 | - | up below the transcript with the extension's name in brackets. | 340 | + | up below the transcript with the extension's name in brackets. Notes |
| 341 | + | are one-shot: they clear automatically when the user sends their next | |||
| 342 | + | prompt (and on `esc` / `/clear`). | |||
| 343 | + | ||||
| 344 | + | #### `clear_notes` (one-way, any time) | |||
| 345 | + | ||||
| 346 | + | Removes every note this extension previously pushed via `notify` / | |||
| 347 | + | `display`. Use it for transient status lines (e.g. an approval prompt) | |||
| 348 | + | so they do not stack up; notes from other extensions are untouched. | |||
| 349 | + | ||||
| 350 | + | ```json | |||
| 351 | + | {"type":"clear_notes"} | |||
| 352 | + | ``` | |||
| 353 | + | ||||
| 354 | + | In `--mode rpc`, this surfaces to the host as an `ext_clear_notes` | |||
| 355 | + | event (alongside `ext_notify` / `ext_display`). | |||
| 341 | 356 | ||||
| 342 | #### `shutdown_ack` | 357 | #### `shutdown_ack` | ||
| 343 | 358 | ||||
| @@ -593,7 +608,7 @@ trust or run zot under your platform's sandboxing tool (`bwrap` / | |||||
| 593 | Phase 1 (shipped): | 608 | Phase 1 (shipped): | ||
| 594 | - [x] subprocess lifecycle + hello handshake | 609 | - [x] subprocess lifecycle + hello handshake | ||
| 595 | - [x] `register_command` + `command_invoked` | 610 | - [x] `register_command` + `command_invoked` | ||
| 596 | - | - [x] `notify` | 611 | + | - [x] `notify` + `clear_notes` |
| 597 | - [x] `zot ext` CLI | 612 | - [x] `zot ext` CLI | ||
| 598 | 613 | ||||
| 599 | Phase 2 (shipped): | 614 | Phase 2 (shipped): | ||
M packages/agent/build.go +6 -4
| @@ -305,9 +305,6 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||
| 305 | 305 | credErr error | |
| 306 | 306 | ) | |
| 307 | 307 | if provName == "ollama" { | |
| 308 | - | if args.BaseURL == "" { | |
| 309 | - | args.BaseURL = "http://localhost:11434" | |
| 310 | - | } | |
| 311 | 308 | cred = firstNonEmpty(args.APIKey, "ollama") | |
| 312 | 309 | method = "apikey" | |
| 313 | 310 | } else { | |
| @@ -398,10 +395,15 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||
| 398 | 395 | } | |
| 399 | 396 | ||
| 400 | 397 | // If the model defines a base URL (e.g. local ollama) and the | |
| 401 | - | // user didn't pass --base-url, use the model's URL. | |
| 398 | + | // user didn't pass --base-url, use the model's URL. For ollama, | |
| 399 | + | // keep http://localhost:11434 as a fallback only after the model | |
| 400 | + | // metadata has had a chance to provide a custom baseUrl. | |
| 402 | 401 | if args.BaseURL == "" && resolvedModel.BaseURL != "" { | |
| 403 | 402 | args.BaseURL = resolvedModel.BaseURL | |
| 404 | 403 | } | |
| 404 | + | if args.BaseURL == "" && provName == "ollama" { | |
| 405 | + | args.BaseURL = "http://localhost:11434" | |
| 406 | + | } | |
| 405 | 407 | ||
| 406 | 408 | // If the model has a base URL, credentials are optional (local | |
| 407 | 409 | // models like ollama don't need real API keys). | |
| @@ -305,9 +305,6 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||||
| 305 | credErr error | 305 | credErr error | ||
| 306 | ) | 306 | ) | ||
| 307 | if provName == "ollama" { | 307 | if provName == "ollama" { | ||
| 308 | - | if args.BaseURL == "" { | |||
| 309 | - | args.BaseURL = "http://localhost:11434" | |||
| 310 | - | } | |||
| 311 | cred = firstNonEmpty(args.APIKey, "ollama") | 308 | cred = firstNonEmpty(args.APIKey, "ollama") | ||
| 312 | method = "apikey" | 309 | method = "apikey" | ||
| 313 | } else { | 310 | } else { | ||
| @@ -398,10 +395,15 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||||
| 398 | } | 395 | } | ||
| 399 | 396 | ||||
| 400 | // If the model defines a base URL (e.g. local ollama) and the | 397 | // If the model defines a base URL (e.g. local ollama) and the | ||
| 401 | - | // user didn't pass --base-url, use the model's URL. | 398 | + | // user didn't pass --base-url, use the model's URL. For ollama, |
| 399 | + | // keep http://localhost:11434 as a fallback only after the model | |||
| 400 | + | // metadata has had a chance to provide a custom baseUrl. | |||
| 402 | if args.BaseURL == "" && resolvedModel.BaseURL != "" { | 401 | if args.BaseURL == "" && resolvedModel.BaseURL != "" { | ||
| 403 | args.BaseURL = resolvedModel.BaseURL | 402 | args.BaseURL = resolvedModel.BaseURL | ||
| 404 | } | 403 | } | ||
| 404 | + | if args.BaseURL == "" && provName == "ollama" { | |||
| 405 | + | args.BaseURL = "http://localhost:11434" | |||
| 406 | + | } | |||
| 405 | 407 | ||||
| 406 | // If the model has a base URL, credentials are optional (local | 408 | // If the model has a base URL, credentials are optional (local | ||
| 407 | // models like ollama don't need real API keys). | 409 | // models like ollama don't need real API keys). | ||
M packages/agent/build_test.go +38 -0
| @@ -5,6 +5,8 @@ import ( | |||
| 5 | 5 | "path/filepath" | |
| 6 | 6 | "strings" | |
| 7 | 7 | "testing" | |
| 8 | + | ||
| 9 | + | "github.com/patriceckhart/zot/packages/provider" | |
| 8 | 10 | ) | |
| 9 | 11 | ||
| 10 | 12 | func TestReadAgentsContextLoadsGlobalAndAncestors(t *testing.T) { | |
| @@ -109,6 +111,42 @@ func TestResolveExplicitFlagStaleDoesNotRepairConfig(t *testing.T) { | |||
| 109 | 111 | } | |
| 110 | 112 | } | |
| 111 | 113 | ||
| 114 | + | func TestResolveOllamaUsesModelBaseURLBeforeDefault(t *testing.T) { | |
| 115 | + | t.Setenv("ZOT_HOME", t.TempDir()) | |
| 116 | + | provider.SetLiveModels(nil) | |
| 117 | + | defer provider.SetLiveModels(nil) | |
| 118 | + | provider.SetUserModels([]provider.Model{{ | |
| 119 | + | Provider: "ollama", | |
| 120 | + | ID: "qwen-local", | |
| 121 | + | DisplayName: "Qwen Local", | |
| 122 | + | ContextWindow: 32768, | |
| 123 | + | MaxOutput: 8192, | |
| 124 | + | BaseURL: "http://localhost:8000/v1", | |
| 125 | + | }}) | |
| 126 | + | ||
| 127 | + | r, err := Resolve(Args{Provider: "ollama", Model: "qwen-local"}, false) | |
| 128 | + | if err != nil { | |
| 129 | + | t.Fatalf("Resolve failed: %v", err) | |
| 130 | + | } | |
| 131 | + | if r.BaseURL != "http://localhost:8000/v1" { | |
| 132 | + | t.Fatalf("BaseURL = %q, want models.json baseUrl", r.BaseURL) | |
| 133 | + | } | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func TestResolveOllamaFallsBackToDefaultBaseURL(t *testing.T) { | |
| 137 | + | t.Setenv("ZOT_HOME", t.TempDir()) | |
| 138 | + | provider.SetLiveModels(nil) | |
| 139 | + | defer provider.SetLiveModels(nil) | |
| 140 | + | ||
| 141 | + | r, err := Resolve(Args{Provider: "ollama", Model: "any-local-model"}, false) | |
| 142 | + | if err != nil { | |
| 143 | + | t.Fatalf("Resolve failed: %v", err) | |
| 144 | + | } | |
| 145 | + | if r.BaseURL != "http://localhost:11434" { | |
| 146 | + | t.Fatalf("BaseURL = %q, want ollama default", r.BaseURL) | |
| 147 | + | } | |
| 148 | + | } | |
| 149 | + | ||
| 112 | 150 | func TestCanonicalProviderResolvesAliases(t *testing.T) { | |
| 113 | 151 | cases := map[string]string{ | |
| 114 | 152 | "bedrock": "amazon-bedrock", | |
| @@ -5,6 +5,8 @@ import ( | |||||
| 5 | "path/filepath" | 5 | "path/filepath" | ||
| 6 | "strings" | 6 | "strings" | ||
| 7 | "testing" | 7 | "testing" | ||
| 8 | + | ||||
| 9 | + | "github.com/patriceckhart/zot/packages/provider" | |||
| 8 | ) | 10 | ) | ||
| 9 | 11 | ||||
| 10 | func TestReadAgentsContextLoadsGlobalAndAncestors(t *testing.T) { | 12 | func TestReadAgentsContextLoadsGlobalAndAncestors(t *testing.T) { | ||
| @@ -109,6 +111,42 @@ func TestResolveExplicitFlagStaleDoesNotRepairConfig(t *testing.T) { | |||||
| 109 | } | 111 | } | ||
| 110 | } | 112 | } | ||
| 111 | 113 | ||||
| 114 | + | func TestResolveOllamaUsesModelBaseURLBeforeDefault(t *testing.T) { | |||
| 115 | + | t.Setenv("ZOT_HOME", t.TempDir()) | |||
| 116 | + | provider.SetLiveModels(nil) | |||
| 117 | + | defer provider.SetLiveModels(nil) | |||
| 118 | + | provider.SetUserModels([]provider.Model{{ | |||
| 119 | + | Provider: "ollama", | |||
| 120 | + | ID: "qwen-local", | |||
| 121 | + | DisplayName: "Qwen Local", | |||
| 122 | + | ContextWindow: 32768, | |||
| 123 | + | MaxOutput: 8192, | |||
| 124 | + | BaseURL: "http://localhost:8000/v1", | |||
| 125 | + | }}) | |||
| 126 | + | ||||
| 127 | + | r, err := Resolve(Args{Provider: "ollama", Model: "qwen-local"}, false) | |||
| 128 | + | if err != nil { | |||
| 129 | + | t.Fatalf("Resolve failed: %v", err) | |||
| 130 | + | } | |||
| 131 | + | if r.BaseURL != "http://localhost:8000/v1" { | |||
| 132 | + | t.Fatalf("BaseURL = %q, want models.json baseUrl", r.BaseURL) | |||
| 133 | + | } | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func TestResolveOllamaFallsBackToDefaultBaseURL(t *testing.T) { | |||
| 137 | + | t.Setenv("ZOT_HOME", t.TempDir()) | |||
| 138 | + | provider.SetLiveModels(nil) | |||
| 139 | + | defer provider.SetLiveModels(nil) | |||
| 140 | + | ||||
| 141 | + | r, err := Resolve(Args{Provider: "ollama", Model: "any-local-model"}, false) | |||
| 142 | + | if err != nil { | |||
| 143 | + | t.Fatalf("Resolve failed: %v", err) | |||
| 144 | + | } | |||
| 145 | + | if r.BaseURL != "http://localhost:11434" { | |||
| 146 | + | t.Fatalf("BaseURL = %q, want ollama default", r.BaseURL) | |||
| 147 | + | } | |||
| 148 | + | } | |||
| 149 | + | ||||
| 112 | func TestCanonicalProviderResolvesAliases(t *testing.T) { | 150 | func TestCanonicalProviderResolvesAliases(t *testing.T) { | ||
| 113 | cases := map[string]string{ | 151 | cases := map[string]string{ | ||
| 114 | "bedrock": "amazon-bedrock", | 152 | "bedrock": "amazon-bedrock", | ||
M packages/agent/cli.go +6 -0
| @@ -67,6 +67,11 @@ func (h *interactiveExtHooks) Display(extName, text string) { | |||
| 67 | 67 | iv.Display(extName, text) | |
| 68 | 68 | } | |
| 69 | 69 | } | |
| 70 | + | func (h *interactiveExtHooks) ClearNotes(extName string) { | |
| 71 | + | if iv := h.iv(); iv != nil { | |
| 72 | + | iv.ClearNotes(extName) | |
| 73 | + | } | |
| 74 | + | } | |
| 70 | 75 | func (h *interactiveExtHooks) OpenPanel(extName string, spec extproto.PanelSpec) { | |
| 71 | 76 | if iv := h.iv(); iv != nil { | |
| 72 | 77 | iv.OpenPanel(extName, spec) | |
| @@ -256,6 +261,7 @@ func (nonInteractiveExtHooks) Submit(string) | |||
| 256 | 261 | func (nonInteractiveExtHooks) SubmitSlash(string) {} | |
| 257 | 262 | func (nonInteractiveExtHooks) Insert(string) {} | |
| 258 | 263 | func (nonInteractiveExtHooks) Display(string, string) {} | |
| 264 | + | func (nonInteractiveExtHooks) ClearNotes(string) {} | |
| 259 | 265 | func (nonInteractiveExtHooks) OpenPanel(string, extproto.PanelSpec) {} | |
| 260 | 266 | func (nonInteractiveExtHooks) UpdatePanel(string, string, string, []string, string) {} | |
| 261 | 267 | func (nonInteractiveExtHooks) ClosePanel(string, string) {} | |
| @@ -67,6 +67,11 @@ func (h *interactiveExtHooks) Display(extName, text string) { | |||||
| 67 | iv.Display(extName, text) | 67 | iv.Display(extName, text) | ||
| 68 | } | 68 | } | ||
| 69 | } | 69 | } | ||
| 70 | + | func (h *interactiveExtHooks) ClearNotes(extName string) { | |||
| 71 | + | if iv := h.iv(); iv != nil { | |||
| 72 | + | iv.ClearNotes(extName) | |||
| 73 | + | } | |||
| 74 | + | } | |||
| 70 | func (h *interactiveExtHooks) OpenPanel(extName string, spec extproto.PanelSpec) { | 75 | func (h *interactiveExtHooks) OpenPanel(extName string, spec extproto.PanelSpec) { | ||
| 71 | if iv := h.iv(); iv != nil { | 76 | if iv := h.iv(); iv != nil { | ||
| 72 | iv.OpenPanel(extName, spec) | 77 | iv.OpenPanel(extName, spec) | ||
| @@ -256,6 +261,7 @@ func (nonInteractiveExtHooks) Submit(string) | |||||
| 256 | func (nonInteractiveExtHooks) SubmitSlash(string) {} | 261 | func (nonInteractiveExtHooks) SubmitSlash(string) {} | ||
| 257 | func (nonInteractiveExtHooks) Insert(string) {} | 262 | func (nonInteractiveExtHooks) Insert(string) {} | ||
| 258 | func (nonInteractiveExtHooks) Display(string, string) {} | 263 | func (nonInteractiveExtHooks) Display(string, string) {} | ||
| 264 | + | func (nonInteractiveExtHooks) ClearNotes(string) {} | |||
| 259 | func (nonInteractiveExtHooks) OpenPanel(string, extproto.PanelSpec) {} | 265 | func (nonInteractiveExtHooks) OpenPanel(string, extproto.PanelSpec) {} | ||
| 260 | func (nonInteractiveExtHooks) UpdatePanel(string, string, string, []string, string) {} | 266 | func (nonInteractiveExtHooks) UpdatePanel(string, string, string, []string, string) {} | ||
| 261 | func (nonInteractiveExtHooks) ClosePanel(string, string) {} | 267 | func (nonInteractiveExtHooks) ClosePanel(string, string) {} | ||
M packages/agent/extcmd.go +129 -2
| @@ -269,12 +269,24 @@ func extInstall(args []string) error { | |||
| 269 | 269 | if _, err := os.Stat(filepath.Join(src, "extension.json")); err != nil { | |
| 270 | 270 | return fmt.Errorf("source lacks extension.json") | |
| 271 | 271 | } | |
| 272 | - | name := filepath.Base(src) | |
| 272 | + | // Resolve to an absolute, cleaned path before deriving the install | |
| 273 | + | // name. Otherwise relative sources like "." or "./" collapse to a | |
| 274 | + | // basename of ".", and the destination wrongly resolves to the | |
| 275 | + | // extensions/ parent directory (which zot creates on first run), | |
| 276 | + | // triggering a false "already exists" failure. | |
| 277 | + | absSrc, err := filepath.Abs(src) | |
| 278 | + | if err != nil { | |
| 279 | + | return err | |
| 280 | + | } | |
| 281 | + | name := filepath.Base(absSrc) | |
| 282 | + | if name == "." || name == ".." || name == string(filepath.Separator) || name == "" { | |
| 283 | + | return fmt.Errorf("cannot derive extension name from %q", src) | |
| 284 | + | } | |
| 273 | 285 | out := filepath.Join(dest, name) | |
| 274 | 286 | if _, err := os.Stat(out); err == nil { | |
| 275 | 287 | return fmt.Errorf("destination %s already exists; remove it first", out) | |
| 276 | 288 | } | |
| 277 | - | if err := copyDir(src, out); err != nil { | |
| 289 | + | if err := copyDir(absSrc, out); err != nil { | |
| 278 | 290 | return err | |
| 279 | 291 | } | |
| 280 | 292 | fmt.Fprintf(os.Stderr, "installed %s\n", out) | |
| @@ -311,7 +323,14 @@ func dashIfEmpty(s string) string { | |||
| 311 | 323 | ||
| 312 | 324 | // copyDir does a recursive copy of src to dst preserving file mode | |
| 313 | 325 | // bits. Used by `zot ext install <local-path>`. | |
| 326 | + | // | |
| 327 | + | // Entries matched by the source's root .gitignore are skipped, and | |
| 328 | + | // .git itself is always skipped. This keeps non-portable, regeneratable | |
| 329 | + | // directories (e.g. .venv with hardcoded rpaths, node_modules, target/) | |
| 330 | + | // out of the installed copy so the extension stays functional at its new | |
| 331 | + | // location. | |
| 314 | 332 | func copyDir(src, dst string) error { | |
| 333 | + | ig := loadGitignore(src) | |
| 315 | 334 | return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { | |
| 316 | 335 | if err != nil { | |
| 317 | 336 | return err | |
| @@ -320,6 +339,18 @@ func copyDir(src, dst string) error { | |||
| 320 | 339 | if err != nil { | |
| 321 | 340 | return err | |
| 322 | 341 | } | |
| 342 | + | if rel != "." { | |
| 343 | + | name := filepath.Base(rel) | |
| 344 | + | if info.IsDir() && name == ".git" { | |
| 345 | + | return filepath.SkipDir | |
| 346 | + | } | |
| 347 | + | if ig.match(filepath.ToSlash(rel), info.IsDir()) { | |
| 348 | + | if info.IsDir() { | |
| 349 | + | return filepath.SkipDir | |
| 350 | + | } | |
| 351 | + | return nil | |
| 352 | + | } | |
| 353 | + | } | |
| 323 | 354 | target := filepath.Join(dst, rel) | |
| 324 | 355 | if info.IsDir() { | |
| 325 | 356 | return os.MkdirAll(target, info.Mode()) | |
| @@ -338,3 +369,99 @@ func copyDir(src, dst string) error { | |||
| 338 | 369 | return err | |
| 339 | 370 | }) | |
| 340 | 371 | } | |
| 372 | + | ||
| 373 | + | // gitignore is a minimal .gitignore matcher. It supports the common | |
| 374 | + | // patterns used in real extension repos: blank lines, comments (#), | |
| 375 | + | // negation (!), directory-only patterns (trailing /), anchored | |
| 376 | + | // patterns (leading /), and the * / ? / [..] wildcards via | |
| 377 | + | // filepath.Match. It intentionally does not implement ** globstar or | |
| 378 | + | // nested per-directory .gitignore files; the goal is to drop obvious | |
| 379 | + | // non-portable directories, not to be a faithful git reimplementation. | |
| 380 | + | type gitignore struct { | |
| 381 | + | rules []gitignoreRule | |
| 382 | + | } | |
| 383 | + | ||
| 384 | + | type gitignoreRule struct { | |
| 385 | + | pattern string | |
| 386 | + | negate bool | |
| 387 | + | dirOnly bool | |
| 388 | + | anchored bool | |
| 389 | + | } | |
| 390 | + | ||
| 391 | + | func loadGitignore(root string) *gitignore { | |
| 392 | + | data, err := os.ReadFile(filepath.Join(root, ".gitignore")) | |
| 393 | + | if err != nil { | |
| 394 | + | return &gitignore{} | |
| 395 | + | } | |
| 396 | + | return loadGitignoreFromString(string(data)) | |
| 397 | + | } | |
| 398 | + | ||
| 399 | + | func loadGitignoreFromString(data string) *gitignore { | |
| 400 | + | g := &gitignore{} | |
| 401 | + | for _, line := range strings.Split(data, "\n") { | |
| 402 | + | line = strings.TrimRight(line, "\r") | |
| 403 | + | trimmed := strings.TrimSpace(line) | |
| 404 | + | if trimmed == "" || strings.HasPrefix(trimmed, "#") { | |
| 405 | + | continue | |
| 406 | + | } | |
| 407 | + | r := gitignoreRule{pattern: trimmed} | |
| 408 | + | if strings.HasPrefix(r.pattern, "!") { | |
| 409 | + | r.negate = true | |
| 410 | + | r.pattern = r.pattern[1:] | |
| 411 | + | } | |
| 412 | + | if strings.HasSuffix(r.pattern, "/") { | |
| 413 | + | r.dirOnly = true | |
| 414 | + | r.pattern = strings.TrimSuffix(r.pattern, "/") | |
| 415 | + | } | |
| 416 | + | if strings.HasPrefix(r.pattern, "/") { | |
| 417 | + | r.anchored = true | |
| 418 | + | r.pattern = strings.TrimPrefix(r.pattern, "/") | |
| 419 | + | } | |
| 420 | + | if r.pattern == "" { | |
| 421 | + | continue | |
| 422 | + | } | |
| 423 | + | g.rules = append(g.rules, r) | |
| 424 | + | } | |
| 425 | + | return g | |
| 426 | + | } | |
| 427 | + | ||
| 428 | + | // match reports whether the slash-separated relative path should be | |
| 429 | + | // ignored. Later rules win, so a trailing negation can re-include a | |
| 430 | + | // previously ignored path. | |
| 431 | + | func (g *gitignore) match(rel string, isDir bool) bool { | |
| 432 | + | ignored := false | |
| 433 | + | for _, r := range g.rules { | |
| 434 | + | if r.dirOnly && !isDir { | |
| 435 | + | continue | |
| 436 | + | } | |
| 437 | + | if r.matchPath(rel) { | |
| 438 | + | ignored = !r.negate | |
| 439 | + | } | |
| 440 | + | } | |
| 441 | + | return ignored | |
| 442 | + | } | |
| 443 | + | ||
| 444 | + | func (r gitignoreRule) matchPath(rel string) bool { | |
| 445 | + | if r.anchored || strings.Contains(r.pattern, "/") { | |
| 446 | + | if ok, _ := filepath.Match(r.pattern, rel); ok { | |
| 447 | + | return true | |
| 448 | + | } | |
| 449 | + | // Anchored directory pattern also matches everything beneath it. | |
| 450 | + | return strings.HasPrefix(rel, r.pattern+"/") | |
| 451 | + | } | |
| 452 | + | // Unanchored: match the basename of any path component. | |
| 453 | + | base := rel | |
| 454 | + | if i := strings.LastIndex(rel, "/"); i >= 0 { | |
| 455 | + | base = rel[i+1:] | |
| 456 | + | } | |
| 457 | + | if ok, _ := filepath.Match(r.pattern, base); ok { | |
| 458 | + | return true | |
| 459 | + | } | |
| 460 | + | // Match a directory component anywhere in the path. | |
| 461 | + | for _, part := range strings.Split(rel, "/") { | |
| 462 | + | if ok, _ := filepath.Match(r.pattern, part); ok { | |
| 463 | + | return true | |
| 464 | + | } | |
| 465 | + | } | |
| 466 | + | return false | |
| 467 | + | } | |
| @@ -269,12 +269,24 @@ func extInstall(args []string) error { | |||||
| 269 | if _, err := os.Stat(filepath.Join(src, "extension.json")); err != nil { | 269 | if _, err := os.Stat(filepath.Join(src, "extension.json")); err != nil { | ||
| 270 | return fmt.Errorf("source lacks extension.json") | 270 | return fmt.Errorf("source lacks extension.json") | ||
| 271 | } | 271 | } | ||
| 272 | - | name := filepath.Base(src) | 272 | + | // Resolve to an absolute, cleaned path before deriving the install |
| 273 | + | // name. Otherwise relative sources like "." or "./" collapse to a | |||
| 274 | + | // basename of ".", and the destination wrongly resolves to the | |||
| 275 | + | // extensions/ parent directory (which zot creates on first run), | |||
| 276 | + | // triggering a false "already exists" failure. | |||
| 277 | + | absSrc, err := filepath.Abs(src) | |||
| 278 | + | if err != nil { | |||
| 279 | + | return err | |||
| 280 | + | } | |||
| 281 | + | name := filepath.Base(absSrc) | |||
| 282 | + | if name == "." || name == ".." || name == string(filepath.Separator) || name == "" { | |||
| 283 | + | return fmt.Errorf("cannot derive extension name from %q", src) | |||
| 284 | + | } | |||
| 273 | out := filepath.Join(dest, name) | 285 | out := filepath.Join(dest, name) | ||
| 274 | if _, err := os.Stat(out); err == nil { | 286 | if _, err := os.Stat(out); err == nil { | ||
| 275 | return fmt.Errorf("destination %s already exists; remove it first", out) | 287 | return fmt.Errorf("destination %s already exists; remove it first", out) | ||
| 276 | } | 288 | } | ||
| 277 | - | if err := copyDir(src, out); err != nil { | 289 | + | if err := copyDir(absSrc, out); err != nil { |
| 278 | return err | 290 | return err | ||
| 279 | } | 291 | } | ||
| 280 | fmt.Fprintf(os.Stderr, "installed %s\n", out) | 292 | fmt.Fprintf(os.Stderr, "installed %s\n", out) | ||
| @@ -311,7 +323,14 @@ func dashIfEmpty(s string) string { | |||||
| 311 | 323 | ||||
| 312 | // copyDir does a recursive copy of src to dst preserving file mode | 324 | // copyDir does a recursive copy of src to dst preserving file mode | ||
| 313 | // bits. Used by `zot ext install <local-path>`. | 325 | // bits. Used by `zot ext install <local-path>`. | ||
| 326 | + | // | |||
| 327 | + | // Entries matched by the source's root .gitignore are skipped, and | |||
| 328 | + | // .git itself is always skipped. This keeps non-portable, regeneratable | |||
| 329 | + | // directories (e.g. .venv with hardcoded rpaths, node_modules, target/) | |||
| 330 | + | // out of the installed copy so the extension stays functional at its new | |||
| 331 | + | // location. | |||
| 314 | func copyDir(src, dst string) error { | 332 | func copyDir(src, dst string) error { | ||
| 333 | + | ig := loadGitignore(src) | |||
| 315 | return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { | 334 | return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { | ||
| 316 | if err != nil { | 335 | if err != nil { | ||
| 317 | return err | 336 | return err | ||
| @@ -320,6 +339,18 @@ func copyDir(src, dst string) error { | |||||
| 320 | if err != nil { | 339 | if err != nil { | ||
| 321 | return err | 340 | return err | ||
| 322 | } | 341 | } | ||
| 342 | + | if rel != "." { | |||
| 343 | + | name := filepath.Base(rel) | |||
| 344 | + | if info.IsDir() && name == ".git" { | |||
| 345 | + | return filepath.SkipDir | |||
| 346 | + | } | |||
| 347 | + | if ig.match(filepath.ToSlash(rel), info.IsDir()) { | |||
| 348 | + | if info.IsDir() { | |||
| 349 | + | return filepath.SkipDir | |||
| 350 | + | } | |||
| 351 | + | return nil | |||
| 352 | + | } | |||
| 353 | + | } | |||
| 323 | target := filepath.Join(dst, rel) | 354 | target := filepath.Join(dst, rel) | ||
| 324 | if info.IsDir() { | 355 | if info.IsDir() { | ||
| 325 | return os.MkdirAll(target, info.Mode()) | 356 | return os.MkdirAll(target, info.Mode()) | ||
| @@ -338,3 +369,99 @@ func copyDir(src, dst string) error { | |||||
| 338 | return err | 369 | return err | ||
| 339 | }) | 370 | }) | ||
| 340 | } | 371 | } | ||
| 372 | + | ||||
| 373 | + | // gitignore is a minimal .gitignore matcher. It supports the common | |||
| 374 | + | // patterns used in real extension repos: blank lines, comments (#), | |||
| 375 | + | // negation (!), directory-only patterns (trailing /), anchored | |||
| 376 | + | // patterns (leading /), and the * / ? / [..] wildcards via | |||
| 377 | + | // filepath.Match. It intentionally does not implement ** globstar or | |||
| 378 | + | // nested per-directory .gitignore files; the goal is to drop obvious | |||
| 379 | + | // non-portable directories, not to be a faithful git reimplementation. | |||
| 380 | + | type gitignore struct { | |||
| 381 | + | rules []gitignoreRule | |||
| 382 | + | } | |||
| 383 | + | ||||
| 384 | + | type gitignoreRule struct { | |||
| 385 | + | pattern string | |||
| 386 | + | negate bool | |||
| 387 | + | dirOnly bool | |||
| 388 | + | anchored bool | |||
| 389 | + | } | |||
| 390 | + | ||||
| 391 | + | func loadGitignore(root string) *gitignore { | |||
| 392 | + | data, err := os.ReadFile(filepath.Join(root, ".gitignore")) | |||
| 393 | + | if err != nil { | |||
| 394 | + | return &gitignore{} | |||
| 395 | + | } | |||
| 396 | + | return loadGitignoreFromString(string(data)) | |||
| 397 | + | } | |||
| 398 | + | ||||
| 399 | + | func loadGitignoreFromString(data string) *gitignore { | |||
| 400 | + | g := &gitignore{} | |||
| 401 | + | for _, line := range strings.Split(data, "\n") { | |||
| 402 | + | line = strings.TrimRight(line, "\r") | |||
| 403 | + | trimmed := strings.TrimSpace(line) | |||
| 404 | + | if trimmed == "" || strings.HasPrefix(trimmed, "#") { | |||
| 405 | + | continue | |||
| 406 | + | } | |||
| 407 | + | r := gitignoreRule{pattern: trimmed} | |||
| 408 | + | if strings.HasPrefix(r.pattern, "!") { | |||
| 409 | + | r.negate = true | |||
| 410 | + | r.pattern = r.pattern[1:] | |||
| 411 | + | } | |||
| 412 | + | if strings.HasSuffix(r.pattern, "/") { | |||
| 413 | + | r.dirOnly = true | |||
| 414 | + | r.pattern = strings.TrimSuffix(r.pattern, "/") | |||
| 415 | + | } | |||
| 416 | + | if strings.HasPrefix(r.pattern, "/") { | |||
| 417 | + | r.anchored = true | |||
| 418 | + | r.pattern = strings.TrimPrefix(r.pattern, "/") | |||
| 419 | + | } | |||
| 420 | + | if r.pattern == "" { | |||
| 421 | + | continue | |||
| 422 | + | } | |||
| 423 | + | g.rules = append(g.rules, r) | |||
| 424 | + | } | |||
| 425 | + | return g | |||
| 426 | + | } | |||
| 427 | + | ||||
| 428 | + | // match reports whether the slash-separated relative path should be | |||
| 429 | + | // ignored. Later rules win, so a trailing negation can re-include a | |||
| 430 | + | // previously ignored path. | |||
| 431 | + | func (g *gitignore) match(rel string, isDir bool) bool { | |||
| 432 | + | ignored := false | |||
| 433 | + | for _, r := range g.rules { | |||
| 434 | + | if r.dirOnly && !isDir { | |||
| 435 | + | continue | |||
| 436 | + | } | |||
| 437 | + | if r.matchPath(rel) { | |||
| 438 | + | ignored = !r.negate | |||
| 439 | + | } | |||
| 440 | + | } | |||
| 441 | + | return ignored | |||
| 442 | + | } | |||
| 443 | + | ||||
| 444 | + | func (r gitignoreRule) matchPath(rel string) bool { | |||
| 445 | + | if r.anchored || strings.Contains(r.pattern, "/") { | |||
| 446 | + | if ok, _ := filepath.Match(r.pattern, rel); ok { | |||
| 447 | + | return true | |||
| 448 | + | } | |||
| 449 | + | // Anchored directory pattern also matches everything beneath it. | |||
| 450 | + | return strings.HasPrefix(rel, r.pattern+"/") | |||
| 451 | + | } | |||
| 452 | + | // Unanchored: match the basename of any path component. | |||
| 453 | + | base := rel | |||
| 454 | + | if i := strings.LastIndex(rel, "/"); i >= 0 { | |||
| 455 | + | base = rel[i+1:] | |||
| 456 | + | } | |||
| 457 | + | if ok, _ := filepath.Match(r.pattern, base); ok { | |||
| 458 | + | return true | |||
| 459 | + | } | |||
| 460 | + | // Match a directory component anywhere in the path. | |||
| 461 | + | for _, part := range strings.Split(rel, "/") { | |||
| 462 | + | if ok, _ := filepath.Match(r.pattern, part); ok { | |||
| 463 | + | return true | |||
| 464 | + | } | |||
| 465 | + | } | |||
| 466 | + | return false | |||
| 467 | + | } | |||
A packages/agent/extcmd_test.go +126 -0
| @@ -0,0 +1,126 @@ | |||
| 1 | + | package agent | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "path/filepath" | |
| 6 | + | "testing" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | // TestExtInstallDotSource verifies that `zot ext install .` derives the | |
| 10 | + | // extension name from the resolved directory name rather than collapsing | |
| 11 | + | // to the extensions/ parent directory (the false "already exists" bug). | |
| 12 | + | func TestExtInstallDotSource(t *testing.T) { | |
| 13 | + | home := t.TempDir() | |
| 14 | + | t.Setenv("ZOT_HOME", home) | |
| 15 | + | ||
| 16 | + | // Pre-create extensions/ to mimic a normal first run. | |
| 17 | + | if err := os.MkdirAll(filepath.Join(home, "extensions"), 0o755); err != nil { | |
| 18 | + | t.Fatal(err) | |
| 19 | + | } | |
| 20 | + | ||
| 21 | + | srcParent := t.TempDir() | |
| 22 | + | src := filepath.Join(srcParent, "kagi") | |
| 23 | + | if err := os.MkdirAll(src, 0o755); err != nil { | |
| 24 | + | t.Fatal(err) | |
| 25 | + | } | |
| 26 | + | if err := os.WriteFile(filepath.Join(src, "extension.json"), []byte(`{"name":"kagi"}`), 0o644); err != nil { | |
| 27 | + | t.Fatal(err) | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | cwd, err := os.Getwd() | |
| 31 | + | if err != nil { | |
| 32 | + | t.Fatal(err) | |
| 33 | + | } | |
| 34 | + | defer os.Chdir(cwd) | |
| 35 | + | if err := os.Chdir(src); err != nil { | |
| 36 | + | t.Fatal(err) | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | if err := extInstall([]string{"."}); err != nil { | |
| 40 | + | t.Fatalf("install with '.' failed: %v", err) | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | out := filepath.Join(home, "extensions", "kagi") | |
| 44 | + | if _, err := os.Stat(filepath.Join(out, "extension.json")); err != nil { | |
| 45 | + | t.Fatalf("expected installed extension at %s: %v", out, err) | |
| 46 | + | } | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | // TestExtInstallRejectsParentName guards against deriving a name of ".." | |
| 50 | + | // from a source that resolves to a filesystem root edge case. A normal | |
| 51 | + | // directory always yields a real basename, so this just ensures the | |
| 52 | + | // guard logic does not crash for well-formed input. | |
| 53 | + | func TestExtInstallNamedDir(t *testing.T) { | |
| 54 | + | home := t.TempDir() | |
| 55 | + | t.Setenv("ZOT_HOME", home) | |
| 56 | + | ||
| 57 | + | src := filepath.Join(t.TempDir(), "myext") | |
| 58 | + | if err := os.MkdirAll(src, 0o755); err != nil { | |
| 59 | + | t.Fatal(err) | |
| 60 | + | } | |
| 61 | + | if err := os.WriteFile(filepath.Join(src, "extension.json"), []byte(`{"name":"myext"}`), 0o644); err != nil { | |
| 62 | + | t.Fatal(err) | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | if err := extInstall([]string{src}); err != nil { | |
| 66 | + | t.Fatalf("install failed: %v", err) | |
| 67 | + | } | |
| 68 | + | if _, err := os.Stat(filepath.Join(home, "extensions", "myext", "extension.json")); err != nil { | |
| 69 | + | t.Fatalf("expected installed extension: %v", err) | |
| 70 | + | } | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | // TestCopyDirRespectsGitignore verifies that non-portable directories | |
| 74 | + | // listed in the source .gitignore (e.g. .venv, node_modules) are not | |
| 75 | + | // copied during install, while tracked files are. | |
| 76 | + | func TestCopyDirRespectsGitignore(t *testing.T) { | |
| 77 | + | src := t.TempDir() | |
| 78 | + | dst := filepath.Join(t.TempDir(), "out") | |
| 79 | + | ||
| 80 | + | mustWrite := func(rel, content string) { | |
| 81 | + | p := filepath.Join(src, filepath.FromSlash(rel)) | |
| 82 | + | if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { | |
| 83 | + | t.Fatal(err) | |
| 84 | + | } | |
| 85 | + | if err := os.WriteFile(p, []byte(content), 0o644); err != nil { | |
| 86 | + | t.Fatal(err) | |
| 87 | + | } | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | mustWrite("extension.json", `{"name":"x"}`) | |
| 91 | + | mustWrite("main.py", "print('hi')") | |
| 92 | + | mustWrite(".gitignore", ".venv/\nnode_modules/\n*.log\n") | |
| 93 | + | mustWrite(".venv/bin/python", "binary") | |
| 94 | + | mustWrite("node_modules/pkg/index.js", "module") | |
| 95 | + | mustWrite("debug.log", "noise") | |
| 96 | + | mustWrite("src/app.py", "code") | |
| 97 | + | mustWrite(".git/config", "gitdir") | |
| 98 | + | ||
| 99 | + | if err := copyDir(src, dst); err != nil { | |
| 100 | + | t.Fatal(err) | |
| 101 | + | } | |
| 102 | + | ||
| 103 | + | wantPresent := []string{"extension.json", "main.py", "src/app.py", ".gitignore"} | |
| 104 | + | for _, rel := range wantPresent { | |
| 105 | + | if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); err != nil { | |
| 106 | + | t.Fatalf("expected %s to be copied: %v", rel, err) | |
| 107 | + | } | |
| 108 | + | } | |
| 109 | + | ||
| 110 | + | wantAbsent := []string{".venv", "node_modules", "debug.log", ".git"} | |
| 111 | + | for _, rel := range wantAbsent { | |
| 112 | + | if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); err == nil { | |
| 113 | + | t.Fatalf("expected %s to be skipped, but it was copied", rel) | |
| 114 | + | } | |
| 115 | + | } | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | func TestGitignoreNegation(t *testing.T) { | |
| 119 | + | g := loadGitignoreFromString("build/\n!build/keep.txt\n") | |
| 120 | + | if !g.match("build", true) { | |
| 121 | + | t.Fatal("expected build/ dir to be ignored") | |
| 122 | + | } | |
| 123 | + | if g.match("build/keep.txt", false) { | |
| 124 | + | t.Fatal("expected build/keep.txt to be re-included by negation") | |
| 125 | + | } | |
| 126 | + | } | |
| @@ -0,0 +1,126 @@ | |||||
| 1 | + | package agent | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "os" | |||
| 5 | + | "path/filepath" | |||
| 6 | + | "testing" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | // TestExtInstallDotSource verifies that `zot ext install .` derives the | |||
| 10 | + | // extension name from the resolved directory name rather than collapsing | |||
| 11 | + | // to the extensions/ parent directory (the false "already exists" bug). | |||
| 12 | + | func TestExtInstallDotSource(t *testing.T) { | |||
| 13 | + | home := t.TempDir() | |||
| 14 | + | t.Setenv("ZOT_HOME", home) | |||
| 15 | + | ||||
| 16 | + | // Pre-create extensions/ to mimic a normal first run. | |||
| 17 | + | if err := os.MkdirAll(filepath.Join(home, "extensions"), 0o755); err != nil { | |||
| 18 | + | t.Fatal(err) | |||
| 19 | + | } | |||
| 20 | + | ||||
| 21 | + | srcParent := t.TempDir() | |||
| 22 | + | src := filepath.Join(srcParent, "kagi") | |||
| 23 | + | if err := os.MkdirAll(src, 0o755); err != nil { | |||
| 24 | + | t.Fatal(err) | |||
| 25 | + | } | |||
| 26 | + | if err := os.WriteFile(filepath.Join(src, "extension.json"), []byte(`{"name":"kagi"}`), 0o644); err != nil { | |||
| 27 | + | t.Fatal(err) | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | cwd, err := os.Getwd() | |||
| 31 | + | if err != nil { | |||
| 32 | + | t.Fatal(err) | |||
| 33 | + | } | |||
| 34 | + | defer os.Chdir(cwd) | |||
| 35 | + | if err := os.Chdir(src); err != nil { | |||
| 36 | + | t.Fatal(err) | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | if err := extInstall([]string{"."}); err != nil { | |||
| 40 | + | t.Fatalf("install with '.' failed: %v", err) | |||
| 41 | + | } | |||
| 42 | + | ||||
| 43 | + | out := filepath.Join(home, "extensions", "kagi") | |||
| 44 | + | if _, err := os.Stat(filepath.Join(out, "extension.json")); err != nil { | |||
| 45 | + | t.Fatalf("expected installed extension at %s: %v", out, err) | |||
| 46 | + | } | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | // TestExtInstallRejectsParentName guards against deriving a name of ".." | |||
| 50 | + | // from a source that resolves to a filesystem root edge case. A normal | |||
| 51 | + | // directory always yields a real basename, so this just ensures the | |||
| 52 | + | // guard logic does not crash for well-formed input. | |||
| 53 | + | func TestExtInstallNamedDir(t *testing.T) { | |||
| 54 | + | home := t.TempDir() | |||
| 55 | + | t.Setenv("ZOT_HOME", home) | |||
| 56 | + | ||||
| 57 | + | src := filepath.Join(t.TempDir(), "myext") | |||
| 58 | + | if err := os.MkdirAll(src, 0o755); err != nil { | |||
| 59 | + | t.Fatal(err) | |||
| 60 | + | } | |||
| 61 | + | if err := os.WriteFile(filepath.Join(src, "extension.json"), []byte(`{"name":"myext"}`), 0o644); err != nil { | |||
| 62 | + | t.Fatal(err) | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | if err := extInstall([]string{src}); err != nil { | |||
| 66 | + | t.Fatalf("install failed: %v", err) | |||
| 67 | + | } | |||
| 68 | + | if _, err := os.Stat(filepath.Join(home, "extensions", "myext", "extension.json")); err != nil { | |||
| 69 | + | t.Fatalf("expected installed extension: %v", err) | |||
| 70 | + | } | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | // TestCopyDirRespectsGitignore verifies that non-portable directories | |||
| 74 | + | // listed in the source .gitignore (e.g. .venv, node_modules) are not | |||
| 75 | + | // copied during install, while tracked files are. | |||
| 76 | + | func TestCopyDirRespectsGitignore(t *testing.T) { | |||
| 77 | + | src := t.TempDir() | |||
| 78 | + | dst := filepath.Join(t.TempDir(), "out") | |||
| 79 | + | ||||
| 80 | + | mustWrite := func(rel, content string) { | |||
| 81 | + | p := filepath.Join(src, filepath.FromSlash(rel)) | |||
| 82 | + | if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { | |||
| 83 | + | t.Fatal(err) | |||
| 84 | + | } | |||
| 85 | + | if err := os.WriteFile(p, []byte(content), 0o644); err != nil { | |||
| 86 | + | t.Fatal(err) | |||
| 87 | + | } | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | mustWrite("extension.json", `{"name":"x"}`) | |||
| 91 | + | mustWrite("main.py", "print('hi')") | |||
| 92 | + | mustWrite(".gitignore", ".venv/\nnode_modules/\n*.log\n") | |||
| 93 | + | mustWrite(".venv/bin/python", "binary") | |||
| 94 | + | mustWrite("node_modules/pkg/index.js", "module") | |||
| 95 | + | mustWrite("debug.log", "noise") | |||
| 96 | + | mustWrite("src/app.py", "code") | |||
| 97 | + | mustWrite(".git/config", "gitdir") | |||
| 98 | + | ||||
| 99 | + | if err := copyDir(src, dst); err != nil { | |||
| 100 | + | t.Fatal(err) | |||
| 101 | + | } | |||
| 102 | + | ||||
| 103 | + | wantPresent := []string{"extension.json", "main.py", "src/app.py", ".gitignore"} | |||
| 104 | + | for _, rel := range wantPresent { | |||
| 105 | + | if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); err != nil { | |||
| 106 | + | t.Fatalf("expected %s to be copied: %v", rel, err) | |||
| 107 | + | } | |||
| 108 | + | } | |||
| 109 | + | ||||
| 110 | + | wantAbsent := []string{".venv", "node_modules", "debug.log", ".git"} | |||
| 111 | + | for _, rel := range wantAbsent { | |||
| 112 | + | if _, err := os.Stat(filepath.Join(dst, filepath.FromSlash(rel))); err == nil { | |||
| 113 | + | t.Fatalf("expected %s to be skipped, but it was copied", rel) | |||
| 114 | + | } | |||
| 115 | + | } | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | func TestGitignoreNegation(t *testing.T) { | |||
| 119 | + | g := loadGitignoreFromString("build/\n!build/keep.txt\n") | |||
| 120 | + | if !g.match("build", true) { | |||
| 121 | + | t.Fatal("expected build/ dir to be ignored") | |||
| 122 | + | } | |||
| 123 | + | if g.match("build/keep.txt", false) { | |||
| 124 | + | t.Fatal("expected build/keep.txt to be re-included by negation") | |||
| 125 | + | } | |||
| 126 | + | } | |||
M packages/agent/extensions/manager.go +6 -0
| @@ -119,6 +119,10 @@ type HostHooks interface { | |||
| 119 | 119 | // invoking the model and without writing to the transcript. | |
| 120 | 120 | Display(extName, text string) | |
| 121 | 121 | ||
| 122 | + | // ClearNotes removes any notes previously pushed by extName via | |
| 123 | + | // Notify/Display so transient status lines do not stack forever. | |
| 124 | + | ClearNotes(extName string) | |
| 125 | + | ||
| 122 | 126 | OpenPanel(extName string, spec extproto.PanelSpec) | |
| 123 | 127 | UpdatePanel(extName, panelID, title string, lines []string, footer string) | |
| 124 | 128 | ClosePanel(extName, panelID string) | |
| @@ -750,6 +754,8 @@ func (m *Manager) readLoop(ext *Extension, scanner *bufio.Scanner) { | |||
| 750 | 754 | if err := json.Unmarshal(line, &n); err == nil { | |
| 751 | 755 | m.hooks.Notify(ext.Manifest.Name, n.Level, n.Message) | |
| 752 | 756 | } | |
| 757 | + | case "clear_notes": | |
| 758 | + | m.hooks.ClearNotes(ext.Manifest.Name) | |
| 753 | 759 | case "submit_slash": | |
| 754 | 760 | // Spontaneous request to invoke a slash command in the | |
| 755 | 761 | // TUI. Refused unless the payload looks like a slash | |
| @@ -119,6 +119,10 @@ type HostHooks interface { | |||||
| 119 | // invoking the model and without writing to the transcript. | 119 | // invoking the model and without writing to the transcript. | ||
| 120 | Display(extName, text string) | 120 | Display(extName, text string) | ||
| 121 | 121 | ||||
| 122 | + | // ClearNotes removes any notes previously pushed by extName via | |||
| 123 | + | // Notify/Display so transient status lines do not stack forever. | |||
| 124 | + | ClearNotes(extName string) | |||
| 125 | + | ||||
| 122 | OpenPanel(extName string, spec extproto.PanelSpec) | 126 | OpenPanel(extName string, spec extproto.PanelSpec) | ||
| 123 | UpdatePanel(extName, panelID, title string, lines []string, footer string) | 127 | UpdatePanel(extName, panelID, title string, lines []string, footer string) | ||
| 124 | ClosePanel(extName, panelID string) | 128 | ClosePanel(extName, panelID string) | ||
| @@ -750,6 +754,8 @@ func (m *Manager) readLoop(ext *Extension, scanner *bufio.Scanner) { | |||||
| 750 | if err := json.Unmarshal(line, &n); err == nil { | 754 | if err := json.Unmarshal(line, &n); err == nil { | ||
| 751 | m.hooks.Notify(ext.Manifest.Name, n.Level, n.Message) | 755 | m.hooks.Notify(ext.Manifest.Name, n.Level, n.Message) | ||
| 752 | } | 756 | } | ||
| 757 | + | case "clear_notes": | |||
| 758 | + | m.hooks.ClearNotes(ext.Manifest.Name) | |||
| 753 | case "submit_slash": | 759 | case "submit_slash": | ||
| 754 | // Spontaneous request to invoke a slash command in the | 760 | // Spontaneous request to invoke a slash command in the | ||
| 755 | // TUI. Refused unless the payload looks like a slash | 761 | // TUI. Refused unless the payload looks like a slash | ||
M packages/agent/extensions/manager_test.go +9 -3
| @@ -16,9 +16,10 @@ import ( | |||
| 16 | 16 | ||
| 17 | 17 | // stubHooks records every callback so the test can assert on them. | |
| 18 | 18 | type stubHooks struct { | |
| 19 | - | mu sync.Mutex | |
| 20 | - | notifies []string | |
| 21 | - | displays []string | |
| 19 | + | mu sync.Mutex | |
| 20 | + | notifies []string | |
| 21 | + | displays []string | |
| 22 | + | clearNotes []string | |
| 22 | 23 | } | |
| 23 | 24 | ||
| 24 | 25 | func (s *stubHooks) Notify(name, level, message string) { | |
| @@ -34,6 +35,11 @@ func (s *stubHooks) Display(name, text string) { | |||
| 34 | 35 | defer s.mu.Unlock() | |
| 35 | 36 | s.displays = append(s.displays, name+":"+text) | |
| 36 | 37 | } | |
| 38 | + | func (s *stubHooks) ClearNotes(name string) { | |
| 39 | + | s.mu.Lock() | |
| 40 | + | defer s.mu.Unlock() | |
| 41 | + | s.clearNotes = append(s.clearNotes, name) | |
| 42 | + | } | |
| 37 | 43 | func (s *stubHooks) OpenPanel(string, extproto.PanelSpec) {} | |
| 38 | 44 | func (s *stubHooks) UpdatePanel(string, string, string, []string, string) {} | |
| 39 | 45 | func (s *stubHooks) ClosePanel(string, string) {} | |
| @@ -16,9 +16,10 @@ import ( | |||||
| 16 | 16 | ||||
| 17 | // stubHooks records every callback so the test can assert on them. | 17 | // stubHooks records every callback so the test can assert on them. | ||
| 18 | type stubHooks struct { | 18 | type stubHooks struct { | ||
| 19 | - | mu sync.Mutex | 19 | + | mu sync.Mutex |
| 20 | - | notifies []string | 20 | + | notifies []string |
| 21 | - | displays []string | 21 | + | displays []string |
| 22 | + | clearNotes []string | |||
| 22 | } | 23 | } | ||
| 23 | 24 | ||||
| 24 | func (s *stubHooks) Notify(name, level, message string) { | 25 | func (s *stubHooks) Notify(name, level, message string) { | ||
| @@ -34,6 +35,11 @@ func (s *stubHooks) Display(name, text string) { | |||||
| 34 | defer s.mu.Unlock() | 35 | defer s.mu.Unlock() | ||
| 35 | s.displays = append(s.displays, name+":"+text) | 36 | s.displays = append(s.displays, name+":"+text) | ||
| 36 | } | 37 | } | ||
| 38 | + | func (s *stubHooks) ClearNotes(name string) { | |||
| 39 | + | s.mu.Lock() | |||
| 40 | + | defer s.mu.Unlock() | |||
| 41 | + | s.clearNotes = append(s.clearNotes, name) | |||
| 42 | + | } | |||
| 37 | func (s *stubHooks) OpenPanel(string, extproto.PanelSpec) {} | 43 | func (s *stubHooks) OpenPanel(string, extproto.PanelSpec) {} | ||
| 38 | func (s *stubHooks) UpdatePanel(string, string, string, []string, string) {} | 44 | func (s *stubHooks) UpdatePanel(string, string, string, []string, string) {} | ||
| 39 | func (s *stubHooks) ClosePanel(string, string) {} | 45 | func (s *stubHooks) ClosePanel(string, string) {} | ||
M packages/agent/extproto/extproto.go +8 -0
| @@ -116,6 +116,14 @@ type NotifyFromExt struct { | |||
| 116 | 116 | Message string `json:"message"` | |
| 117 | 117 | } | |
| 118 | 118 | ||
| 119 | + | // ClearNotesFromExt is a spontaneous frame an extension can send to | |
| 120 | + | // retract every note it previously pushed via notify/display, so | |
| 121 | + | // transient status lines (e.g. an approval prompt) do not stack up | |
| 122 | + | // forever in the bottom-sticky notes block. | |
| 123 | + | type ClearNotesFromExt struct { | |
| 124 | + | Type string `json:"type"` | |
| 125 | + | } | |
| 126 | + | ||
| 119 | 127 | // SubmitSlashFromExt is a spontaneous frame an extension can send at | |
| 120 | 128 | // any time (typically from a panel_key handler) to invoke a slash | |
| 121 | 129 | // command in the host's TUI as if the user had typed it. Text must | |
| @@ -116,6 +116,14 @@ type NotifyFromExt struct { | |||||
| 116 | Message string `json:"message"` | 116 | Message string `json:"message"` | ||
| 117 | } | 117 | } | ||
| 118 | 118 | ||||
| 119 | + | // ClearNotesFromExt is a spontaneous frame an extension can send to | |||
| 120 | + | // retract every note it previously pushed via notify/display, so | |||
| 121 | + | // transient status lines (e.g. an approval prompt) do not stack up | |||
| 122 | + | // forever in the bottom-sticky notes block. | |||
| 123 | + | type ClearNotesFromExt struct { | |||
| 124 | + | Type string `json:"type"` | |||
| 125 | + | } | |||
| 126 | + | ||||
| 119 | // SubmitSlashFromExt is a spontaneous frame an extension can send at | 127 | // SubmitSlashFromExt is a spontaneous frame an extension can send at | ||
| 120 | // any time (typically from a panel_key handler) to invoke a slash | 128 | // any time (typically from a panel_key handler) to invoke a slash | ||
| 121 | // command in the host's TUI as if the user had typed it. Text must | 129 | // command in the host's TUI as if the user had typed it. Text must | ||
A packages/agent/modes/ext_notes_test.go +46 -0
| @@ -0,0 +1,46 @@ | |||
| 1 | + | package modes | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "strings" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/patriceckhart/zot/packages/tui" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func newNotesTestInteractive() *Interactive { | |
| 11 | + | i := &Interactive{dirty: make(chan struct{}, 1)} | |
| 12 | + | i.cfg.Theme = tui.Theme{Muted: 8, Warning: 3, Error: 1, Tool: 2, Accent: 4} | |
| 13 | + | return i | |
| 14 | + | } | |
| 15 | + | ||
| 16 | + | func TestClearNotesRemovesOnlyOwnerNotes(t *testing.T) { | |
| 17 | + | i := newNotesTestInteractive() | |
| 18 | + | ||
| 19 | + | i.Notify("kagi", "info", "pending") | |
| 20 | + | i.Notify("kagi", "success", "approved") | |
| 21 | + | i.Notify("other", "info", "keep me") | |
| 22 | + | ||
| 23 | + | if len(i.extNotes) != 3 { | |
| 24 | + | t.Fatalf("expected 3 notes, got %d", len(i.extNotes)) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | i.ClearNotes("kagi") | |
| 28 | + | ||
| 29 | + | if len(i.extNotes) != 1 { | |
| 30 | + | t.Fatalf("expected 1 note after clear, got %d: %v", len(i.extNotes), i.extNotes) | |
| 31 | + | } | |
| 32 | + | if !strings.Contains(i.extNotes[0], "[other] ") { | |
| 33 | + | t.Fatalf("expected the surviving note to belong to other, got %q", i.extNotes[0]) | |
| 34 | + | } | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | func TestClearNotesNoMatchKeepsNotes(t *testing.T) { | |
| 38 | + | i := newNotesTestInteractive() | |
| 39 | + | i.Notify("kagi", "info", "pending") | |
| 40 | + | ||
| 41 | + | i.ClearNotes("nope") | |
| 42 | + | ||
| 43 | + | if len(i.extNotes) != 1 { | |
| 44 | + | t.Fatalf("expected note to survive, got %d", len(i.extNotes)) | |
| 45 | + | } | |
| 46 | + | } | |
| @@ -0,0 +1,46 @@ | |||||
| 1 | + | package modes | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "strings" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/patriceckhart/zot/packages/tui" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func newNotesTestInteractive() *Interactive { | |||
| 11 | + | i := &Interactive{dirty: make(chan struct{}, 1)} | |||
| 12 | + | i.cfg.Theme = tui.Theme{Muted: 8, Warning: 3, Error: 1, Tool: 2, Accent: 4} | |||
| 13 | + | return i | |||
| 14 | + | } | |||
| 15 | + | ||||
| 16 | + | func TestClearNotesRemovesOnlyOwnerNotes(t *testing.T) { | |||
| 17 | + | i := newNotesTestInteractive() | |||
| 18 | + | ||||
| 19 | + | i.Notify("kagi", "info", "pending") | |||
| 20 | + | i.Notify("kagi", "success", "approved") | |||
| 21 | + | i.Notify("other", "info", "keep me") | |||
| 22 | + | ||||
| 23 | + | if len(i.extNotes) != 3 { | |||
| 24 | + | t.Fatalf("expected 3 notes, got %d", len(i.extNotes)) | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | i.ClearNotes("kagi") | |||
| 28 | + | ||||
| 29 | + | if len(i.extNotes) != 1 { | |||
| 30 | + | t.Fatalf("expected 1 note after clear, got %d: %v", len(i.extNotes), i.extNotes) | |||
| 31 | + | } | |||
| 32 | + | if !strings.Contains(i.extNotes[0], "[other] ") { | |||
| 33 | + | t.Fatalf("expected the surviving note to belong to other, got %q", i.extNotes[0]) | |||
| 34 | + | } | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | func TestClearNotesNoMatchKeepsNotes(t *testing.T) { | |||
| 38 | + | i := newNotesTestInteractive() | |||
| 39 | + | i.Notify("kagi", "info", "pending") | |||
| 40 | + | ||||
| 41 | + | i.ClearNotes("nope") | |||
| 42 | + | ||||
| 43 | + | if len(i.extNotes) != 1 { | |||
| 44 | + | t.Fatalf("expected note to survive, got %d", len(i.extNotes)) | |||
| 45 | + | } | |||
| 46 | + | } | |||
M packages/agent/modes/interactive.go +120 -11
| @@ -260,17 +260,28 @@ type Interactive struct { | |||
| 260 | 260 | streamFlushPending bool | |
| 261 | 261 | toolCalls map[string]*tui.ToolCallView | |
| 262 | 262 | toolOrder []string | |
| 263 | - | statusErr string | |
| 264 | - | statusOK string | |
| 265 | - | liveBlock []string // live streaming/tool progress rendered outside scrollback | |
| 266 | - | helpBlock []string // rendered above the chat when /help was typed | |
| 267 | - | cumUsage provider.Usage | |
| 268 | - | lastCtxInput int // input_tokens of the most recent turn — approximates current context size | |
| 269 | - | busy bool | |
| 270 | - | dirty chan struct{} | |
| 271 | - | cancelTurn context.CancelFunc | |
| 272 | - | scrollOffset int // rows from the bottom; 0 = pinned to latest | |
| 273 | - | prevScrollOffset int // last value redraw snapped against; tracks intent | |
| 263 | + | // toolGate records, per tool-call id, how many runes of paced | |
| 264 | + | // assistant text must have drained into `streaming` before that | |
| 265 | + | // tool block may appear. It exists to make stream ordering | |
| 266 | + | // deterministic: a tool call can arrive from the provider while | |
| 267 | + | // the prose that logically precedes it is still being typed out | |
| 268 | + | // by the pacer. Without gating, the tool block would render | |
| 269 | + | // immediately while the intro paragraph keeps filling in below | |
| 270 | + | // it. We snapshot the total expected stream length (already | |
| 271 | + | // streamed + still pending) at the moment the tool starts, and | |
| 272 | + | // hold the block back until the pacer reaches it. | |
| 273 | + | toolGate map[string]int | |
| 274 | + | statusErr string | |
| 275 | + | statusOK string | |
| 276 | + | liveBlock []string // live streaming/tool progress rendered outside scrollback | |
| 277 | + | helpBlock []string // rendered above the chat when /help was typed | |
| 278 | + | cumUsage provider.Usage | |
| 279 | + | lastCtxInput int // input_tokens of the most recent turn — approximates current context size | |
| 280 | + | busy bool | |
| 281 | + | dirty chan struct{} | |
| 282 | + | cancelTurn context.CancelFunc | |
| 283 | + | scrollOffset int // rows from the bottom; 0 = pinned to latest | |
| 284 | + | prevScrollOffset int // last value redraw snapped against; tracks intent | |
| 274 | 285 | ||
| 275 | 286 | // prevChatLen and prevChatCols track the chat buffer's size at the | |
| 276 | 287 | // last redraw so that when content grows below the user's viewport | |
| @@ -440,6 +451,7 @@ func NewInteractive(cfg InteractiveConfig) *Interactive { | |||
| 440 | 451 | ed: tui.NewEditor(cfg.Theme.AccentBar(cfg.Theme.Accent)), | |
| 441 | 452 | rend: renderer, | |
| 442 | 453 | toolCalls: map[string]*tui.ToolCallView{}, | |
| 454 | + | toolGate: map[string]int{}, | |
| 443 | 455 | dirty: make(chan struct{}, 8), | |
| 444 | 456 | dialog: newLoginDialog(), | |
| 445 | 457 | modelDialog: newModelDialog(), | |
| @@ -824,6 +836,14 @@ func (i *Interactive) buildChatLocked(cols int) []string { | |||
| 824 | 836 | i.view.ToolCalls = i.view.ToolCalls[:0] | |
| 825 | 837 | if i.busy { | |
| 826 | 838 | for _, id := range i.toolOrder { | |
| 839 | + | // Deterministic ordering: a tool block stays hidden until | |
| 840 | + | // the paced assistant text that preceded it has finished | |
| 841 | + | // typing out. toolOrder is append-only in arrival order, | |
| 842 | + | // so once one tool is still gated, every later tool is too, | |
| 843 | + | // so stop here to avoid showing a tool out of sequence. | |
| 844 | + | if !i.toolGateOpenLocked(id) { | |
| 845 | + | break | |
| 846 | + | } | |
| 827 | 847 | if tc, ok := i.toolCalls[id]; ok { | |
| 828 | 848 | i.view.ToolCalls = append(i.view.ToolCalls, *tc) | |
| 829 | 849 | } | |
| @@ -2356,6 +2376,36 @@ func (i *Interactive) Notify(extName, level, message string) { | |||
| 2356 | 2376 | i.invalidate() | |
| 2357 | 2377 | } | |
| 2358 | 2378 | ||
| 2379 | + | // ClearNotes removes every note line owned by extName from the | |
| 2380 | + | // bottom-sticky ext-notes block. Extensions use this to retract a | |
| 2381 | + | // transient status line (e.g. an approval prompt) once it no longer | |
| 2382 | + | // applies, instead of leaving it stacked forever. Notes from other | |
| 2383 | + | // extensions and internal notes (auto-compact) are left untouched. | |
| 2384 | + | func (i *Interactive) ClearNotes(extName string) { | |
| 2385 | + | marker := "[" + extName + "] " | |
| 2386 | + | i.mu.Lock() | |
| 2387 | + | if len(i.extNotes) == 0 { | |
| 2388 | + | i.mu.Unlock() | |
| 2389 | + | return | |
| 2390 | + | } | |
| 2391 | + | kept := i.extNotes[:0:0] | |
| 2392 | + | changed := false | |
| 2393 | + | for _, line := range i.extNotes { | |
| 2394 | + | if strings.Contains(line, marker) { | |
| 2395 | + | changed = true | |
| 2396 | + | continue | |
| 2397 | + | } | |
| 2398 | + | kept = append(kept, line) | |
| 2399 | + | } | |
| 2400 | + | if changed { | |
| 2401 | + | i.extNotes = kept | |
| 2402 | + | } | |
| 2403 | + | i.mu.Unlock() | |
| 2404 | + | if changed { | |
| 2405 | + | i.invalidate() | |
| 2406 | + | } | |
| 2407 | + | } | |
| 2408 | + | ||
| 2359 | 2409 | // Submit feeds text through the agent loop as if the user had typed it. | |
| 2360 | 2410 | func (i *Interactive) Submit(text string) { | |
| 2361 | 2411 | if cmd, ok := shellEscapeCommand(text); ok { | |
| @@ -2382,6 +2432,7 @@ func (i *Interactive) ApplyChangedCWD(ag *core.Agent, provider, model, cwd strin | |||
| 2382 | 2432 | i.cfg.Model = model | |
| 2383 | 2433 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| 2384 | 2434 | i.toolOrder = nil | |
| 2435 | + | i.toolGate = map[string]int{} | |
| 2385 | 2436 | i.helpBlock = nil | |
| 2386 | 2437 | i.parkedTurn = 0 | |
| 2387 | 2438 | i.statusErr = "" | |
| @@ -3051,6 +3102,7 @@ func (i *Interactive) runSlash(ctx context.Context, cmd string) (done bool) { | |||
| 3051 | 3102 | i.mu.Lock() | |
| 3052 | 3103 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| 3053 | 3104 | i.toolOrder = nil | |
| 3105 | + | i.toolGate = map[string]int{} | |
| 3054 | 3106 | i.statusErr = "" | |
| 3055 | 3107 | i.statusOK = "" | |
| 3056 | 3108 | i.helpBlock = nil | |
| @@ -3170,6 +3222,7 @@ func (i *Interactive) runSlash(ctx context.Context, cmd string) (done bool) { | |||
| 3170 | 3222 | i.mu.Lock() | |
| 3171 | 3223 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| 3172 | 3224 | i.toolOrder = nil | |
| 3225 | + | i.toolGate = map[string]int{} | |
| 3173 | 3226 | i.helpBlock = nil | |
| 3174 | 3227 | i.parkedTurn = 0 | |
| 3175 | 3228 | i.statusOK = "cwd " + i.cfg.CWD | |
| @@ -4048,6 +4101,7 @@ func (i *Interactive) runCompact(parent context.Context, auto bool) { | |||
| 4048 | 4101 | i.lastCtxInput = 0 | |
| 4049 | 4102 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| 4050 | 4103 | i.toolOrder = nil | |
| 4104 | + | i.toolGate = map[string]int{} | |
| 4051 | 4105 | i.view.InvalidateRenderCache() | |
| 4052 | 4106 | // Pop queued prompt if any. | |
| 4053 | 4107 | if len(i.queued) > 0 { | |
| @@ -4236,7 +4290,9 @@ func (i *Interactive) startTurnWithImages(parent context.Context, prompt string, | |||
| 4236 | 4290 | i.streamOn = true | |
| 4237 | 4291 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| 4238 | 4292 | i.toolOrder = nil | |
| 4293 | + | i.toolGate = map[string]int{} | |
| 4239 | 4294 | i.shellBlock = nil // sending a prompt clears any parked shell-escape log | |
| 4295 | + | i.extNotes = nil // ext notes are one-shot; a new prompt clears them | |
| 4240 | 4296 | i.scrollOffset = 0 // jump back to the bottom on new turn | |
| 4241 | 4297 | // Reset the auto-follow baseline so the very next render at | |
| 4242 | 4298 | // interactive.go:1053 doesn't see a synthetic shrink between | |
| @@ -4502,6 +4558,7 @@ func (i *Interactive) handleEvent(ev core.AgentEvent) { | |||
| 4502 | 4558 | // will populate fresh entries for this turn's tools. | |
| 4503 | 4559 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| 4504 | 4560 | i.toolOrder = nil | |
| 4561 | + | i.toolGate = map[string]int{} | |
| 4505 | 4562 | case core.EvTextDelta: | |
| 4506 | 4563 | // Buffer into streamPending; the paintPace ticker drains | |
| 4507 | 4564 | // it into i.streaming a few runes at a time for a smooth | |
| @@ -4535,6 +4592,7 @@ func (i *Interactive) handleEvent(ev core.AgentEvent) { | |||
| 4535 | 4592 | Streaming: true, | |
| 4536 | 4593 | } | |
| 4537 | 4594 | i.toolOrder = append(i.toolOrder, e.ID) | |
| 4595 | + | i.gateToolLocked(e.ID) | |
| 4538 | 4596 | } | |
| 4539 | 4597 | case core.EvToolUseArgs: | |
| 4540 | 4598 | if tc, ok := i.toolCalls[e.ID]; ok { | |
| @@ -4566,6 +4624,7 @@ func (i *Interactive) handleEvent(ev core.AgentEvent) { | |||
| 4566 | 4624 | Args: tui.ShortArgs(e.Name, e.Args), | |
| 4567 | 4625 | } | |
| 4568 | 4626 | i.toolOrder = append(i.toolOrder, e.ID) | |
| 4627 | + | i.gateToolLocked(e.ID) | |
| 4569 | 4628 | } | |
| 4570 | 4629 | case core.EvToolResult: | |
| 4571 | 4630 | if tc, ok := i.toolCalls[e.ID]; ok { | |
| @@ -5500,6 +5559,55 @@ func (i *Interactive) resetStreamingStateLocked() { | |||
| 5500 | 5559 | i.streamPending = i.streamPending[:0] | |
| 5501 | 5560 | i.streamFlushPending = false | |
| 5502 | 5561 | i.streamOn = false | |
| 5562 | + | i.openAllToolGatesLocked() | |
| 5563 | + | } | |
| 5564 | + | ||
| 5565 | + | // openAllToolGatesLocked drops every pending tool gate so that any | |
| 5566 | + | // tool registered during this turn renders unconditionally from now | |
| 5567 | + | // on. Called when streaming finalizes (the paced text has fully | |
| 5568 | + | // drained and `streaming` is about to reset to length 0): without | |
| 5569 | + | // this, the gate comparison against a freshly-reset streaming buffer | |
| 5570 | + | // would wrongly re-hide tools that had already cleared their gate. | |
| 5571 | + | // Must be called with i.mu held. | |
| 5572 | + | func (i *Interactive) openAllToolGatesLocked() { | |
| 5573 | + | for id := range i.toolGate { | |
| 5574 | + | i.toolGate[id] = 0 | |
| 5575 | + | } | |
| 5576 | + | } | |
| 5577 | + | ||
| 5578 | + | // gateToolLocked records the stream position at which a tool call may | |
| 5579 | + | // become visible. The gate is the total length the streaming buffer | |
| 5580 | + | // will reach once the pacer has drained everything currently queued | |
| 5581 | + | // (already painted + still pending). Holding the tool block back | |
| 5582 | + | // until the pacer crosses that mark guarantees the prose emitted | |
| 5583 | + | // before the tool call finishes typing out above it, instead of the | |
| 5584 | + | // tool block snapping in while the paragraph is still filling in. | |
| 5585 | + | // | |
| 5586 | + | // We only gate while text is actively streaming. If no stream is in | |
| 5587 | + | // flight (gate 0), the tool shows immediately, which is the correct | |
| 5588 | + | // behaviour for tool-only turns and replayed sessions. First | |
| 5589 | + | // registration wins so a later EvToolCall can't move an existing | |
| 5590 | + | // gate. Must be called with i.mu held. | |
| 5591 | + | func (i *Interactive) gateToolLocked(id string) { | |
| 5592 | + | if _, ok := i.toolGate[id]; ok { | |
| 5593 | + | return | |
| 5594 | + | } | |
| 5595 | + | if !i.streamOn { | |
| 5596 | + | i.toolGate[id] = 0 | |
| 5597 | + | return | |
| 5598 | + | } | |
| 5599 | + | i.toolGate[id] = i.streaming.Len() + len(i.streamPending) | |
| 5600 | + | } | |
| 5601 | + | ||
| 5602 | + | // toolGateOpenLocked reports whether the gated tool block may render | |
| 5603 | + | // yet, i.e. the pacer has drained enough text to reach the position | |
| 5604 | + | // recorded when the tool call arrived. Must be called with i.mu held. | |
| 5605 | + | func (i *Interactive) toolGateOpenLocked(id string) bool { | |
| 5606 | + | gate, ok := i.toolGate[id] | |
| 5607 | + | if !ok || gate == 0 { | |
| 5608 | + | return true | |
| 5609 | + | } | |
| 5610 | + | return i.streaming.Len() >= gate | |
| 5503 | 5611 | } | |
| 5504 | 5612 | ||
| 5505 | 5613 | // assistantMessageSideEffects runs the non-visual hooks attached to | |
| @@ -5572,6 +5680,7 @@ func (i *Interactive) runStreamPacer(ctx context.Context) { | |||
| 5572 | 5680 | i.streamFlushPending = false | |
| 5573 | 5681 | i.streaming.Reset() | |
| 5574 | 5682 | i.streamOn = false | |
| 5683 | + | i.openAllToolGatesLocked() | |
| 5575 | 5684 | i.mu.Unlock() | |
| 5576 | 5685 | i.invalidate() | |
| 5577 | 5686 | continue | |
| @@ -260,17 +260,28 @@ type Interactive struct { | |||||
| 260 | streamFlushPending bool | 260 | streamFlushPending bool | ||
| 261 | toolCalls map[string]*tui.ToolCallView | 261 | toolCalls map[string]*tui.ToolCallView | ||
| 262 | toolOrder []string | 262 | toolOrder []string | ||
| 263 | - | statusErr string | 263 | + | // toolGate records, per tool-call id, how many runes of paced |
| 264 | - | statusOK string | 264 | + | // assistant text must have drained into `streaming` before that |
| 265 | - | liveBlock []string // live streaming/tool progress rendered outside scrollback | 265 | + | // tool block may appear. It exists to make stream ordering |
| 266 | - | helpBlock []string // rendered above the chat when /help was typed | 266 | + | // deterministic: a tool call can arrive from the provider while |
| 267 | - | cumUsage provider.Usage | 267 | + | // the prose that logically precedes it is still being typed out |
| 268 | - | lastCtxInput int // input_tokens of the most recent turn — approximates current context size | 268 | + | // by the pacer. Without gating, the tool block would render |
| 269 | - | busy bool | 269 | + | // immediately while the intro paragraph keeps filling in below |
| 270 | - | dirty chan struct{} | 270 | + | // it. We snapshot the total expected stream length (already |
| 271 | - | cancelTurn context.CancelFunc | 271 | + | // streamed + still pending) at the moment the tool starts, and |
| 272 | - | scrollOffset int // rows from the bottom; 0 = pinned to latest | 272 | + | // hold the block back until the pacer reaches it. |
| 273 | - | prevScrollOffset int // last value redraw snapped against; tracks intent | 273 | + | toolGate map[string]int |
| 274 | + | statusErr string | |||
| 275 | + | statusOK string | |||
| 276 | + | liveBlock []string // live streaming/tool progress rendered outside scrollback | |||
| 277 | + | helpBlock []string // rendered above the chat when /help was typed | |||
| 278 | + | cumUsage provider.Usage | |||
| 279 | + | lastCtxInput int // input_tokens of the most recent turn — approximates current context size | |||
| 280 | + | busy bool | |||
| 281 | + | dirty chan struct{} | |||
| 282 | + | cancelTurn context.CancelFunc | |||
| 283 | + | scrollOffset int // rows from the bottom; 0 = pinned to latest | |||
| 284 | + | prevScrollOffset int // last value redraw snapped against; tracks intent | |||
| 274 | 285 | ||||
| 275 | // prevChatLen and prevChatCols track the chat buffer's size at the | 286 | // prevChatLen and prevChatCols track the chat buffer's size at the | ||
| 276 | // last redraw so that when content grows below the user's viewport | 287 | // last redraw so that when content grows below the user's viewport | ||
| @@ -440,6 +451,7 @@ func NewInteractive(cfg InteractiveConfig) *Interactive { | |||||
| 440 | ed: tui.NewEditor(cfg.Theme.AccentBar(cfg.Theme.Accent)), | 451 | ed: tui.NewEditor(cfg.Theme.AccentBar(cfg.Theme.Accent)), | ||
| 441 | rend: renderer, | 452 | rend: renderer, | ||
| 442 | toolCalls: map[string]*tui.ToolCallView{}, | 453 | toolCalls: map[string]*tui.ToolCallView{}, | ||
| 454 | + | toolGate: map[string]int{}, | |||
| 443 | dirty: make(chan struct{}, 8), | 455 | dirty: make(chan struct{}, 8), | ||
| 444 | dialog: newLoginDialog(), | 456 | dialog: newLoginDialog(), | ||
| 445 | modelDialog: newModelDialog(), | 457 | modelDialog: newModelDialog(), | ||
| @@ -824,6 +836,14 @@ func (i *Interactive) buildChatLocked(cols int) []string { | |||||
| 824 | i.view.ToolCalls = i.view.ToolCalls[:0] | 836 | i.view.ToolCalls = i.view.ToolCalls[:0] | ||
| 825 | if i.busy { | 837 | if i.busy { | ||
| 826 | for _, id := range i.toolOrder { | 838 | for _, id := range i.toolOrder { | ||
| 839 | + | // Deterministic ordering: a tool block stays hidden until | |||
| 840 | + | // the paced assistant text that preceded it has finished | |||
| 841 | + | // typing out. toolOrder is append-only in arrival order, | |||
| 842 | + | // so once one tool is still gated, every later tool is too, | |||
| 843 | + | // so stop here to avoid showing a tool out of sequence. | |||
| 844 | + | if !i.toolGateOpenLocked(id) { | |||
| 845 | + | break | |||
| 846 | + | } | |||
| 827 | if tc, ok := i.toolCalls[id]; ok { | 847 | if tc, ok := i.toolCalls[id]; ok { | ||
| 828 | i.view.ToolCalls = append(i.view.ToolCalls, *tc) | 848 | i.view.ToolCalls = append(i.view.ToolCalls, *tc) | ||
| 829 | } | 849 | } | ||
| @@ -2356,6 +2376,36 @@ func (i *Interactive) Notify(extName, level, message string) { | |||||
| 2356 | i.invalidate() | 2376 | i.invalidate() | ||
| 2357 | } | 2377 | } | ||
| 2358 | 2378 | ||||
| 2379 | + | // ClearNotes removes every note line owned by extName from the | |||
| 2380 | + | // bottom-sticky ext-notes block. Extensions use this to retract a | |||
| 2381 | + | // transient status line (e.g. an approval prompt) once it no longer | |||
| 2382 | + | // applies, instead of leaving it stacked forever. Notes from other | |||
| 2383 | + | // extensions and internal notes (auto-compact) are left untouched. | |||
| 2384 | + | func (i *Interactive) ClearNotes(extName string) { | |||
| 2385 | + | marker := "[" + extName + "] " | |||
| 2386 | + | i.mu.Lock() | |||
| 2387 | + | if len(i.extNotes) == 0 { | |||
| 2388 | + | i.mu.Unlock() | |||
| 2389 | + | return | |||
| 2390 | + | } | |||
| 2391 | + | kept := i.extNotes[:0:0] | |||
| 2392 | + | changed := false | |||
| 2393 | + | for _, line := range i.extNotes { | |||
| 2394 | + | if strings.Contains(line, marker) { | |||
| 2395 | + | changed = true | |||
| 2396 | + | continue | |||
| 2397 | + | } | |||
| 2398 | + | kept = append(kept, line) | |||
| 2399 | + | } | |||
| 2400 | + | if changed { | |||
| 2401 | + | i.extNotes = kept | |||
| 2402 | + | } | |||
| 2403 | + | i.mu.Unlock() | |||
| 2404 | + | if changed { | |||
| 2405 | + | i.invalidate() | |||
| 2406 | + | } | |||
| 2407 | + | } | |||
| 2408 | + | ||||
| 2359 | // Submit feeds text through the agent loop as if the user had typed it. | 2409 | // Submit feeds text through the agent loop as if the user had typed it. | ||
| 2360 | func (i *Interactive) Submit(text string) { | 2410 | func (i *Interactive) Submit(text string) { | ||
| 2361 | if cmd, ok := shellEscapeCommand(text); ok { | 2411 | if cmd, ok := shellEscapeCommand(text); ok { | ||
| @@ -2382,6 +2432,7 @@ func (i *Interactive) ApplyChangedCWD(ag *core.Agent, provider, model, cwd strin | |||||
| 2382 | i.cfg.Model = model | 2432 | i.cfg.Model = model | ||
| 2383 | i.toolCalls = map[string]*tui.ToolCallView{} | 2433 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
| 2384 | i.toolOrder = nil | 2434 | i.toolOrder = nil | ||
| 2435 | + | i.toolGate = map[string]int{} | |||
| 2385 | i.helpBlock = nil | 2436 | i.helpBlock = nil | ||
| 2386 | i.parkedTurn = 0 | 2437 | i.parkedTurn = 0 | ||
| 2387 | i.statusErr = "" | 2438 | i.statusErr = "" | ||
| @@ -3051,6 +3102,7 @@ func (i *Interactive) runSlash(ctx context.Context, cmd string) (done bool) { | |||||
| 3051 | i.mu.Lock() | 3102 | i.mu.Lock() | ||
| 3052 | i.toolCalls = map[string]*tui.ToolCallView{} | 3103 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
| 3053 | i.toolOrder = nil | 3104 | i.toolOrder = nil | ||
| 3105 | + | i.toolGate = map[string]int{} | |||
| 3054 | i.statusErr = "" | 3106 | i.statusErr = "" | ||
| 3055 | i.statusOK = "" | 3107 | i.statusOK = "" | ||
| 3056 | i.helpBlock = nil | 3108 | i.helpBlock = nil | ||
| @@ -3170,6 +3222,7 @@ func (i *Interactive) runSlash(ctx context.Context, cmd string) (done bool) { | |||||
| 3170 | i.mu.Lock() | 3222 | i.mu.Lock() | ||
| 3171 | i.toolCalls = map[string]*tui.ToolCallView{} | 3223 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
| 3172 | i.toolOrder = nil | 3224 | i.toolOrder = nil | ||
| 3225 | + | i.toolGate = map[string]int{} | |||
| 3173 | i.helpBlock = nil | 3226 | i.helpBlock = nil | ||
| 3174 | i.parkedTurn = 0 | 3227 | i.parkedTurn = 0 | ||
| 3175 | i.statusOK = "cwd " + i.cfg.CWD | 3228 | i.statusOK = "cwd " + i.cfg.CWD | ||
| @@ -4048,6 +4101,7 @@ func (i *Interactive) runCompact(parent context.Context, auto bool) { | |||||
| 4048 | i.lastCtxInput = 0 | 4101 | i.lastCtxInput = 0 | ||
| 4049 | i.toolCalls = map[string]*tui.ToolCallView{} | 4102 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
| 4050 | i.toolOrder = nil | 4103 | i.toolOrder = nil | ||
| 4104 | + | i.toolGate = map[string]int{} | |||
| 4051 | i.view.InvalidateRenderCache() | 4105 | i.view.InvalidateRenderCache() | ||
| 4052 | // Pop queued prompt if any. | 4106 | // Pop queued prompt if any. | ||
| 4053 | if len(i.queued) > 0 { | 4107 | if len(i.queued) > 0 { | ||
| @@ -4236,7 +4290,9 @@ func (i *Interactive) startTurnWithImages(parent context.Context, prompt string, | |||||
| 4236 | i.streamOn = true | 4290 | i.streamOn = true | ||
| 4237 | i.toolCalls = map[string]*tui.ToolCallView{} | 4291 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
| 4238 | i.toolOrder = nil | 4292 | i.toolOrder = nil | ||
| 4293 | + | i.toolGate = map[string]int{} | |||
| 4239 | i.shellBlock = nil // sending a prompt clears any parked shell-escape log | 4294 | i.shellBlock = nil // sending a prompt clears any parked shell-escape log | ||
| 4295 | + | i.extNotes = nil // ext notes are one-shot; a new prompt clears them | |||
| 4240 | i.scrollOffset = 0 // jump back to the bottom on new turn | 4296 | i.scrollOffset = 0 // jump back to the bottom on new turn | ||
| 4241 | // Reset the auto-follow baseline so the very next render at | 4297 | // Reset the auto-follow baseline so the very next render at | ||
| 4242 | // interactive.go:1053 doesn't see a synthetic shrink between | 4298 | // interactive.go:1053 doesn't see a synthetic shrink between | ||
| @@ -4502,6 +4558,7 @@ func (i *Interactive) handleEvent(ev core.AgentEvent) { | |||||
| 4502 | // will populate fresh entries for this turn's tools. | 4558 | // will populate fresh entries for this turn's tools. | ||
| 4503 | i.toolCalls = map[string]*tui.ToolCallView{} | 4559 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
| 4504 | i.toolOrder = nil | 4560 | i.toolOrder = nil | ||
| 4561 | + | i.toolGate = map[string]int{} | |||
| 4505 | case core.EvTextDelta: | 4562 | case core.EvTextDelta: | ||
| 4506 | // Buffer into streamPending; the paintPace ticker drains | 4563 | // Buffer into streamPending; the paintPace ticker drains | ||
| 4507 | // it into i.streaming a few runes at a time for a smooth | 4564 | // it into i.streaming a few runes at a time for a smooth | ||
| @@ -4535,6 +4592,7 @@ func (i *Interactive) handleEvent(ev core.AgentEvent) { | |||||
| 4535 | Streaming: true, | 4592 | Streaming: true, | ||
| 4536 | } | 4593 | } | ||
| 4537 | i.toolOrder = append(i.toolOrder, e.ID) | 4594 | i.toolOrder = append(i.toolOrder, e.ID) | ||
| 4595 | + | i.gateToolLocked(e.ID) | |||
| 4538 | } | 4596 | } | ||
| 4539 | case core.EvToolUseArgs: | 4597 | case core.EvToolUseArgs: | ||
| 4540 | if tc, ok := i.toolCalls[e.ID]; ok { | 4598 | if tc, ok := i.toolCalls[e.ID]; ok { | ||
| @@ -4566,6 +4624,7 @@ func (i *Interactive) handleEvent(ev core.AgentEvent) { | |||||
| 4566 | Args: tui.ShortArgs(e.Name, e.Args), | 4624 | Args: tui.ShortArgs(e.Name, e.Args), | ||
| 4567 | } | 4625 | } | ||
| 4568 | i.toolOrder = append(i.toolOrder, e.ID) | 4626 | i.toolOrder = append(i.toolOrder, e.ID) | ||
| 4627 | + | i.gateToolLocked(e.ID) | |||
| 4569 | } | 4628 | } | ||
| 4570 | case core.EvToolResult: | 4629 | case core.EvToolResult: | ||
| 4571 | if tc, ok := i.toolCalls[e.ID]; ok { | 4630 | if tc, ok := i.toolCalls[e.ID]; ok { | ||
| @@ -5500,6 +5559,55 @@ func (i *Interactive) resetStreamingStateLocked() { | |||||
| 5500 | i.streamPending = i.streamPending[:0] | 5559 | i.streamPending = i.streamPending[:0] | ||
| 5501 | i.streamFlushPending = false | 5560 | i.streamFlushPending = false | ||
| 5502 | i.streamOn = false | 5561 | i.streamOn = false | ||
| 5562 | + | i.openAllToolGatesLocked() | |||
| 5563 | + | } | |||
| 5564 | + | ||||
| 5565 | + | // openAllToolGatesLocked drops every pending tool gate so that any | |||
| 5566 | + | // tool registered during this turn renders unconditionally from now | |||
| 5567 | + | // on. Called when streaming finalizes (the paced text has fully | |||
| 5568 | + | // drained and `streaming` is about to reset to length 0): without | |||
| 5569 | + | // this, the gate comparison against a freshly-reset streaming buffer | |||
| 5570 | + | // would wrongly re-hide tools that had already cleared their gate. | |||
| 5571 | + | // Must be called with i.mu held. | |||
| 5572 | + | func (i *Interactive) openAllToolGatesLocked() { | |||
| 5573 | + | for id := range i.toolGate { | |||
| 5574 | + | i.toolGate[id] = 0 | |||
| 5575 | + | } | |||
| 5576 | + | } | |||
| 5577 | + | ||||
| 5578 | + | // gateToolLocked records the stream position at which a tool call may | |||
| 5579 | + | // become visible. The gate is the total length the streaming buffer | |||
| 5580 | + | // will reach once the pacer has drained everything currently queued | |||
| 5581 | + | // (already painted + still pending). Holding the tool block back | |||
| 5582 | + | // until the pacer crosses that mark guarantees the prose emitted | |||
| 5583 | + | // before the tool call finishes typing out above it, instead of the | |||
| 5584 | + | // tool block snapping in while the paragraph is still filling in. | |||
| 5585 | + | // | |||
| 5586 | + | // We only gate while text is actively streaming. If no stream is in | |||
| 5587 | + | // flight (gate 0), the tool shows immediately, which is the correct | |||
| 5588 | + | // behaviour for tool-only turns and replayed sessions. First | |||
| 5589 | + | // registration wins so a later EvToolCall can't move an existing | |||
| 5590 | + | // gate. Must be called with i.mu held. | |||
| 5591 | + | func (i *Interactive) gateToolLocked(id string) { | |||
| 5592 | + | if _, ok := i.toolGate[id]; ok { | |||
| 5593 | + | return | |||
| 5594 | + | } | |||
| 5595 | + | if !i.streamOn { | |||
| 5596 | + | i.toolGate[id] = 0 | |||
| 5597 | + | return | |||
| 5598 | + | } | |||
| 5599 | + | i.toolGate[id] = i.streaming.Len() + len(i.streamPending) | |||
| 5600 | + | } | |||
| 5601 | + | ||||
| 5602 | + | // toolGateOpenLocked reports whether the gated tool block may render | |||
| 5603 | + | // yet, i.e. the pacer has drained enough text to reach the position | |||
| 5604 | + | // recorded when the tool call arrived. Must be called with i.mu held. | |||
| 5605 | + | func (i *Interactive) toolGateOpenLocked(id string) bool { | |||
| 5606 | + | gate, ok := i.toolGate[id] | |||
| 5607 | + | if !ok || gate == 0 { | |||
| 5608 | + | return true | |||
| 5609 | + | } | |||
| 5610 | + | return i.streaming.Len() >= gate | |||
| 5503 | } | 5611 | } | ||
| 5504 | 5612 | ||||
| 5505 | // assistantMessageSideEffects runs the non-visual hooks attached to | 5613 | // assistantMessageSideEffects runs the non-visual hooks attached to | ||
| @@ -5572,6 +5680,7 @@ func (i *Interactive) runStreamPacer(ctx context.Context) { | |||||
| 5572 | i.streamFlushPending = false | 5680 | i.streamFlushPending = false | ||
| 5573 | i.streaming.Reset() | 5681 | i.streaming.Reset() | ||
| 5574 | i.streamOn = false | 5682 | i.streamOn = false | ||
| 5683 | + | i.openAllToolGatesLocked() | |||
| 5575 | i.mu.Unlock() | 5684 | i.mu.Unlock() | ||
| 5576 | i.invalidate() | 5685 | i.invalidate() | ||
| 5577 | continue | 5686 | continue | ||
A packages/agent/modes/tool_gate_test.go +101 -0
| @@ -0,0 +1,101 @@ | |||
| 1 | + | package modes | |
| 2 | + | ||
| 3 | + | import "testing" | |
| 4 | + | ||
| 5 | + | func newGateTestInteractive() *Interactive { | |
| 6 | + | return &Interactive{ | |
| 7 | + | toolGate: map[string]int{}, | |
| 8 | + | dirty: make(chan struct{}, 1), | |
| 9 | + | } | |
| 10 | + | } | |
| 11 | + | ||
| 12 | + | // While paced text is still draining, a tool that arrives mid-stream | |
| 13 | + | // must stay gated until the streaming buffer reaches the position | |
| 14 | + | // captured when the tool started. | |
| 15 | + | func TestToolGateHoldsToolUntilTextDrains(t *testing.T) { | |
| 16 | + | i := newGateTestInteractive() | |
| 17 | + | ||
| 18 | + | // Simulate a turn that streamed 10 runes already with 20 more | |
| 19 | + | // queued in the pacer when the tool call arrives. | |
| 20 | + | i.streamOn = true | |
| 21 | + | i.streaming.WriteString("0123456789") // 10 painted | |
| 22 | + | i.streamPending = []rune("01234567890123456789") | |
| 23 | + | ||
| 24 | + | i.gateToolLocked("t1") | |
| 25 | + | if got := i.toolGate["t1"]; got != 30 { | |
| 26 | + | t.Fatalf("gate = %d, want 30 (10 painted + 20 pending)", got) | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | if i.toolGateOpenLocked("t1") { | |
| 30 | + | t.Fatal("tool should be gated while only 10/30 runes painted") | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | // Drain halfway: still gated. | |
| 34 | + | i.streaming.WriteString("0123456789") // 20 painted | |
| 35 | + | if i.toolGateOpenLocked("t1") { | |
| 36 | + | t.Fatal("tool should still be gated at 20/30") | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | // Reach the gate: now visible. | |
| 40 | + | i.streaming.WriteString("0123456789") // 30 painted | |
| 41 | + | if !i.toolGateOpenLocked("t1") { | |
| 42 | + | t.Fatal("tool should be visible once streaming reaches the gate") | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | // A tool call on a turn with no active text stream shows immediately. | |
| 47 | + | func TestToolGateOpenWhenNoStream(t *testing.T) { | |
| 48 | + | i := newGateTestInteractive() | |
| 49 | + | i.streamOn = false | |
| 50 | + | ||
| 51 | + | i.gateToolLocked("t1") | |
| 52 | + | if got := i.toolGate["t1"]; got != 0 { | |
| 53 | + | t.Fatalf("gate = %d, want 0 for non-streaming turn", got) | |
| 54 | + | } | |
| 55 | + | if !i.toolGateOpenLocked("t1") { | |
| 56 | + | t.Fatal("tool should be visible immediately when no text is streaming") | |
| 57 | + | } | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | // First registration wins: a later EvToolCall must not move an | |
| 61 | + | // existing gate (e.g. push it forward as more text arrives). | |
| 62 | + | func TestToolGateFirstRegistrationWins(t *testing.T) { | |
| 63 | + | i := newGateTestInteractive() | |
| 64 | + | i.streamOn = true | |
| 65 | + | i.streaming.WriteString("01234") // 5 | |
| 66 | + | i.streamPending = []rune("01234") | |
| 67 | + | ||
| 68 | + | i.gateToolLocked("t1") // gate 10 | |
| 69 | + | first := i.toolGate["t1"] | |
| 70 | + | ||
| 71 | + | // More text queues up, then the same tool is re-registered. | |
| 72 | + | i.streamPending = append(i.streamPending, []rune("567890")...) | |
| 73 | + | i.gateToolLocked("t1") | |
| 74 | + | ||
| 75 | + | if i.toolGate["t1"] != first { | |
| 76 | + | t.Fatalf("gate moved from %d to %d; first registration must win", first, i.toolGate["t1"]) | |
| 77 | + | } | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | // Once streaming finalizes, the buffer resets to length 0; gates that | |
| 81 | + | // were already satisfied must not re-hide their tools. | |
| 82 | + | func TestOpenAllToolGatesSurvivesStreamReset(t *testing.T) { | |
| 83 | + | i := newGateTestInteractive() | |
| 84 | + | i.streamOn = true | |
| 85 | + | i.streaming.WriteString("0123456789") | |
| 86 | + | i.gateToolLocked("t1") // gate 10, satisfied | |
| 87 | + | ||
| 88 | + | if !i.toolGateOpenLocked("t1") { | |
| 89 | + | t.Fatal("precondition: tool should be open before reset") | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | // Finalize the stream (mirrors resetStreamingStateLocked / pacer | |
| 93 | + | // flush): buffer resets to 0 but gates are opened first. | |
| 94 | + | i.openAllToolGatesLocked() | |
| 95 | + | i.streaming.Reset() | |
| 96 | + | i.streamOn = false | |
| 97 | + | ||
| 98 | + | if !i.toolGateOpenLocked("t1") { | |
| 99 | + | t.Fatal("tool re-hidden after stream reset; gate should have been opened") | |
| 100 | + | } | |
| 101 | + | } | |
| @@ -0,0 +1,101 @@ | |||||
| 1 | + | package modes | |||
| 2 | + | ||||
| 3 | + | import "testing" | |||
| 4 | + | ||||
| 5 | + | func newGateTestInteractive() *Interactive { | |||
| 6 | + | return &Interactive{ | |||
| 7 | + | toolGate: map[string]int{}, | |||
| 8 | + | dirty: make(chan struct{}, 1), | |||
| 9 | + | } | |||
| 10 | + | } | |||
| 11 | + | ||||
| 12 | + | // While paced text is still draining, a tool that arrives mid-stream | |||
| 13 | + | // must stay gated until the streaming buffer reaches the position | |||
| 14 | + | // captured when the tool started. | |||
| 15 | + | func TestToolGateHoldsToolUntilTextDrains(t *testing.T) { | |||
| 16 | + | i := newGateTestInteractive() | |||
| 17 | + | ||||
| 18 | + | // Simulate a turn that streamed 10 runes already with 20 more | |||
| 19 | + | // queued in the pacer when the tool call arrives. | |||
| 20 | + | i.streamOn = true | |||
| 21 | + | i.streaming.WriteString("0123456789") // 10 painted | |||
| 22 | + | i.streamPending = []rune("01234567890123456789") | |||
| 23 | + | ||||
| 24 | + | i.gateToolLocked("t1") | |||
| 25 | + | if got := i.toolGate["t1"]; got != 30 { | |||
| 26 | + | t.Fatalf("gate = %d, want 30 (10 painted + 20 pending)", got) | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | if i.toolGateOpenLocked("t1") { | |||
| 30 | + | t.Fatal("tool should be gated while only 10/30 runes painted") | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | // Drain halfway: still gated. | |||
| 34 | + | i.streaming.WriteString("0123456789") // 20 painted | |||
| 35 | + | if i.toolGateOpenLocked("t1") { | |||
| 36 | + | t.Fatal("tool should still be gated at 20/30") | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | // Reach the gate: now visible. | |||
| 40 | + | i.streaming.WriteString("0123456789") // 30 painted | |||
| 41 | + | if !i.toolGateOpenLocked("t1") { | |||
| 42 | + | t.Fatal("tool should be visible once streaming reaches the gate") | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | // A tool call on a turn with no active text stream shows immediately. | |||
| 47 | + | func TestToolGateOpenWhenNoStream(t *testing.T) { | |||
| 48 | + | i := newGateTestInteractive() | |||
| 49 | + | i.streamOn = false | |||
| 50 | + | ||||
| 51 | + | i.gateToolLocked("t1") | |||
| 52 | + | if got := i.toolGate["t1"]; got != 0 { | |||
| 53 | + | t.Fatalf("gate = %d, want 0 for non-streaming turn", got) | |||
| 54 | + | } | |||
| 55 | + | if !i.toolGateOpenLocked("t1") { | |||
| 56 | + | t.Fatal("tool should be visible immediately when no text is streaming") | |||
| 57 | + | } | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | // First registration wins: a later EvToolCall must not move an | |||
| 61 | + | // existing gate (e.g. push it forward as more text arrives). | |||
| 62 | + | func TestToolGateFirstRegistrationWins(t *testing.T) { | |||
| 63 | + | i := newGateTestInteractive() | |||
| 64 | + | i.streamOn = true | |||
| 65 | + | i.streaming.WriteString("01234") // 5 | |||
| 66 | + | i.streamPending = []rune("01234") | |||
| 67 | + | ||||
| 68 | + | i.gateToolLocked("t1") // gate 10 | |||
| 69 | + | first := i.toolGate["t1"] | |||
| 70 | + | ||||
| 71 | + | // More text queues up, then the same tool is re-registered. | |||
| 72 | + | i.streamPending = append(i.streamPending, []rune("567890")...) | |||
| 73 | + | i.gateToolLocked("t1") | |||
| 74 | + | ||||
| 75 | + | if i.toolGate["t1"] != first { | |||
| 76 | + | t.Fatalf("gate moved from %d to %d; first registration must win", first, i.toolGate["t1"]) | |||
| 77 | + | } | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | // Once streaming finalizes, the buffer resets to length 0; gates that | |||
| 81 | + | // were already satisfied must not re-hide their tools. | |||
| 82 | + | func TestOpenAllToolGatesSurvivesStreamReset(t *testing.T) { | |||
| 83 | + | i := newGateTestInteractive() | |||
| 84 | + | i.streamOn = true | |||
| 85 | + | i.streaming.WriteString("0123456789") | |||
| 86 | + | i.gateToolLocked("t1") // gate 10, satisfied | |||
| 87 | + | ||||
| 88 | + | if !i.toolGateOpenLocked("t1") { | |||
| 89 | + | t.Fatal("precondition: tool should be open before reset") | |||
| 90 | + | } | |||
| 91 | + | ||||
| 92 | + | // Finalize the stream (mirrors resetStreamingStateLocked / pacer | |||
| 93 | + | // flush): buffer resets to 0 but gates are opened first. | |||
| 94 | + | i.openAllToolGatesLocked() | |||
| 95 | + | i.streaming.Reset() | |||
| 96 | + | i.streamOn = false | |||
| 97 | + | ||||
| 98 | + | if !i.toolGateOpenLocked("t1") { | |||
| 99 | + | t.Fatal("tool re-hidden after stream reset; gate should have been opened") | |||
| 100 | + | } | |||
| 101 | + | } | |||
M packages/agent/rpc.go +8 -0
| @@ -142,6 +142,14 @@ func (h *rpcExtHooks) Display(extName, text string) { | |||
| 142 | 142 | }) | |
| 143 | 143 | } | |
| 144 | 144 | } | |
| 145 | + | func (h *rpcExtHooks) ClearNotes(extName string) { | |
| 146 | + | if h.server != nil { | |
| 147 | + | h.server.writeEvent(map[string]any{ | |
| 148 | + | "type": "ext_clear_notes", | |
| 149 | + | "extension": extName, | |
| 150 | + | }) | |
| 151 | + | } | |
| 152 | + | } | |
| 145 | 153 | func (h *rpcExtHooks) Submit(string) {} // ignored in rpc mode | |
| 146 | 154 | func (h *rpcExtHooks) SubmitSlash(string) {} // ignored in rpc mode | |
| 147 | 155 | func (h *rpcExtHooks) Insert(string) {} // ignored in rpc mode | |
| @@ -142,6 +142,14 @@ func (h *rpcExtHooks) Display(extName, text string) { | |||||
| 142 | }) | 142 | }) | ||
| 143 | } | 143 | } | ||
| 144 | } | 144 | } | ||
| 145 | + | func (h *rpcExtHooks) ClearNotes(extName string) { | |||
| 146 | + | if h.server != nil { | |||
| 147 | + | h.server.writeEvent(map[string]any{ | |||
| 148 | + | "type": "ext_clear_notes", | |||
| 149 | + | "extension": extName, | |||
| 150 | + | }) | |||
| 151 | + | } | |||
| 152 | + | } | |||
| 145 | func (h *rpcExtHooks) Submit(string) {} // ignored in rpc mode | 153 | func (h *rpcExtHooks) Submit(string) {} // ignored in rpc mode | ||
| 146 | func (h *rpcExtHooks) SubmitSlash(string) {} // ignored in rpc mode | 154 | func (h *rpcExtHooks) SubmitSlash(string) {} // ignored in rpc mode | ||
| 147 | func (h *rpcExtHooks) Insert(string) {} // ignored in rpc mode | 155 | func (h *rpcExtHooks) Insert(string) {} // ignored in rpc mode | ||
M packages/agent/skills/builtin/write-zot-extension/SKILL.md +6 -0
| @@ -145,9 +145,15 @@ you can — newer extensions on faster hosts shave that 250ms off. | |||
| 145 | 145 | {"type":"event_intercept_response","id":"ghi","block":true, | |
| 146 | 146 | "reason":"refused: command matches the danger pattern \"rm -rf\""} | |
| 147 | 147 | {"type":"notify","level":"info","message":"refreshed cache"} | |
| 148 | + | {"type":"clear_notes"} | |
| 148 | 149 | {"type":"shutdown_ack"} | |
| 149 | 150 | ``` | |
| 150 | 151 | ||
| 152 | + | `notify` notes are one-shot: they clear when the user sends their next | |
| 153 | + | prompt (and on `esc` / `/clear`). Send `clear_notes` to retract every | |
| 154 | + | note this extension pushed earlier (e.g. a transient approval prompt) | |
| 155 | + | without waiting for the next turn; other extensions' notes are kept. | |
| 156 | + | ||
| 151 | 157 | `command_response.action` values: | |
| 152 | 158 | - `"prompt"` — submit `prompt` as a fresh user message | |
| 153 | 159 | - `"insert"` — drop `insert` into the editor at the cursor | |
| @@ -145,9 +145,15 @@ you can — newer extensions on faster hosts shave that 250ms off. | |||||
| 145 | {"type":"event_intercept_response","id":"ghi","block":true, | 145 | {"type":"event_intercept_response","id":"ghi","block":true, | ||
| 146 | "reason":"refused: command matches the danger pattern \"rm -rf\""} | 146 | "reason":"refused: command matches the danger pattern \"rm -rf\""} | ||
| 147 | {"type":"notify","level":"info","message":"refreshed cache"} | 147 | {"type":"notify","level":"info","message":"refreshed cache"} | ||
| 148 | + | {"type":"clear_notes"} | |||
| 148 | {"type":"shutdown_ack"} | 149 | {"type":"shutdown_ack"} | ||
| 149 | ``` | 150 | ``` | ||
| 150 | 151 | ||||
| 152 | + | `notify` notes are one-shot: they clear when the user sends their next | |||
| 153 | + | prompt (and on `esc` / `/clear`). Send `clear_notes` to retract every | |||
| 154 | + | note this extension pushed earlier (e.g. a transient approval prompt) | |||
| 155 | + | without waiting for the next turn; other extensions' notes are kept. | |||
| 156 | + | ||||
| 151 | `command_response.action` values: | 157 | `command_response.action` values: | ||
| 152 | - `"prompt"` — submit `prompt` as a fresh user message | 158 | - `"prompt"` — submit `prompt` as a fresh user message | ||
| 153 | - `"insert"` — drop `insert` into the editor at the cursor | 159 | - `"insert"` — drop `insert` into the editor at the cursor | ||
M packages/provider/amazon_bedrock.go +59 -1
| @@ -216,6 +216,61 @@ type bedrockRequest struct { | |||
| 216 | 216 | } `json:"toolConfig,omitempty"` | |
| 217 | 217 | } | |
| 218 | 218 | ||
| 219 | + | func normalizeBedrockToolResults(msgs []Message) []Message { | |
| 220 | + | resultByID := map[string]ToolResultBlock{} | |
| 221 | + | for _, m := range msgs { | |
| 222 | + | for _, c := range m.Content { | |
| 223 | + | if tr, ok := c.(ToolResultBlock); ok { | |
| 224 | + | if _, exists := resultByID[tr.CallID]; !exists { | |
| 225 | + | resultByID[tr.CallID] = tr | |
| 226 | + | } | |
| 227 | + | } | |
| 228 | + | } | |
| 229 | + | } | |
| 230 | + | ||
| 231 | + | out := make([]Message, 0, len(msgs)) | |
| 232 | + | for _, m := range msgs { | |
| 233 | + | copy := m | |
| 234 | + | copy.Content = nil | |
| 235 | + | var toolCalls []ToolCallBlock | |
| 236 | + | for _, c := range m.Content { | |
| 237 | + | switch v := c.(type) { | |
| 238 | + | case ToolResultBlock: | |
| 239 | + | // Bedrock requires toolResult blocks immediately after the | |
| 240 | + | // assistant toolUse they answer. Reinsert them from the | |
| 241 | + | // assistant pass below instead of preserving their original | |
| 242 | + | // location, which may be separated by user text in active | |
| 243 | + | // sessions. | |
| 244 | + | continue | |
| 245 | + | case ToolCallBlock: | |
| 246 | + | toolCalls = append(toolCalls, v) | |
| 247 | + | } | |
| 248 | + | copy.Content = append(copy.Content, c) | |
| 249 | + | } | |
| 250 | + | if len(copy.Content) > 0 { | |
| 251 | + | out = append(out, copy) | |
| 252 | + | } | |
| 253 | + | if m.Role != RoleAssistant || len(toolCalls) == 0 { | |
| 254 | + | continue | |
| 255 | + | } | |
| 256 | + | ||
| 257 | + | results := make([]Content, 0, len(toolCalls)) | |
| 258 | + | for _, tc := range toolCalls { | |
| 259 | + | if tr, ok := resultByID[tc.ID]; ok { | |
| 260 | + | results = append(results, tr) | |
| 261 | + | continue | |
| 262 | + | } | |
| 263 | + | results = append(results, ToolResultBlock{ | |
| 264 | + | CallID: tc.ID, | |
| 265 | + | IsError: true, | |
| 266 | + | Content: []Content{TextBlock{Text: "tool call did not complete before the next user message"}}, | |
| 267 | + | }) | |
| 268 | + | } | |
| 269 | + | out = append(out, Message{Role: RoleTool, Content: results, Time: m.Time}) | |
| 270 | + | } | |
| 271 | + | return out | |
| 272 | + | } | |
| 273 | + | ||
| 219 | 274 | func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |
| 220 | 275 | out := &bedrockRequest{} | |
| 221 | 276 | if req.System != "" { | |
| @@ -226,7 +281,7 @@ func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |||
| 226 | 281 | if out.InferenceConfig.MaxTokens == 0 { | |
| 227 | 282 | out.InferenceConfig.MaxTokens = 4096 | |
| 228 | 283 | } | |
| 229 | - | for _, m := range req.Messages { | |
| 284 | + | for _, m := range normalizeBedrockToolResults(req.Messages) { | |
| 230 | 285 | role := string(m.Role) | |
| 231 | 286 | if role == "tool" { | |
| 232 | 287 | role = "user" | |
| @@ -267,6 +322,9 @@ func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |||
| 267 | 322 | }) | |
| 268 | 323 | } | |
| 269 | 324 | } | |
| 325 | + | if len(bm.Content) == 0 { | |
| 326 | + | continue | |
| 327 | + | } | |
| 270 | 328 | out.Messages = append(out.Messages, bm) | |
| 271 | 329 | } | |
| 272 | 330 | if len(req.Tools) > 0 { | |
| @@ -216,6 +216,61 @@ type bedrockRequest struct { | |||||
| 216 | } `json:"toolConfig,omitempty"` | 216 | } `json:"toolConfig,omitempty"` | ||
| 217 | } | 217 | } | ||
| 218 | 218 | ||||
| 219 | + | func normalizeBedrockToolResults(msgs []Message) []Message { | |||
| 220 | + | resultByID := map[string]ToolResultBlock{} | |||
| 221 | + | for _, m := range msgs { | |||
| 222 | + | for _, c := range m.Content { | |||
| 223 | + | if tr, ok := c.(ToolResultBlock); ok { | |||
| 224 | + | if _, exists := resultByID[tr.CallID]; !exists { | |||
| 225 | + | resultByID[tr.CallID] = tr | |||
| 226 | + | } | |||
| 227 | + | } | |||
| 228 | + | } | |||
| 229 | + | } | |||
| 230 | + | ||||
| 231 | + | out := make([]Message, 0, len(msgs)) | |||
| 232 | + | for _, m := range msgs { | |||
| 233 | + | copy := m | |||
| 234 | + | copy.Content = nil | |||
| 235 | + | var toolCalls []ToolCallBlock | |||
| 236 | + | for _, c := range m.Content { | |||
| 237 | + | switch v := c.(type) { | |||
| 238 | + | case ToolResultBlock: | |||
| 239 | + | // Bedrock requires toolResult blocks immediately after the | |||
| 240 | + | // assistant toolUse they answer. Reinsert them from the | |||
| 241 | + | // assistant pass below instead of preserving their original | |||
| 242 | + | // location, which may be separated by user text in active | |||
| 243 | + | // sessions. | |||
| 244 | + | continue | |||
| 245 | + | case ToolCallBlock: | |||
| 246 | + | toolCalls = append(toolCalls, v) | |||
| 247 | + | } | |||
| 248 | + | copy.Content = append(copy.Content, c) | |||
| 249 | + | } | |||
| 250 | + | if len(copy.Content) > 0 { | |||
| 251 | + | out = append(out, copy) | |||
| 252 | + | } | |||
| 253 | + | if m.Role != RoleAssistant || len(toolCalls) == 0 { | |||
| 254 | + | continue | |||
| 255 | + | } | |||
| 256 | + | ||||
| 257 | + | results := make([]Content, 0, len(toolCalls)) | |||
| 258 | + | for _, tc := range toolCalls { | |||
| 259 | + | if tr, ok := resultByID[tc.ID]; ok { | |||
| 260 | + | results = append(results, tr) | |||
| 261 | + | continue | |||
| 262 | + | } | |||
| 263 | + | results = append(results, ToolResultBlock{ | |||
| 264 | + | CallID: tc.ID, | |||
| 265 | + | IsError: true, | |||
| 266 | + | Content: []Content{TextBlock{Text: "tool call did not complete before the next user message"}}, | |||
| 267 | + | }) | |||
| 268 | + | } | |||
| 269 | + | out = append(out, Message{Role: RoleTool, Content: results, Time: m.Time}) | |||
| 270 | + | } | |||
| 271 | + | return out | |||
| 272 | + | } | |||
| 273 | + | ||||
| 219 | func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | 274 | func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | ||
| 220 | out := &bedrockRequest{} | 275 | out := &bedrockRequest{} | ||
| 221 | if req.System != "" { | 276 | if req.System != "" { | ||
| @@ -226,7 +281,7 @@ func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |||||
| 226 | if out.InferenceConfig.MaxTokens == 0 { | 281 | if out.InferenceConfig.MaxTokens == 0 { | ||
| 227 | out.InferenceConfig.MaxTokens = 4096 | 282 | out.InferenceConfig.MaxTokens = 4096 | ||
| 228 | } | 283 | } | ||
| 229 | - | for _, m := range req.Messages { | 284 | + | for _, m := range normalizeBedrockToolResults(req.Messages) { |
| 230 | role := string(m.Role) | 285 | role := string(m.Role) | ||
| 231 | if role == "tool" { | 286 | if role == "tool" { | ||
| 232 | role = "user" | 287 | role = "user" | ||
| @@ -267,6 +322,9 @@ func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |||||
| 267 | }) | 322 | }) | ||
| 268 | } | 323 | } | ||
| 269 | } | 324 | } | ||
| 325 | + | if len(bm.Content) == 0 { | |||
| 326 | + | continue | |||
| 327 | + | } | |||
| 270 | out.Messages = append(out.Messages, bm) | 328 | out.Messages = append(out.Messages, bm) | ||
| 271 | } | 329 | } | ||
| 272 | if len(req.Tools) > 0 { | 330 | if len(req.Tools) > 0 { | ||
M packages/provider/amazon_bedrock_test.go +66 -0
| @@ -2,6 +2,7 @@ package provider | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "bytes" | |
| 5 | + | "encoding/json" | |
| 5 | 6 | "net/http" | |
| 6 | 7 | "strings" | |
| 7 | 8 | "testing" | |
| @@ -82,6 +83,71 @@ func TestReadAWSCredentialsFile(t *testing.T) { | |||
| 82 | 83 | } | |
| 83 | 84 | } | |
| 84 | 85 | ||
| 86 | + | func TestNormalizeBedrockToolResultsMovesResultsAdjacentToToolUse(t *testing.T) { | |
| 87 | + | msgs := []Message{ | |
| 88 | + | {Role: RoleAssistant, Content: []Content{ToolCallBlock{ID: "tool-1", Name: "edit", Arguments: json.RawMessage(`{"path":"a"}`)}}}, | |
| 89 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "what's wrong"}}}, | |
| 90 | + | {Role: RoleTool, Content: []Content{ToolResultBlock{CallID: "tool-1", Content: []Content{TextBlock{Text: "edited"}}}}}, | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | out := normalizeBedrockToolResults(msgs) | |
| 94 | + | if len(out) != 3 { | |
| 95 | + | t.Fatalf("got %d messages, want 3: %+v", len(out), out) | |
| 96 | + | } | |
| 97 | + | if out[0].Role != RoleAssistant { | |
| 98 | + | t.Fatalf("first role = %s, want assistant", out[0].Role) | |
| 99 | + | } | |
| 100 | + | if out[1].Role != RoleTool { | |
| 101 | + | t.Fatalf("second role = %s, want tool", out[1].Role) | |
| 102 | + | } | |
| 103 | + | tr, ok := out[1].Content[0].(ToolResultBlock) | |
| 104 | + | if !ok { | |
| 105 | + | t.Fatalf("second message content = %T, want ToolResultBlock", out[1].Content[0]) | |
| 106 | + | } | |
| 107 | + | if tr.CallID != "tool-1" || tr.IsError { | |
| 108 | + | t.Fatalf("unexpected tool result: %+v", tr) | |
| 109 | + | } | |
| 110 | + | if out[2].Role != RoleUser { | |
| 111 | + | t.Fatalf("third role = %s, want user", out[2].Role) | |
| 112 | + | } | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | func TestNormalizeBedrockToolResultsInjectsMissingResult(t *testing.T) { | |
| 116 | + | msgs := []Message{ | |
| 117 | + | {Role: RoleAssistant, Content: []Content{ToolCallBlock{ID: "tool-1", Name: "edit", Arguments: json.RawMessage(`{}`)}}}, | |
| 118 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "what's wrong"}}}, | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | out := normalizeBedrockToolResults(msgs) | |
| 122 | + | if len(out) != 3 { | |
| 123 | + | t.Fatalf("got %d messages, want 3: %+v", len(out), out) | |
| 124 | + | } | |
| 125 | + | tr, ok := out[1].Content[0].(ToolResultBlock) | |
| 126 | + | if !ok { | |
| 127 | + | t.Fatalf("second message content = %T, want ToolResultBlock", out[1].Content[0]) | |
| 128 | + | } | |
| 129 | + | if tr.CallID != "tool-1" || !tr.IsError { | |
| 130 | + | t.Fatalf("unexpected synthetic result: %+v", tr) | |
| 131 | + | } | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | func TestBedrockBuildRequestSkipsEmptyToolMessages(t *testing.T) { | |
| 135 | + | client := &bedrockClient{} | |
| 136 | + | req, err := client.buildRequest(Request{Messages: []Message{ | |
| 137 | + | {Role: RoleTool, Content: []Content{ToolResultBlock{CallID: "missing", Content: []Content{TextBlock{Text: "orphan"}}}}}, | |
| 138 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "hello"}}}, | |
| 139 | + | }}) | |
| 140 | + | if err != nil { | |
| 141 | + | t.Fatal(err) | |
| 142 | + | } | |
| 143 | + | if len(req.Messages) != 1 { | |
| 144 | + | t.Fatalf("got %d bedrock messages, want 1: %+v", len(req.Messages), req.Messages) | |
| 145 | + | } | |
| 146 | + | if req.Messages[0].Role != "user" { | |
| 147 | + | t.Fatalf("role = %s, want user", req.Messages[0].Role) | |
| 148 | + | } | |
| 149 | + | } | |
| 150 | + | ||
| 85 | 151 | func TestBedrockEventPayloadHelpers(t *testing.T) { | |
| 86 | 152 | wrapped := []byte(`{"contentBlockDelta":{"contentBlockIndex":0,"delta":{"text":"Hello"}}}`) | |
| 87 | 153 | if got := bedrockEventTypeFromPayload(wrapped); got != "contentBlockDelta" { | |
| @@ -2,6 +2,7 @@ package provider | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | + | "encoding/json" | |||
| 5 | "net/http" | 6 | "net/http" | ||
| 6 | "strings" | 7 | "strings" | ||
| 7 | "testing" | 8 | "testing" | ||
| @@ -82,6 +83,71 @@ func TestReadAWSCredentialsFile(t *testing.T) { | |||||
| 82 | } | 83 | } | ||
| 83 | } | 84 | } | ||
| 84 | 85 | ||||
| 86 | + | func TestNormalizeBedrockToolResultsMovesResultsAdjacentToToolUse(t *testing.T) { | |||
| 87 | + | msgs := []Message{ | |||
| 88 | + | {Role: RoleAssistant, Content: []Content{ToolCallBlock{ID: "tool-1", Name: "edit", Arguments: json.RawMessage(`{"path":"a"}`)}}}, | |||
| 89 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "what's wrong"}}}, | |||
| 90 | + | {Role: RoleTool, Content: []Content{ToolResultBlock{CallID: "tool-1", Content: []Content{TextBlock{Text: "edited"}}}}}, | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | out := normalizeBedrockToolResults(msgs) | |||
| 94 | + | if len(out) != 3 { | |||
| 95 | + | t.Fatalf("got %d messages, want 3: %+v", len(out), out) | |||
| 96 | + | } | |||
| 97 | + | if out[0].Role != RoleAssistant { | |||
| 98 | + | t.Fatalf("first role = %s, want assistant", out[0].Role) | |||
| 99 | + | } | |||
| 100 | + | if out[1].Role != RoleTool { | |||
| 101 | + | t.Fatalf("second role = %s, want tool", out[1].Role) | |||
| 102 | + | } | |||
| 103 | + | tr, ok := out[1].Content[0].(ToolResultBlock) | |||
| 104 | + | if !ok { | |||
| 105 | + | t.Fatalf("second message content = %T, want ToolResultBlock", out[1].Content[0]) | |||
| 106 | + | } | |||
| 107 | + | if tr.CallID != "tool-1" || tr.IsError { | |||
| 108 | + | t.Fatalf("unexpected tool result: %+v", tr) | |||
| 109 | + | } | |||
| 110 | + | if out[2].Role != RoleUser { | |||
| 111 | + | t.Fatalf("third role = %s, want user", out[2].Role) | |||
| 112 | + | } | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | func TestNormalizeBedrockToolResultsInjectsMissingResult(t *testing.T) { | |||
| 116 | + | msgs := []Message{ | |||
| 117 | + | {Role: RoleAssistant, Content: []Content{ToolCallBlock{ID: "tool-1", Name: "edit", Arguments: json.RawMessage(`{}`)}}}, | |||
| 118 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "what's wrong"}}}, | |||
| 119 | + | } | |||
| 120 | + | ||||
| 121 | + | out := normalizeBedrockToolResults(msgs) | |||
| 122 | + | if len(out) != 3 { | |||
| 123 | + | t.Fatalf("got %d messages, want 3: %+v", len(out), out) | |||
| 124 | + | } | |||
| 125 | + | tr, ok := out[1].Content[0].(ToolResultBlock) | |||
| 126 | + | if !ok { | |||
| 127 | + | t.Fatalf("second message content = %T, want ToolResultBlock", out[1].Content[0]) | |||
| 128 | + | } | |||
| 129 | + | if tr.CallID != "tool-1" || !tr.IsError { | |||
| 130 | + | t.Fatalf("unexpected synthetic result: %+v", tr) | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | func TestBedrockBuildRequestSkipsEmptyToolMessages(t *testing.T) { | |||
| 135 | + | client := &bedrockClient{} | |||
| 136 | + | req, err := client.buildRequest(Request{Messages: []Message{ | |||
| 137 | + | {Role: RoleTool, Content: []Content{ToolResultBlock{CallID: "missing", Content: []Content{TextBlock{Text: "orphan"}}}}}, | |||
| 138 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "hello"}}}, | |||
| 139 | + | }}) | |||
| 140 | + | if err != nil { | |||
| 141 | + | t.Fatal(err) | |||
| 142 | + | } | |||
| 143 | + | if len(req.Messages) != 1 { | |||
| 144 | + | t.Fatalf("got %d bedrock messages, want 1: %+v", len(req.Messages), req.Messages) | |||
| 145 | + | } | |||
| 146 | + | if req.Messages[0].Role != "user" { | |||
| 147 | + | t.Fatalf("role = %s, want user", req.Messages[0].Role) | |||
| 148 | + | } | |||
| 149 | + | } | |||
| 150 | + | ||||
| 85 | func TestBedrockEventPayloadHelpers(t *testing.T) { | 151 | func TestBedrockEventPayloadHelpers(t *testing.T) { | ||
| 86 | wrapped := []byte(`{"contentBlockDelta":{"contentBlockIndex":0,"delta":{"text":"Hello"}}}`) | 152 | wrapped := []byte(`{"contentBlockDelta":{"contentBlockIndex":0,"delta":{"text":"Hello"}}}`) | ||
| 87 | if got := bedrockEventTypeFromPayload(wrapped); got != "contentBlockDelta" { | 153 | if got := bedrockEventTypeFromPayload(wrapped); got != "contentBlockDelta" { | ||