mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 17:56:14 -10:00
Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c427052f9b | ||
|
|
fce9569362 | ||
|
|
3259ea3f6a | ||
|
|
f6e298ad20 | ||
|
|
15ddc79e75 |
No files matched your search
@@ -37,6 +37,7 @@ report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json
|
||||
|
||||
/zero
|
||||
/zero.exe
|
||||
/zero-skills
|
||||
/snake-game.html
|
||||
|
||||
# Superpowers skill artifacts (brainstorm mockups, specs, plans) — local only
|
||||
|
||||
@@ -1,185 +0,0 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Post-edit LSP diagnostics used to run synchronously inside edit_file /
|
||||
// write_file: every edit blocked its tool result on a fresh-publish wait with
|
||||
// a quiet-period debounce (≥300ms floor that reset on each publish, 10s cap) —
|
||||
// the largest recurring latency tax in an edit-heavy session. Collection is
|
||||
// now asynchronous: a file is enqueued the moment a mutating tool finishes, a
|
||||
// single worker checks it while the rest of the tool batch (and any
|
||||
// self-correct verification) runs, and the loop drains completed results just
|
||||
// before building the NEXT provider request, appending errors as a user-role
|
||||
// nudge. The model always issues another request after an edit turn — it has
|
||||
// to read its tool results — so it still sees introduced errors at the same
|
||||
// decision point as before, without any tool call stalling on the language
|
||||
// server.
|
||||
//
|
||||
// A file re-edited before its check runs is checked once in its final state
|
||||
// (the checker re-reads the file); a file re-edited after its check completed
|
||||
// is re-enqueued and the newer result wins. When the worker is still busy at
|
||||
// drain time, the drain gives up after a short wait and the results are
|
||||
// delivered on the following turn instead — they stay accurate because any
|
||||
// further edit re-enqueues the file.
|
||||
|
||||
// asyncDiagnosticsDrainTimeout bounds how long a turn waits on the in-flight
|
||||
// check before deferring delivery to the next turn. Most checks finish well
|
||||
// under it (debounce floor + analysis); a cold language server on a large
|
||||
// repo does not get to stall the loop the way the old inline 10s cap could.
|
||||
// A var so tests can shorten the wait.
|
||||
var asyncDiagnosticsDrainTimeout = 3 * time.Second
|
||||
|
||||
// asyncDiagnosticsFinalDrainTimeout is the wait used when the run is about to
|
||||
// FINALIZE (natural completion or the max-turns summary): there is no later
|
||||
// turn to defer to, so the gate waits out the old inline-era budget rather
|
||||
// than silently dropping an error the last edit introduced. A var so tests
|
||||
// can shorten the wait.
|
||||
var asyncDiagnosticsFinalDrainTimeout = 10 * time.Second
|
||||
|
||||
// asyncDiagnosticsNudge prefixes the drained diagnostics blocks; phrased like
|
||||
// the old inline block so the model reacts the same way.
|
||||
const asyncDiagnosticsNudge = "Diagnostics after your recent edits (fix any errors you introduced):\n"
|
||||
|
||||
type asyncDiagnostics struct {
|
||||
check func(context.Context, string) string
|
||||
workspaceRoot string
|
||||
|
||||
mu sync.Mutex
|
||||
queue []string
|
||||
queued map[string]bool
|
||||
results map[string]string
|
||||
// working is non-nil while the worker goroutine runs and is closed when it
|
||||
// exits, so drain can wait for quiescence without polling.
|
||||
working chan struct{}
|
||||
}
|
||||
|
||||
// newAsyncDiagnostics returns nil when check is nil, and every method on a nil
|
||||
// receiver is a no-op — callers wire it unconditionally.
|
||||
func newAsyncDiagnostics(check func(context.Context, string) string, workspaceRoot string) *asyncDiagnostics {
|
||||
if check == nil {
|
||||
return nil
|
||||
}
|
||||
return &asyncDiagnostics{
|
||||
check: check,
|
||||
workspaceRoot: workspaceRoot,
|
||||
queued: map[string]bool{},
|
||||
results: map[string]string{},
|
||||
}
|
||||
}
|
||||
|
||||
// enqueue schedules changed files for a background check. Paths are
|
||||
// workspace-relative from Result.ChangedFiles (absolute when under a granted
|
||||
// extra write root), mirroring SelfCorrector's resolution.
|
||||
func (diagnostics *asyncDiagnostics) enqueue(ctx context.Context, files []string) {
|
||||
if diagnostics == nil || len(files) == 0 {
|
||||
return
|
||||
}
|
||||
diagnostics.mu.Lock()
|
||||
defer diagnostics.mu.Unlock()
|
||||
for _, file := range files {
|
||||
path := diagnostics.absPath(file)
|
||||
if diagnostics.queued[path] {
|
||||
continue
|
||||
}
|
||||
diagnostics.queued[path] = true
|
||||
// A completed result for this file describes a pre-edit state now.
|
||||
delete(diagnostics.results, path)
|
||||
diagnostics.queue = append(diagnostics.queue, path)
|
||||
}
|
||||
if diagnostics.working == nil && len(diagnostics.queue) > 0 {
|
||||
done := make(chan struct{})
|
||||
diagnostics.working = done
|
||||
go diagnostics.work(ctx, done)
|
||||
}
|
||||
}
|
||||
|
||||
func (diagnostics *asyncDiagnostics) work(ctx context.Context, done chan struct{}) {
|
||||
defer close(done)
|
||||
for {
|
||||
diagnostics.mu.Lock()
|
||||
if len(diagnostics.queue) == 0 || ctx.Err() != nil {
|
||||
diagnostics.working = nil
|
||||
diagnostics.mu.Unlock()
|
||||
return
|
||||
}
|
||||
path := diagnostics.queue[0]
|
||||
diagnostics.queue = diagnostics.queue[1:]
|
||||
// Un-mark before checking so an edit that lands DURING the check
|
||||
// re-enqueues the file and it is checked again in its final state.
|
||||
delete(diagnostics.queued, path)
|
||||
diagnostics.mu.Unlock()
|
||||
|
||||
block := diagnostics.check(ctx, path)
|
||||
|
||||
diagnostics.mu.Lock()
|
||||
if block != "" {
|
||||
diagnostics.results[path] = block
|
||||
} else {
|
||||
delete(diagnostics.results, path)
|
||||
}
|
||||
diagnostics.mu.Unlock()
|
||||
}
|
||||
}
|
||||
|
||||
// drain waits briefly for the worker to go quiet, then formats and consumes
|
||||
// all completed error blocks as one nudge. Returns "" when there is nothing
|
||||
// to report or the worker is still busy (results then surface next turn).
|
||||
func (diagnostics *asyncDiagnostics) drain(ctx context.Context) string {
|
||||
return diagnostics.drainWithin(ctx, asyncDiagnosticsDrainTimeout)
|
||||
}
|
||||
|
||||
// drainFinal is drain with the finalization budget: called before the run's
|
||||
// last model request, where an undelivered result would otherwise be lost.
|
||||
func (diagnostics *asyncDiagnostics) drainFinal(ctx context.Context) string {
|
||||
return diagnostics.drainWithin(ctx, asyncDiagnosticsFinalDrainTimeout)
|
||||
}
|
||||
|
||||
func (diagnostics *asyncDiagnostics) drainWithin(ctx context.Context, timeout time.Duration) string {
|
||||
if diagnostics == nil {
|
||||
return ""
|
||||
}
|
||||
diagnostics.mu.Lock()
|
||||
busy := diagnostics.working
|
||||
diagnostics.mu.Unlock()
|
||||
if busy != nil {
|
||||
timer := time.NewTimer(timeout)
|
||||
defer timer.Stop()
|
||||
select {
|
||||
case <-busy:
|
||||
case <-timer.C:
|
||||
return ""
|
||||
case <-ctx.Done():
|
||||
return ""
|
||||
}
|
||||
}
|
||||
diagnostics.mu.Lock()
|
||||
defer diagnostics.mu.Unlock()
|
||||
if len(diagnostics.results) == 0 {
|
||||
return ""
|
||||
}
|
||||
paths := make([]string, 0, len(diagnostics.results))
|
||||
for path := range diagnostics.results {
|
||||
paths = append(paths, path)
|
||||
}
|
||||
sort.Strings(paths)
|
||||
blocks := make([]string, 0, len(paths))
|
||||
for _, path := range paths {
|
||||
blocks = append(blocks, diagnostics.results[path])
|
||||
}
|
||||
diagnostics.results = map[string]string{}
|
||||
return asyncDiagnosticsNudge + strings.Join(blocks, "\n")
|
||||
}
|
||||
|
||||
func (diagnostics *asyncDiagnostics) absPath(path string) string {
|
||||
if filepath.IsAbs(path) {
|
||||
return path
|
||||
}
|
||||
return filepath.Join(diagnostics.workspaceRoot, path)
|
||||
}
|
||||
@@ -1,338 +0,0 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
func TestAsyncDiagnosticsNilCollectorNoOps(t *testing.T) {
|
||||
var diagnostics *asyncDiagnostics
|
||||
if diagnostics != newAsyncDiagnostics(nil, "/ws") {
|
||||
t.Fatal("nil check must yield a nil collector")
|
||||
}
|
||||
diagnostics.enqueue(context.Background(), []string{"a.go"})
|
||||
if nudge := diagnostics.drain(context.Background()); nudge != "" {
|
||||
t.Fatalf("nil collector drain = %q, want empty", nudge)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAsyncDiagnosticsCollectsAndDrainsOnce(t *testing.T) {
|
||||
// A real absolute root: a literal "/ws" is not filepath.IsAbs on Windows
|
||||
// (no drive letter), which is exactly what the assertions below check.
|
||||
root := t.TempDir()
|
||||
var mu sync.Mutex
|
||||
var checked []string
|
||||
check := func(_ context.Context, absPath string) string {
|
||||
mu.Lock()
|
||||
checked = append(checked, absPath)
|
||||
mu.Unlock()
|
||||
if filepath.Base(absPath) == "clean.go" {
|
||||
return ""
|
||||
}
|
||||
return "ERR " + filepath.Base(absPath)
|
||||
}
|
||||
diagnostics := newAsyncDiagnostics(check, root)
|
||||
diagnostics.enqueue(context.Background(), []string{"broken.go", "clean.go"})
|
||||
|
||||
nudge := diagnostics.drain(context.Background())
|
||||
if !strings.HasPrefix(nudge, asyncDiagnosticsNudge) {
|
||||
t.Fatalf("nudge missing prefix: %q", nudge)
|
||||
}
|
||||
if !strings.Contains(nudge, "ERR broken.go") || strings.Contains(nudge, "clean.go") {
|
||||
t.Fatalf("nudge = %q, want broken.go errors only", nudge)
|
||||
}
|
||||
mu.Lock()
|
||||
for _, path := range checked {
|
||||
if !filepath.IsAbs(path) || !strings.HasPrefix(path, root+string(filepath.Separator)) {
|
||||
t.Fatalf("check received %q, want path under workspace root %q", path, root)
|
||||
}
|
||||
}
|
||||
mu.Unlock()
|
||||
if second := diagnostics.drain(context.Background()); second != "" {
|
||||
t.Fatalf("second drain = %q, want empty (results are consumed)", second)
|
||||
}
|
||||
}
|
||||
|
||||
// A check still running at drain time defers delivery to the next drain
|
||||
// instead of blocking the turn.
|
||||
func TestAsyncDiagnosticsSlowCheckDefersToNextDrain(t *testing.T) {
|
||||
previous := asyncDiagnosticsDrainTimeout
|
||||
asyncDiagnosticsDrainTimeout = 20 * time.Millisecond
|
||||
defer func() { asyncDiagnosticsDrainTimeout = previous }()
|
||||
|
||||
release := make(chan struct{})
|
||||
check := func(_ context.Context, absPath string) string {
|
||||
<-release
|
||||
return "ERR " + filepath.Base(absPath)
|
||||
}
|
||||
diagnostics := newAsyncDiagnostics(check, "/ws")
|
||||
diagnostics.enqueue(context.Background(), []string{"slow.go"})
|
||||
|
||||
start := time.Now()
|
||||
if nudge := diagnostics.drain(context.Background()); nudge != "" {
|
||||
t.Fatalf("drain during in-flight check = %q, want empty", nudge)
|
||||
}
|
||||
if elapsed := time.Since(start); elapsed > time.Second {
|
||||
t.Fatalf("drain blocked %v, want bounded by the (shortened) timeout", elapsed)
|
||||
}
|
||||
|
||||
close(release)
|
||||
deadline := time.Now().Add(2 * time.Second)
|
||||
for {
|
||||
if nudge := diagnostics.drain(context.Background()); nudge != "" {
|
||||
if !strings.Contains(nudge, "ERR slow.go") {
|
||||
t.Fatalf("deferred nudge = %q", nudge)
|
||||
}
|
||||
return
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
t.Fatal("deferred diagnostics never delivered")
|
||||
}
|
||||
time.Sleep(5 * time.Millisecond)
|
||||
}
|
||||
}
|
||||
|
||||
// Re-enqueueing a file after its check completed replaces the stale result.
|
||||
func TestAsyncDiagnosticsReEditReplacesResult(t *testing.T) {
|
||||
var mu sync.Mutex
|
||||
response := "ERR old"
|
||||
check := func(context.Context, string) string {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
return response
|
||||
}
|
||||
diagnostics := newAsyncDiagnostics(check, "/ws")
|
||||
|
||||
diagnostics.enqueue(context.Background(), []string{"a.go"})
|
||||
waitForIdle(t, diagnostics)
|
||||
mu.Lock()
|
||||
response = "ERR new"
|
||||
mu.Unlock()
|
||||
diagnostics.enqueue(context.Background(), []string{"a.go"})
|
||||
|
||||
nudge := diagnostics.drain(context.Background())
|
||||
if !strings.Contains(nudge, "ERR new") || strings.Contains(nudge, "ERR old") {
|
||||
t.Fatalf("nudge = %q, want only the re-check result", nudge)
|
||||
}
|
||||
}
|
||||
|
||||
func waitForIdle(t *testing.T, diagnostics *asyncDiagnostics) {
|
||||
t.Helper()
|
||||
deadline := time.Now().Add(2 * time.Second)
|
||||
for {
|
||||
diagnostics.mu.Lock()
|
||||
busy := diagnostics.working
|
||||
diagnostics.mu.Unlock()
|
||||
if busy == nil {
|
||||
return
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
t.Fatal("worker never went idle")
|
||||
}
|
||||
time.Sleep(2 * time.Millisecond)
|
||||
}
|
||||
}
|
||||
|
||||
// changedFilesTool is a fake mutating tool reporting a changed file without
|
||||
// touching the filesystem.
|
||||
type changedFilesTool struct{}
|
||||
|
||||
func (changedFilesTool) Name() string { return "fake_edit" }
|
||||
func (changedFilesTool) Description() string { return "test mutating tool" }
|
||||
func (changedFilesTool) Parameters() tools.Schema {
|
||||
return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
|
||||
}
|
||||
func (changedFilesTool) Safety() tools.Safety {
|
||||
return tools.Safety{SideEffect: tools.SideEffectWrite, Permission: tools.PermissionAllow, Reason: "test"}
|
||||
}
|
||||
func (changedFilesTool) Run(context.Context, map[string]any) tools.Result {
|
||||
return tools.Result{Status: tools.StatusOK, Output: "edited", ChangedFiles: []string{"main.go"}}
|
||||
}
|
||||
|
||||
// End-to-end through Run: an edit turn does NOT carry diagnostics in the tool
|
||||
// result (the old inline path), but the next request contains the background
|
||||
// diagnostics nudge, before the model finalizes.
|
||||
func TestRunDeliversAsyncDiagnosticsNudgeNextTurn(t *testing.T) {
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(changedFilesTool{})
|
||||
provider := &mockProvider{
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "fake_edit"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "done"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
},
|
||||
}
|
||||
root := t.TempDir()
|
||||
var checkedPath string
|
||||
var mu sync.Mutex
|
||||
|
||||
result, err := Run(context.Background(), "fix main.go", provider, Options{
|
||||
Registry: registry,
|
||||
Cwd: root,
|
||||
FileDiagnostics: func(_ context.Context, absPath string) string {
|
||||
mu.Lock()
|
||||
checkedPath = absPath
|
||||
mu.Unlock()
|
||||
return "main.go:1:1 error: boom"
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.FinalAnswer != "done" {
|
||||
t.Fatalf("final answer = %q", result.FinalAnswer)
|
||||
}
|
||||
if len(provider.requests) != 2 {
|
||||
t.Fatalf("expected 2 provider turns, got %d", len(provider.requests))
|
||||
}
|
||||
|
||||
// The tool result itself must NOT block on / embed diagnostics.
|
||||
for _, message := range provider.requests[1].Messages {
|
||||
if message.Role == zeroruntime.MessageRoleTool && strings.Contains(message.Content, "boom") {
|
||||
t.Fatalf("diagnostics leaked into the tool result: %q", message.Content)
|
||||
}
|
||||
}
|
||||
// The nudge must be present in the second request as a user message.
|
||||
found := false
|
||||
for _, message := range provider.requests[1].Messages {
|
||||
if message.Role == zeroruntime.MessageRoleUser && strings.HasPrefix(message.Content, asyncDiagnosticsNudge) {
|
||||
if !strings.Contains(message.Content, "boom") {
|
||||
t.Fatalf("nudge missing diagnostics: %q", message.Content)
|
||||
}
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("no async diagnostics nudge in the follow-up request")
|
||||
}
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
if want := filepath.Join(root, "main.go"); checkedPath != want {
|
||||
t.Fatalf("checked path = %q, want %q", checkedPath, want)
|
||||
}
|
||||
}
|
||||
|
||||
// When the run hits the maxTurns ceiling right after an edit turn, the
|
||||
// final-answer request must still carry the diagnostics nudge — otherwise an
|
||||
// error introduced by the last edit would go unreported in the summary.
|
||||
func TestRunDrainsAsyncDiagnosticsBeforeMaxTurnsFinalAnswer(t *testing.T) {
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(changedFilesTool{})
|
||||
provider := &mockProvider{
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "fake_edit"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "summary"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
result, err := Run(context.Background(), "fix main.go", provider, Options{
|
||||
Registry: registry,
|
||||
Cwd: t.TempDir(),
|
||||
MaxTurns: 1,
|
||||
FileDiagnostics: func(context.Context, string) string {
|
||||
return "main.go:1:1 error: boom"
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.FinalAnswer != "summary" {
|
||||
t.Fatalf("final answer = %q", result.FinalAnswer)
|
||||
}
|
||||
if len(provider.requests) != 2 {
|
||||
t.Fatalf("expected the max-turns final-answer request, got %d requests", len(provider.requests))
|
||||
}
|
||||
found := false
|
||||
for _, message := range provider.requests[1].Messages {
|
||||
if message.Role == zeroruntime.MessageRoleUser && strings.HasPrefix(message.Content, asyncDiagnosticsNudge) {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("max-turns final-answer request missing the diagnostics nudge")
|
||||
}
|
||||
}
|
||||
|
||||
// When the model finalizes while a slow check is still in flight (the
|
||||
// per-turn drain missed it), the finalization gate must wait it out and give
|
||||
// the model one more turn with the nudge instead of dropping the errors.
|
||||
func TestRunFinalizationGateDeliversLateDiagnostics(t *testing.T) {
|
||||
previous := asyncDiagnosticsDrainTimeout
|
||||
asyncDiagnosticsDrainTimeout = time.Millisecond
|
||||
defer func() { asyncDiagnosticsDrainTimeout = previous }()
|
||||
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(changedFilesTool{})
|
||||
provider := &mockProvider{
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "fake_edit"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "done"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "done after fix"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
result, err := Run(context.Background(), "fix main.go", provider, Options{
|
||||
Registry: registry,
|
||||
Cwd: t.TempDir(),
|
||||
FileDiagnostics: func(context.Context, string) string {
|
||||
// Slower than the (shortened) per-turn drain, well under the
|
||||
// finalization budget.
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
return "main.go:1:1 error: boom"
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.FinalAnswer != "done after fix" {
|
||||
t.Fatalf("final answer = %q, want the post-nudge answer", result.FinalAnswer)
|
||||
}
|
||||
if len(provider.requests) != 3 {
|
||||
t.Fatalf("expected a third request carrying the late nudge, got %d requests", len(provider.requests))
|
||||
}
|
||||
found := false
|
||||
for _, message := range provider.requests[2].Messages {
|
||||
if message.Role == zeroruntime.MessageRoleUser && strings.HasPrefix(message.Content, asyncDiagnosticsNudge) {
|
||||
if !strings.Contains(message.Content, "boom") {
|
||||
t.Fatalf("nudge missing diagnostics: %q", message.Content)
|
||||
}
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatal("finalization gate did not deliver the late diagnostics nudge")
|
||||
}
|
||||
}
|
||||
+6
-54
@@ -137,12 +137,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
guards := newGuardState()
|
||||
compactor := newCompactionState(options)
|
||||
|
||||
// Background post-edit diagnostics: files changed by mutating tools are
|
||||
// checked off the tool-call critical path and any errors are appended as a
|
||||
// nudge before the NEXT request (see async_diagnostics.go). nil when no
|
||||
// FileDiagnostics callback is wired; every method no-ops on nil.
|
||||
postEditDiagnostics := newAsyncDiagnostics(options.FileDiagnostics, options.Cwd)
|
||||
|
||||
// loaded tracks deferred-eligible tools the model has pulled via tool_search
|
||||
// during THIS run. It is consulted by partitionTools each turn to expose a
|
||||
// loaded tool's full schema; it lives only for the run (v1 within-run scope).
|
||||
@@ -165,17 +159,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
for turn := 0; turn < maxTurns; turn++ {
|
||||
result.Turns = turn + 1
|
||||
|
||||
// Deliver background post-edit diagnostics from the previous turn's edits
|
||||
// BEFORE compaction so the nudge is part of the request being budgeted.
|
||||
// A brief wait at most (asyncDiagnosticsDrainTimeout); an unfinished check
|
||||
// simply delivers on a later turn.
|
||||
if nudge := postEditDiagnostics.drain(ctx); nudge != "" {
|
||||
messages = append(messages, zeroruntime.Message{
|
||||
Role: zeroruntime.MessageRoleUser,
|
||||
Content: nudge,
|
||||
})
|
||||
}
|
||||
|
||||
// Build the per-turn tool list first so proactive compaction can include
|
||||
// the tool-definition tokens (they ride on every request) in its estimate.
|
||||
// partitionTools depends only on registry/permissions/options/loaded, not on
|
||||
@@ -491,19 +474,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
continue
|
||||
}
|
||||
}
|
||||
// Finalization diagnostics gate: edits from this run may still have
|
||||
// checks in flight — the per-turn drain waits only briefly and defers
|
||||
// to "a later turn", but a final answer means there is no later turn.
|
||||
// Wait out the full inline-era budget once; an introduced error gives
|
||||
// the model one more turn to see (and fix) it instead of being lost.
|
||||
// Free for runs that never edited: an idle collector returns "".
|
||||
if nudge := postEditDiagnostics.drainFinal(ctx); nudge != "" {
|
||||
messages = append(messages, zeroruntime.Message{
|
||||
Role: zeroruntime.MessageRoleUser,
|
||||
Content: nudge,
|
||||
})
|
||||
continue
|
||||
}
|
||||
result.FinalAnswer = collected.Text
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, nil
|
||||
@@ -620,14 +590,11 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
failureHint = toolFailureHint(call.Name, toolSchemaJSON(registry, call.Name), toolResult.Output)
|
||||
}
|
||||
|
||||
// Collect the files this successful mutating tool changed. Self-correct
|
||||
// verification runs once over the union after the batch; background
|
||||
// diagnostics start NOW so the language server analyzes while the rest
|
||||
// of the batch executes. A read-only tool (no ChangedFiles) never
|
||||
// contributes.
|
||||
if toolResult.Status == tools.StatusOK && len(toolResult.ChangedFiles) > 0 {
|
||||
// Post-edit self-correction: collect the files this successful mutating
|
||||
// tool changed; verification runs once over the union after the batch.
|
||||
// A read-only tool (no ChangedFiles) never contributes.
|
||||
if options.SelfCorrect != nil && toolResult.Status == tools.StatusOK && len(toolResult.ChangedFiles) > 0 {
|
||||
changedFilesThisBatch = append(changedFilesThisBatch, toolResult.ChangedFiles...)
|
||||
postEditDiagnostics.enqueue(ctx, toolResult.ChangedFiles)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -710,17 +677,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
// mid-run, not a natural completion (a finished run returns via the no-tool-call
|
||||
// success path before this). Under the headless completion gate that is INCOMPLETE,
|
||||
// not success, so a run that loops to the turn limit isn't reported as done.
|
||||
//
|
||||
// The final-answer call is one more model request, so diagnostics from the
|
||||
// LAST turn's edits get a drain too — with the finalization budget, since
|
||||
// there is no later turn to defer to — otherwise an error introduced by
|
||||
// the final edit would go unreported in the summary.
|
||||
if nudge := postEditDiagnostics.drainFinal(ctx); nudge != "" {
|
||||
messages = append(messages, zeroruntime.Message{
|
||||
Role: zeroruntime.MessageRoleUser,
|
||||
Content: nudge,
|
||||
})
|
||||
}
|
||||
if answer, finalMessages, finishReason := finalAnswerAfterMaxTurns(ctx, provider, messages, options); strings.TrimSpace(answer) != "" {
|
||||
result.FinalAnswer = answer
|
||||
result.FinishReason = finishReason
|
||||
@@ -1144,12 +1100,8 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
|
||||
// Per-session file version tracker so write_file/edit_file refuse to clobber
|
||||
// a file that changed on disk outside Zero since it was last read.
|
||||
FileTracker: options.FileTracker,
|
||||
// Post-edit diagnostics are NOT forwarded to the tools: they used to run
|
||||
// synchronously inside edit_file/write_file and block every mutating tool
|
||||
// call on the language server (≥300ms debounce floor, 10s cap). The loop
|
||||
// now checks changed files in the background and nudges the model before
|
||||
// its next request instead (see async_diagnostics.go). The tools' inline
|
||||
// mechanism (tools.RunOptions.Diagnostics) stays for direct API callers.
|
||||
// Inline post-edit diagnostics for mutating tools (nil = disabled).
|
||||
Diagnostics: options.FileDiagnostics,
|
||||
// Forward the run's operator tool filters so a filter-aware tool
|
||||
// (tool_search) never discloses or loads an operator-hidden deferred tool.
|
||||
EnabledTools: options.EnabledTools,
|
||||
|
||||
@@ -7,14 +7,18 @@ import (
|
||||
)
|
||||
|
||||
func TestSkillsContextCapsLongList(t *testing.T) {
|
||||
skills := make([]SkillInfo, 0, 40)
|
||||
for i := 0; i < 40; i++ {
|
||||
// 60 skills with realistic trigger-rich descriptions (~180 chars each, under
|
||||
// the 200-rune truncation) total well past the 4096-byte list budget, so the
|
||||
// overflow summary must kick in while the block stays bounded.
|
||||
longDesc := strings.Repeat("use when the request touches deployments, release notes, or verification; ", 2) + "number "
|
||||
skills := make([]SkillInfo, 0, 60)
|
||||
for i := 0; i < 60; i++ {
|
||||
n := strconv.Itoa(i)
|
||||
skills = append(skills, SkillInfo{Name: "skill-" + n, Description: "does something useful, number " + n})
|
||||
skills = append(skills, SkillInfo{Name: "skill-" + n, Description: longDesc + n})
|
||||
}
|
||||
got := skillsContext(Options{Skills: skills})
|
||||
if len(got) > 1200 {
|
||||
t.Fatalf("skills block should stay bounded, got %d bytes:\n%s", len(got), got)
|
||||
if len(got) > skillsContextListBudget+1000 {
|
||||
t.Fatalf("skills block should stay bounded near the %d-byte budget, got %d bytes:\n%s", skillsContextListBudget, len(got), got)
|
||||
}
|
||||
if !strings.Contains(got, "more (call skill") {
|
||||
t.Fatalf("expected an overflow summary line, got:\n%s", got)
|
||||
@@ -25,6 +29,27 @@ func TestSkillsContextCapsLongList(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A realistic mid-size skill set (20 skills, trigger-rich descriptions) must be
|
||||
// listed in FULL — no overflow summary. This pins the fix for the old 640-byte
|
||||
// budget, under which skills past ~#6 were invisible to the model and therefore
|
||||
// never triggered.
|
||||
func TestSkillsContextListsRealisticSetInFull(t *testing.T) {
|
||||
desc := "Use when the user asks about deployments, release notes, or pre-merge verification runs."
|
||||
skills := make([]SkillInfo, 0, 20)
|
||||
for i := 0; i < 20; i++ {
|
||||
skills = append(skills, SkillInfo{Name: "skill-" + strconv.Itoa(i), Description: desc})
|
||||
}
|
||||
got := skillsContext(Options{Skills: skills})
|
||||
if strings.Contains(got, "more (call skill") {
|
||||
t.Fatalf("20 described skills must all be listed without overflow, got:\n%s", got)
|
||||
}
|
||||
for i := 0; i < 20; i++ {
|
||||
if !strings.Contains(got, "- skill-"+strconv.Itoa(i)+":") {
|
||||
t.Fatalf("skill-%d missing from the list:\n%s", i, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillsContext(t *testing.T) {
|
||||
if got := skillsContext(Options{}); got != "" {
|
||||
t.Fatalf("no skills should yield an empty section, got %q", got)
|
||||
|
||||
@@ -191,10 +191,19 @@ func specialistDelegationContext(options Options) string {
|
||||
// when no skills are installed, so a skill-less run reproduces the previous prompt
|
||||
// byte-for-byte.
|
||||
// skillsContextListBudget bounds the bytes spent listing individual skills so a
|
||||
// workspace with dozens of them can't bloat every turn's prompt. Skills past the
|
||||
// budget are summarized as a count rather than dropped — the model can still load
|
||||
// any of them by name (an unknown name returns the full list from the skill tool).
|
||||
const skillsContextListBudget = 640
|
||||
// workspace with an extreme number of them can't bloat every turn's prompt.
|
||||
// Skills past the budget are summarized as a count rather than dropped — the
|
||||
// model can still load any of them by name (an unknown name returns the full
|
||||
// list from the skill tool).
|
||||
//
|
||||
// The budget is deliberately generous — roughly 18 skills with maximally long
|
||||
// (200-rune, ASCII) descriptions, or 40+ with typical terser ones; non-ASCII
|
||||
// descriptions consume it faster since the budget counts bytes. This list is
|
||||
// the model's ONLY discovery surface, so a skill squeezed out of it effectively
|
||||
// never triggers. The previous 640-byte cap silently broke invocation for
|
||||
// anyone with more than ~6 skills — the model cannot match a request against a
|
||||
// skill it cannot see.
|
||||
const skillsContextListBudget = 4096
|
||||
|
||||
func skillsContext(options Options) string {
|
||||
if len(options.Skills) == 0 {
|
||||
@@ -202,7 +211,7 @@ func skillsContext(options Options) string {
|
||||
}
|
||||
var b strings.Builder
|
||||
b.WriteString("<available_skills>\n")
|
||||
b.WriteString("Reusable, on-demand instruction sets you can load with the skill tool. When a request matches one, call skill with its exact name to pull in its guidance before acting — do not guess names or skip a relevant skill.\n")
|
||||
b.WriteString("Reusable, on-demand instruction sets you can load with the skill tool. Before acting on a request, scan this list; when the request matches a skill's name or description, call skill with its exact name FIRST and follow the loaded guidance — do not guess names, do not skip a matching skill, and do not substitute your own approach for its instructions.\n")
|
||||
listed, spent, omitted := 0, 0, 0
|
||||
for _, info := range options.Skills {
|
||||
name := strings.TrimSpace(info.Name)
|
||||
@@ -232,9 +241,11 @@ func skillsContext(options Options) string {
|
||||
}
|
||||
|
||||
// truncateForSkillLine keeps a skill's one-line description short so a single
|
||||
// verbose description can't dominate the skills-list budget.
|
||||
// verbose description can't dominate the skills-list budget. The cap is roomy on
|
||||
// purpose: the description carries the skill's trigger conditions ("use when…"),
|
||||
// and truncating those is what stops the model from ever invoking the skill.
|
||||
func truncateForSkillLine(desc string) string {
|
||||
const maxDescRunes = 100
|
||||
const maxDescRunes = 200
|
||||
runes := []rune(desc)
|
||||
if len(runes) <= maxDescRunes {
|
||||
return desc
|
||||
|
||||
@@ -285,13 +285,11 @@ type Options struct {
|
||||
// ceiling and the autonomy gate. nil disables it entirely (the loop is
|
||||
// byte-identical to before). One instance per run — it holds attempt state.
|
||||
SelfCorrect *SelfCorrector
|
||||
// FileDiagnostics, when set, checks files changed by mutating tools for
|
||||
// error-severity language diagnostics IN THE BACKGROUND and appends any
|
||||
// errors as a nudge before the model's next request — the model still sees
|
||||
// an error it introduced at its next decision point, but no tool call ever
|
||||
// blocks on the language server (the old inline path stalled every edit on
|
||||
// a ≥300ms debounce, 10s cap). Build one with NewFileDiagnostics. nil
|
||||
// disables post-edit diagnostics.
|
||||
// FileDiagnostics, when set, is passed to mutating tools so edit_file /
|
||||
// write_file append error-severity language diagnostics for the file they
|
||||
// just wrote directly to their tool output — the model sees an error it
|
||||
// introduced in the same turn instead of a later verification pass. Build
|
||||
// one with NewFileDiagnostics. nil disables inline diagnostics.
|
||||
FileDiagnostics func(ctx context.Context, absPath string) string
|
||||
|
||||
// RequireCompletionSignal gates run completion for HEADLESS exec. Without it,
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/agent"
|
||||
@@ -782,6 +783,14 @@ func runInteractiveTUIWithSetup(stderr io.Writer, deps appDeps, permissionMode a
|
||||
Specialists: specialistRuntime.specialists,
|
||||
Skills: pluginActivation.skillInfos(deps.skillsDir()),
|
||||
},
|
||||
// LoadSkills backs /skills and direct /<skill-name> invocation in the TUI.
|
||||
// It resolves against the same merged set (default dir + plugin skill
|
||||
// roots) as the skill tool and the system-prompt list, re-read per use so
|
||||
// newly installed skills work without a restart.
|
||||
LoadSkills: cachedSkillsLoader(func() []skills.Skill {
|
||||
merged, _ := plugins.MergedSkillsLoaded(deps.skillsDir(), pluginActivation.skillRoots)
|
||||
return merged
|
||||
}),
|
||||
PermissionMode: permissionMode,
|
||||
Notify: resolved.Notify,
|
||||
KeyBindings: resolved.KeyBindings,
|
||||
@@ -1277,3 +1286,26 @@ Flags:
|
||||
`)
|
||||
return err
|
||||
}
|
||||
|
||||
// cachedSkillsLoader memoizes a skills loader for a short interval. The TUI
|
||||
// calls the loader from autocomplete on every "/x" keystroke; without a cache
|
||||
// each keystroke would re-read every SKILL.md body. Two seconds is fresh enough
|
||||
// that a newly installed skill still shows up "immediately" while typing.
|
||||
func cachedSkillsLoader(load func() []skills.Skill) func() []skills.Skill {
|
||||
var (
|
||||
mu sync.Mutex
|
||||
at time.Time
|
||||
cached []skills.Skill
|
||||
)
|
||||
const ttl = 2 * time.Second
|
||||
return func() []skills.Skill {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
if cached != nil && time.Since(at) < ttl {
|
||||
return cached
|
||||
}
|
||||
cached = load()
|
||||
at = time.Now()
|
||||
return cached
|
||||
}
|
||||
}
|
||||
@@ -711,10 +711,9 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
|
||||
// sessions); it is a no-op when self-correct is off.
|
||||
func newExecSelfCorrector(enabled bool, workspaceRoot string, autonomy string) (*agent.SelfCorrector, func(context.Context, string) string, func()) {
|
||||
// The manager is created regardless of --self-correct: it also backs the
|
||||
// always-on background post-edit diagnostics (agent.NewFileDiagnostics,
|
||||
// collected off the tool-call path by the loop), and it stays lazy — no
|
||||
// language server is spawned unless an edited file's language actually has
|
||||
// one installed on PATH.
|
||||
// always-on inline post-edit diagnostics (agent.NewFileDiagnostics), and it
|
||||
// stays lazy — no language server is spawned unless an edited file's
|
||||
// language actually has one installed on PATH.
|
||||
manager := lsp.NewManager(workspaceRoot)
|
||||
cleanup := func() {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
|
||||
+8
-67
@@ -126,8 +126,8 @@ func (tool bashTool) run(ctx context.Context, args map[string]any, engine *zeroS
|
||||
// can't grow Zero's memory before truncation: only the head+tail each stream
|
||||
// will ever surface to the model are retained, the middle is discarded as it
|
||||
// streams, and the true size is counted for the truncation marker.
|
||||
stdout := newBoundedBuffer(bashCaptureBudgetBytes, bashCaptureBudgetBytes)
|
||||
stderr := newBoundedBuffer(bashCaptureBudgetBytes, bashCaptureBudgetBytes)
|
||||
stdout := newBoundedBuffer(bashOutputBudgetBytes, bashOutputBudgetBytes)
|
||||
stderr := newBoundedBuffer(bashOutputBudgetBytes, bashOutputBudgetBytes)
|
||||
command.Stdout = stdout
|
||||
command.Stderr = stderr
|
||||
|
||||
@@ -370,21 +370,12 @@ func formatBashOutput(stdout string, stderr string, exitCode int) string {
|
||||
return strings.Join(parts, "\n")
|
||||
}
|
||||
|
||||
// bashOutputBudgetBytes caps each of stdout/stderr shown to the model (32 KiB
|
||||
// ≈ 8k tokens per stream). bash is the one tool that can emit unbounded output
|
||||
// (`cat large.log`, `find /`, verbose test runs); a chatty build log at the old
|
||||
// 96 KiB budget injected ~24k tokens per call and was then re-billed on every
|
||||
// following turn. Head+tail truncation keeps both the start and the end of an
|
||||
// oversized stream, since build/test failures usually surface at the tail, and
|
||||
// the full captured text is spilled to disk so nothing is lost — the model
|
||||
// greps/reads the spill instead of re-running the command.
|
||||
const bashOutputBudgetBytes = 32 * 1024
|
||||
|
||||
// bashCaptureBudgetBytes bounds the head and tail retained IN MEMORY per
|
||||
// stream during capture. Deliberately larger than the emit budget: the extra
|
||||
// retained bytes never reach the transcript, but they make the spill file —
|
||||
// the model's recovery path — cover 6× more of the output.
|
||||
const bashCaptureBudgetBytes = 96 * 1024
|
||||
// bashOutputBudgetBytes caps each of stdout/stderr shown to the model. bash is the
|
||||
// one tool that can emit unbounded output (`cat large.log`, `find /`, verbose test
|
||||
// runs); every other read/search tool already budgets its output. Head+tail
|
||||
// truncation keeps both the start and the end of an oversized stream, since
|
||||
// build/test failures usually surface at the tail.
|
||||
const bashOutputBudgetBytes = 96 * 1024
|
||||
|
||||
// budgetBashOutput truncates stdout and stderr to bashOutputBudgetBytes each,
|
||||
// keeping the head and tail of anything larger, and records raw/emitted byte
|
||||
@@ -404,19 +395,6 @@ func budgetBashCapture(out string, outTotal int, errStr string, errTotal int, me
|
||||
outText, outRaw, outTrunc := truncateHeadTailWithTotal(out, outTotal, bashOutputBudgetBytes)
|
||||
errText, errRaw, errTrunc := truncateHeadTailWithTotal(errStr, errTotal, bashOutputBudgetBytes)
|
||||
truncated := outTrunc || errTrunc
|
||||
if truncated {
|
||||
if spillPath := spillBashStreams(out, outTotal, errStr, errTotal); spillPath != "" {
|
||||
hint := "\n[zero] captured output saved to " + spillPath + " (grep or read_file it instead of re-running)"
|
||||
if errTrunc {
|
||||
errText += hint
|
||||
} else {
|
||||
outText += hint
|
||||
}
|
||||
if meta != nil {
|
||||
meta["spill_path"] = spillPath
|
||||
}
|
||||
}
|
||||
}
|
||||
if meta != nil {
|
||||
emitted := len(outText) + len(errText)
|
||||
meta["raw_bytes"] = strconv.Itoa(outRaw + errRaw)
|
||||
@@ -429,43 +407,6 @@ func budgetBashCapture(out string, outTotal int, errStr string, errTotal int, me
|
||||
return outText, errText, truncated
|
||||
}
|
||||
|
||||
// spillBashStreams writes the retained (capture-bounded) stdout and stderr to
|
||||
// the spill directory as one sectioned file and returns its path, or "" when
|
||||
// spilling fails. The spill holds up to bashCaptureBudgetBytes of head and
|
||||
// tail per stream — everything zero kept in memory, which is more than the
|
||||
// emit budget shows the model but not necessarily the whole output. When the
|
||||
// capture itself dropped the middle of a stream (total exceeds the retained
|
||||
// bytes), a gap marker is inserted at the head/tail junction so the spilled
|
||||
// log never reads as contiguous when it is not.
|
||||
func spillBashStreams(stdout string, stdoutTotal int, stderr string, stderrTotal int) string {
|
||||
stdout = sectionWithCaptureGap(stdout, stdoutTotal)
|
||||
stderr = sectionWithCaptureGap(stderr, stderrTotal)
|
||||
var combined strings.Builder
|
||||
combined.Grow(len(stdout) + len(stderr) + 64)
|
||||
combined.WriteString("### stdout\n")
|
||||
combined.WriteString(stdout)
|
||||
if !strings.HasSuffix(stdout, "\n") {
|
||||
combined.WriteString("\n")
|
||||
}
|
||||
combined.WriteString("### stderr\n")
|
||||
combined.WriteString(stderr)
|
||||
return spillTruncatedOutput("bash", combined.String())
|
||||
}
|
||||
|
||||
// sectionWithCaptureGap marks the point where boundedBuffer dropped the middle
|
||||
// of a stream. When total exceeds the retained bytes, the retained text is the
|
||||
// frozen head (bashCaptureBudgetBytes, always full once overflow happened)
|
||||
// followed immediately by the rolling tail — the junction sits at the head cap,
|
||||
// snapped back to a rune boundary.
|
||||
func sectionWithCaptureGap(text string, total int) string {
|
||||
if total <= len(text) || len(text) <= bashCaptureBudgetBytes {
|
||||
return text
|
||||
}
|
||||
head := utf8Prefix(text, bashCaptureBudgetBytes)
|
||||
marker := fmt.Sprintf("\n[zero] capture gap: %d bytes omitted from the middle of this stream\n", total-len(text))
|
||||
return head + marker + text[len(head):]
|
||||
}
|
||||
|
||||
// boundedBuffer is an io.Writer that retains at most headCap bytes from the start
|
||||
// and tailCap bytes from the end of a stream while counting the total written, so
|
||||
// a command emitting unbounded output (`cat huge.log`, `yes`) cannot grow Zero's
|
||||
|
||||
@@ -2,23 +2,11 @@ package tools
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// setTestTempDir points every platform's temp-dir lookup at a per-test
|
||||
// directory so spill files never land in the real temp dir: os.TempDir reads
|
||||
// TMPDIR on Unix but TMP/TEMP on Windows.
|
||||
func setTestTempDir(t *testing.T) {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
t.Setenv("TMPDIR", dir)
|
||||
t.Setenv("TMP", dir)
|
||||
t.Setenv("TEMP", dir)
|
||||
}
|
||||
|
||||
// Small output passes through untouched and records raw==emitted, no truncated flag.
|
||||
func TestBudgetBashOutputSmallPassesThrough(t *testing.T) {
|
||||
meta := map[string]string{}
|
||||
@@ -38,10 +26,8 @@ func TestBudgetBashOutputSmallPassesThrough(t *testing.T) {
|
||||
}
|
||||
|
||||
// Oversized stdout is truncated head+tail: both the first and last lines survive,
|
||||
// the middle is dropped behind a marker, meta is flagged, and the captured text
|
||||
// is spilled to a re-readable file.
|
||||
// the middle is dropped behind a marker, and meta is flagged.
|
||||
func TestBudgetBashOutputTruncatesHeadAndTail(t *testing.T) {
|
||||
setTestTempDir(t)
|
||||
head := "FIRST_LINE_MARKER\n"
|
||||
tail := "\nLAST_LINE_MARKER"
|
||||
big := head + strings.Repeat("x", bashOutputBudgetBytes) + tail
|
||||
@@ -61,23 +47,9 @@ func TestBudgetBashOutputTruncatesHeadAndTail(t *testing.T) {
|
||||
if !strings.Contains(out, "output truncated") {
|
||||
t.Fatalf("expected a truncation marker, got:\n%s", out[:200])
|
||||
}
|
||||
// The spill hint (path length varies) is the only allowed overage past the budget.
|
||||
if len(out) > bashOutputBudgetBytes+512 {
|
||||
if len(out) > bashOutputBudgetBytes {
|
||||
t.Fatalf("emitted %d bytes exceeds budget %d", len(out), bashOutputBudgetBytes)
|
||||
}
|
||||
if !strings.Contains(out, "captured output saved to ") {
|
||||
t.Fatalf("truncated bash output must carry a spill hint:\n%s", out[len(out)-300:])
|
||||
}
|
||||
if meta["spill_path"] == "" {
|
||||
t.Fatal("spill path missing from meta")
|
||||
}
|
||||
content, err := os.ReadFile(meta["spill_path"])
|
||||
if err != nil {
|
||||
t.Fatalf("spill file unreadable: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(content), "### stdout") || !strings.Contains(string(content), strings.Repeat("x", 1024)) {
|
||||
t.Fatalf("spill must hold the sectioned captured streams (got %d bytes)", len(content))
|
||||
}
|
||||
if meta["truncated"] != "true" {
|
||||
t.Fatalf("expected truncated=true, got %v", meta)
|
||||
}
|
||||
@@ -128,12 +100,10 @@ func TestBoundedBufferKeepsHeadAndTailBounded(t *testing.T) {
|
||||
// budgetBashCapture reports the TRUE total (not the retained size) in the marker
|
||||
// and raw_bytes, even though only a bounded head+tail was ever held in memory.
|
||||
func TestBudgetBashCaptureReportsTrueTotal(t *testing.T) {
|
||||
setTestTempDir(t)
|
||||
// Retained head+tail as boundedBuffer would hand over after overflow: the
|
||||
// frozen head is full at bashCaptureBudgetBytes, then the rolling tail; the
|
||||
// real command produced far more than was kept.
|
||||
retained := "HEAD_START" + strings.Repeat("y", bashCaptureBudgetBytes) + "TAIL_END"
|
||||
total := 10 * bashCaptureBudgetBytes
|
||||
// Retained head+tail as boundedBuffer would hand over; the real command produced
|
||||
// far more than was kept.
|
||||
retained := "HEAD_START" + strings.Repeat("y", bashOutputBudgetBytes) + "TAIL_END"
|
||||
total := 10 * bashOutputBudgetBytes
|
||||
|
||||
meta := map[string]string{}
|
||||
out, _, truncated := budgetBashCapture(retained, total, "", 0, meta)
|
||||
@@ -144,8 +114,7 @@ func TestBudgetBashCaptureReportsTrueTotal(t *testing.T) {
|
||||
if !strings.Contains(out, "HEAD_START") || !strings.Contains(out, "TAIL_END") {
|
||||
t.Fatalf("head/tail lost after budgeting:\n%s", out[:min(120, len(out))])
|
||||
}
|
||||
// Spill hint is the only allowed overage past the budget.
|
||||
if len(out) > bashOutputBudgetBytes+512 {
|
||||
if len(out) > bashOutputBudgetBytes {
|
||||
t.Fatalf("emitted %d bytes exceeds budget %d", len(out), bashOutputBudgetBytes)
|
||||
}
|
||||
if meta["raw_bytes"] != strconv.Itoa(total) {
|
||||
@@ -155,29 +124,4 @@ func TestBudgetBashCaptureReportsTrueTotal(t *testing.T) {
|
||||
if !strings.Contains(out, strconv.Itoa(total-bashOutputBudgetBytes)) {
|
||||
t.Fatalf("marker should cite the true omitted byte count:\n%s", out[:min(200, len(out))])
|
||||
}
|
||||
|
||||
// The spill must be recorded, sectioned, and carry a capture-gap marker at
|
||||
// the head/tail junction — the capture dropped the middle, so the spilled
|
||||
// log must never read as contiguous.
|
||||
if meta["spill_path"] == "" {
|
||||
t.Fatal("spill path missing from meta")
|
||||
}
|
||||
content, err := os.ReadFile(meta["spill_path"])
|
||||
if err != nil {
|
||||
t.Fatalf("spill file unreadable: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(content), "### stdout") {
|
||||
t.Fatal("spill must be sectioned by stream")
|
||||
}
|
||||
gapMarker := fmt.Sprintf("capture gap: %d bytes omitted", total-len(retained))
|
||||
gapIndex := strings.Index(string(content), gapMarker)
|
||||
if gapIndex < 0 {
|
||||
t.Fatalf("spill missing capture-gap marker %q", gapMarker)
|
||||
}
|
||||
// The junction sits right after the frozen head (plus the section header),
|
||||
// not appended at the end of the stream.
|
||||
headerOffset := len("### stdout\n")
|
||||
if gapIndex < headerOffset+bashCaptureBudgetBytes-64 || gapIndex > headerOffset+bashCaptureBudgetBytes+64 {
|
||||
t.Fatalf("capture-gap marker at offset %d, want ~%d (the head cap)", gapIndex, headerOffset+bashCaptureBudgetBytes)
|
||||
}
|
||||
}
|
||||
@@ -1,81 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Universal output ceiling. Every tool result leaving the registry boundary is
|
||||
// capped at a single token-denominated budget unless the tool manages its own
|
||||
// deliberate budget (selfBudgeting below). This is the safety net for tools
|
||||
// with no cap of their own — web_fetch bodies, skill files, MCP server tools,
|
||||
// browser snapshots, anything added later — so no single call can flood the
|
||||
// context window with output that is then re-billed on every following turn.
|
||||
// Truncation keeps head+tail and spills the full text to disk (re-readable via
|
||||
// grep/read_file), so nothing is lost, only deferred.
|
||||
|
||||
// defaultOutputCeilingTokens is the per-call ceiling in estimated tokens
|
||||
// (bytes/4). 16k tokens = 64 KiB — deliberately equal to the search-tool
|
||||
// budget: generous enough for a large fetched document, small enough that one
|
||||
// call cannot eat a third of a small context window.
|
||||
const defaultOutputCeilingTokens = 16_000
|
||||
|
||||
// outputCeilingEnv overrides the ceiling (in tokens). Zero or negative
|
||||
// disables the ceiling entirely; unset or unparsable keeps the default.
|
||||
const outputCeilingEnv = "ZERO_TOOL_OUTPUT_CEILING_TOKENS"
|
||||
|
||||
// selfBudgeting marks a tool that enforces its own deliberate output budget —
|
||||
// possibly model-raisable (exec_command) — which the registry ceiling must not
|
||||
// second-guess. The method is unexported on purpose: only tools in this
|
||||
// package can opt out, so an MCP-served tool can never exempt itself.
|
||||
type selfBudgeting interface{ managesOutputBudget() }
|
||||
|
||||
// The exemption list, kept in one place. Each of these applies its own budget
|
||||
// before returning: bash (bashOutputBudgetBytes per stream + spill),
|
||||
// exec_command (model-raisable token budget + spill), read tools (128 KiB),
|
||||
// search tools (64 KiB).
|
||||
func (bashTool) managesOutputBudget() {}
|
||||
func (execCommandTool) managesOutputBudget() {}
|
||||
func (readFileTool) managesOutputBudget() {}
|
||||
func (readMinifiedFileTool) managesOutputBudget() {}
|
||||
func (grepTool) managesOutputBudget() {}
|
||||
func (globTool) managesOutputBudget() {}
|
||||
func (listDirectoryTool) managesOutputBudget() {}
|
||||
|
||||
func resolveOutputCeilingTokens() int {
|
||||
raw := strings.TrimSpace(os.Getenv(outputCeilingEnv))
|
||||
if raw == "" {
|
||||
return defaultOutputCeilingTokens
|
||||
}
|
||||
parsed, err := strconv.Atoi(raw)
|
||||
if err != nil {
|
||||
return defaultOutputCeilingTokens
|
||||
}
|
||||
return parsed
|
||||
}
|
||||
|
||||
// enforceOutputCeiling truncates an over-ceiling result to head+tail within
|
||||
// the token budget, spilling the full (already redaction-scrubbed) output to
|
||||
// disk with a recovery hint. Runs after scrubResultSecrets so the transcript
|
||||
// and the spill file agree on what was hidden.
|
||||
func enforceOutputCeiling(toolName string, result Result) Result {
|
||||
ceiling := resolveOutputCeilingTokens()
|
||||
if ceiling <= 0 {
|
||||
return result
|
||||
}
|
||||
if len(result.Output) <= ceiling*4 {
|
||||
return result
|
||||
}
|
||||
rawBytes := len(result.Output)
|
||||
truncated, _ := truncateExecOutputSpill(result.Output, ceiling, toolName)
|
||||
result.Output = truncated
|
||||
result.Truncated = true
|
||||
if result.Meta == nil {
|
||||
result.Meta = map[string]string{}
|
||||
}
|
||||
result.Meta["raw_bytes"] = strconv.Itoa(rawBytes)
|
||||
result.Meta["emitted_bytes"] = strconv.Itoa(len(result.Output))
|
||||
result.Meta["estimated_tokens"] = strconv.Itoa(estimatedTokensFromBytes(len(result.Output)))
|
||||
return result
|
||||
}
|
||||
@@ -1,140 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// ceilingFakeTool is a minimal tool with no output budget of its own — the
|
||||
// stand-in for an MCP-served tool or any future builtin that forgets to cap
|
||||
// its output.
|
||||
type ceilingFakeTool struct {
|
||||
baseTool
|
||||
output string
|
||||
}
|
||||
|
||||
func (tool ceilingFakeTool) Run(context.Context, map[string]any) Result {
|
||||
return okResult(tool.output)
|
||||
}
|
||||
|
||||
// ceilingExemptTool is the same fake but marked self-budgeting.
|
||||
type ceilingExemptTool struct{ ceilingFakeTool }
|
||||
|
||||
func (ceilingExemptTool) managesOutputBudget() {}
|
||||
|
||||
func newCeilingFakeTool(name, output string) ceilingFakeTool {
|
||||
return ceilingFakeTool{
|
||||
baseTool: baseTool{name: name, safety: readOnlySafety("test tool")},
|
||||
output: output,
|
||||
}
|
||||
}
|
||||
|
||||
// An unbudgeted tool's oversized output is capped at the universal ceiling,
|
||||
// head and tail survive, and the full output is spilled to a re-readable file.
|
||||
func TestOutputCeilingCapsUnbudgetedTool(t *testing.T) {
|
||||
setTestTempDir(t)
|
||||
big := "HEAD_MARK\n" + strings.Repeat("z", defaultOutputCeilingTokens*4*3) + "\nTAIL_MARK"
|
||||
|
||||
registry := NewRegistry()
|
||||
registry.Register(newCeilingFakeTool("fake_big", big))
|
||||
result := registry.Run(context.Background(), "fake_big", map[string]any{})
|
||||
|
||||
if !result.Truncated {
|
||||
t.Fatal("over-ceiling output must report truncated=true")
|
||||
}
|
||||
maxBytes := defaultOutputCeilingTokens * 4
|
||||
if len(result.Output) > maxBytes+512 {
|
||||
t.Fatalf("emitted %d bytes, ceiling is %d", len(result.Output), maxBytes)
|
||||
}
|
||||
if !strings.Contains(result.Output, "HEAD_MARK") || !strings.Contains(result.Output, "TAIL_MARK") {
|
||||
t.Fatal("head/tail lost at the ceiling")
|
||||
}
|
||||
if result.Meta["raw_bytes"] != strconv.Itoa(len(big)) {
|
||||
t.Fatalf("raw_bytes = %s, want %d", result.Meta["raw_bytes"], len(big))
|
||||
}
|
||||
if !strings.Contains(result.Output, "full output saved to ") {
|
||||
t.Fatalf("ceiling truncation must include a spill hint: %q", result.Output[:200])
|
||||
}
|
||||
start := strings.Index(result.Output, "full output saved to ") + len("full output saved to ")
|
||||
end := strings.Index(result.Output[start:], " (grep")
|
||||
content, err := os.ReadFile(result.Output[start : start+end])
|
||||
if err != nil {
|
||||
t.Fatalf("spill file unreadable: %v", err)
|
||||
}
|
||||
if string(content) != big {
|
||||
t.Fatalf("spill must hold the full output: got %d bytes, want %d", len(content), len(big))
|
||||
}
|
||||
}
|
||||
|
||||
// A self-budgeting tool's output passes the boundary untouched even when it
|
||||
// exceeds the universal ceiling — its own (possibly model-raised) budget wins.
|
||||
func TestOutputCeilingSkipsSelfBudgetingTool(t *testing.T) {
|
||||
big := strings.Repeat("z", defaultOutputCeilingTokens*4*2)
|
||||
registry := NewRegistry()
|
||||
registry.Register(ceilingExemptTool{newCeilingFakeTool("fake_exempt", big)})
|
||||
|
||||
result := registry.Run(context.Background(), "fake_exempt", map[string]any{})
|
||||
if result.Truncated || result.Output != big {
|
||||
t.Fatalf("self-budgeting tool must bypass the ceiling (truncated=%v, %d bytes)", result.Truncated, len(result.Output))
|
||||
}
|
||||
}
|
||||
|
||||
// Under-ceiling output is untouched and gains no budget metadata.
|
||||
func TestOutputCeilingSmallOutputUntouched(t *testing.T) {
|
||||
registry := NewRegistry()
|
||||
registry.Register(newCeilingFakeTool("fake_small", "hello"))
|
||||
|
||||
result := registry.Run(context.Background(), "fake_small", map[string]any{})
|
||||
if result.Truncated || result.Output != "hello" {
|
||||
t.Fatalf("small output altered: %+v", result)
|
||||
}
|
||||
if _, ok := result.Meta["raw_bytes"]; ok {
|
||||
t.Fatal("small output must not gain budget metadata")
|
||||
}
|
||||
}
|
||||
|
||||
// ZERO_TOOL_OUTPUT_CEILING_TOKENS tightens, loosens, or disables the ceiling.
|
||||
func TestOutputCeilingEnvOverride(t *testing.T) {
|
||||
setTestTempDir(t)
|
||||
big := strings.Repeat("z", 100*1024)
|
||||
|
||||
t.Setenv(outputCeilingEnv, "1000")
|
||||
registry := NewRegistry()
|
||||
registry.Register(newCeilingFakeTool("fake_env", big))
|
||||
result := registry.Run(context.Background(), "fake_env", map[string]any{})
|
||||
if !result.Truncated || len(result.Output) > 1000*4+512 {
|
||||
t.Fatalf("ceiling override ignored: truncated=%v, %d bytes", result.Truncated, len(result.Output))
|
||||
}
|
||||
|
||||
t.Setenv(outputCeilingEnv, "0")
|
||||
result = registry.Run(context.Background(), "fake_env", map[string]any{})
|
||||
if result.Truncated || len(result.Output) != len(big) {
|
||||
t.Fatalf("ceiling=0 must disable the net: truncated=%v, %d bytes", result.Truncated, len(result.Output))
|
||||
}
|
||||
}
|
||||
|
||||
// The exemption list matches the tools that really do budget themselves, and
|
||||
// the unbudgeted ones (web_fetch, skill, browser) stay under the net.
|
||||
func TestSelfBudgetingExemptionList(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
exempt := []Tool{
|
||||
NewBashTool(dir),
|
||||
NewExecCommandTool(dir, newExecSessionManager()),
|
||||
NewReadFileTool(dir),
|
||||
NewReadMinifiedFileTool(dir),
|
||||
NewGrepTool(dir),
|
||||
NewGlobTool(dir),
|
||||
NewListDirectoryTool(dir),
|
||||
}
|
||||
for _, tool := range exempt {
|
||||
if _, ok := tool.(selfBudgeting); !ok {
|
||||
t.Errorf("%s must be exempt from the output ceiling", tool.Name())
|
||||
}
|
||||
}
|
||||
if _, ok := NewWebFetchTool().(selfBudgeting); ok {
|
||||
t.Error("web_fetch must NOT be exempt — the ceiling is its only budget")
|
||||
}
|
||||
}
|
||||
@@ -123,24 +123,13 @@ func (registry *Registry) Run(ctx context.Context, name string, args map[string]
|
||||
func (registry *Registry) RunWithOptions(ctx context.Context, name string, args map[string]any, options RunOptions) (result Result) {
|
||||
// Every return path passes through scrubResultSecrets exactly once, so denial,
|
||||
// permission, and unknown-tool error messages (which can echo secret-bearing
|
||||
// args/paths) are redacted at the boundary just like tool output. The output
|
||||
// ceiling runs after the scrub so the transcript and the spill file agree on
|
||||
// what was hidden.
|
||||
ceilingExempt := false
|
||||
defer func() {
|
||||
result = scrubResultSecrets(result)
|
||||
if !ceilingExempt {
|
||||
result = enforceOutputCeiling(name, result)
|
||||
}
|
||||
}()
|
||||
// args/paths) are redacted at the boundary just like tool output.
|
||||
defer func() { result = scrubResultSecrets(result) }()
|
||||
|
||||
tool, ok := registry.Get(name)
|
||||
if !ok {
|
||||
return errorResult(`Error: Unknown tool "` + name + `".`)
|
||||
}
|
||||
if _, ok := tool.(selfBudgeting); ok {
|
||||
ceilingExempt = true
|
||||
}
|
||||
if rejecter, ok := tool.(PrePermissionRejecter); ok {
|
||||
if res, rejected := rejecter.RejectBeforePermission(args); rejected {
|
||||
return res
|
||||
|
||||
@@ -8,7 +8,6 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/redaction"
|
||||
"github.com/Gitlawb/zero/internal/secrets"
|
||||
)
|
||||
|
||||
// Spill-to-disk for truncated tool output. When a command produces more than
|
||||
@@ -91,10 +90,8 @@ func spillDir() (string, error) {
|
||||
// spillTruncatedOutput writes the full pre-truncation output to the spill
|
||||
// directory and returns the file path, or "" when spilling fails. Output is
|
||||
// scrubbed with the same configured-key redaction the registry applies at the
|
||||
// tool boundary PLUS the pattern-based secret scanner (AWS keys, tokens, PEM
|
||||
// blocks, JWTs) that bash applies to its model-visible output — a spill runs
|
||||
// before that formatter, so without the scan here a spilled file would hold
|
||||
// pattern-matched credentials in cleartext that the transcript hides.
|
||||
// tool boundary, so a spilled file never holds a secret the transcript would
|
||||
// have hidden.
|
||||
func spillTruncatedOutput(toolName, output string) string {
|
||||
dir, err := spillDir()
|
||||
if err != nil {
|
||||
@@ -113,7 +110,6 @@ func spillTruncatedOutput(toolName, output string) string {
|
||||
}
|
||||
defer file.Close()
|
||||
scrubbed := redaction.RedactString(output, redaction.Options{})
|
||||
scrubbed, _ = secrets.Redact(scrubbed)
|
||||
if _, err := file.WriteString(scrubbed); err != nil {
|
||||
_ = os.Remove(file.Name())
|
||||
return ""
|
||||
|
||||
@@ -63,33 +63,6 @@ func TestSweepSpillDirRemovesOnlyOldFiles(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A spill happens BEFORE bash's model-facing formatter runs its pattern-based
|
||||
// secret scan, so the spill itself must scrub pattern-matched credentials —
|
||||
// otherwise the file would hold in cleartext exactly what the transcript hides.
|
||||
func TestSpillTruncatedOutputScrubsPatternSecrets(t *testing.T) {
|
||||
setTestTempDir(t)
|
||||
githubToken := "ghp_" + strings.Repeat("a", 36)
|
||||
body := "before\nAKIAIOSFODNN7EXAMPLE\n" + githubToken + "\nafter"
|
||||
|
||||
path := spillTruncatedOutput("bash", body)
|
||||
if path == "" {
|
||||
t.Fatal("spill must return a file path")
|
||||
}
|
||||
content, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Contains(string(content), "AKIAIOSFODNN7EXAMPLE") {
|
||||
t.Fatal("AWS access key reached the spill file in cleartext")
|
||||
}
|
||||
if strings.Contains(string(content), githubToken) {
|
||||
t.Fatal("GitHub token reached the spill file in cleartext")
|
||||
}
|
||||
if !strings.Contains(string(content), "before") || !strings.Contains(string(content), "after") {
|
||||
t.Fatalf("non-secret content must survive the scrub: %q", content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpillTruncatedOutputWritesFile(t *testing.T) {
|
||||
t.Setenv("TMPDIR", t.TempDir())
|
||||
path := spillTruncatedOutput("exec_command", "some output body")
|
||||
|
||||
@@ -188,7 +188,54 @@ func (m model) matchCommandSuggestions(token string) []commandSuggestion {
|
||||
out := matchCommandSuggestionsWithFilter(token, func(command commandDefinition) bool {
|
||||
return command.kind != commandSandboxSetup || m.sandboxSetupCommand != nil
|
||||
})
|
||||
return append(out, m.matchUserCommandSuggestions(token)...)
|
||||
out = append(out, m.matchUserCommandSuggestions(token)...)
|
||||
out = append(out, m.matchSkillSuggestions(token)...)
|
||||
// Each source caps itself, but the merged list must honor the shared cap too
|
||||
// (three sources could otherwise stack up to ~3x the palette bound).
|
||||
if len(out) > maxCommandSuggestions {
|
||||
out = out[:maxCommandSuggestions]
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// matchSkillSuggestions returns installed skills whose slash name has the typed
|
||||
// prefix, so skills are discoverable and invocable from the palette like user
|
||||
// commands. Precedence (builtin > user command > skill) is enforced here by
|
||||
// skipping any skill whose name is claimed by a builtin (or alias) or a user
|
||||
// command — unlike the builtin/user pair, a shadowed skill row would be dead at
|
||||
// dispatch time, so it must not be advertised at all.
|
||||
func (m model) matchSkillSuggestions(token string) []commandSuggestion {
|
||||
prefix := strings.ToLower(strings.TrimSpace(strings.TrimPrefix(token, "/")))
|
||||
if prefix == "" || m.loadSkills == nil {
|
||||
return nil
|
||||
}
|
||||
taken := m.takenSlashNames()
|
||||
var out []commandSuggestion
|
||||
// Read through the loader (not the startup snapshot in agentOptions) so the
|
||||
// palette matches what dispatch will actually resolve — a skill installed or
|
||||
// removed mid-session appears/disappears without a restart.
|
||||
for _, skill := range m.installedSkills() {
|
||||
name := skillSlashName(skill.Name)
|
||||
if name == "" || taken[name] || !strings.HasPrefix(name, prefix) {
|
||||
continue
|
||||
}
|
||||
// Two skills whose names collide after lowercasing (e.g. "Deploy" and
|
||||
// "deploy") map to one slash name; only the first — the one dispatch will
|
||||
// run — may be advertised, or the row's description and the executed
|
||||
// instructions silently diverge.
|
||||
taken[name] = true
|
||||
desc := strings.TrimSpace(skill.Description)
|
||||
if desc == "" {
|
||||
desc = "Skill: /" + name
|
||||
} else {
|
||||
desc += " (skill)"
|
||||
}
|
||||
out = append(out, commandSuggestion{Name: "/" + name, Desc: desc})
|
||||
if len(out) >= maxCommandSuggestions {
|
||||
break
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// matchUserCommandSuggestions returns file-sourced /commands whose name has the
|
||||
|
||||
@@ -698,3 +698,19 @@ func (m model) debugText() string {
|
||||
}},
|
||||
})
|
||||
}
|
||||
|
||||
// skillsText is the /skills fallback when NO skills are installed — an install
|
||||
// hint. With skills present /skills opens the searchable skill picker instead
|
||||
// (see newSkillPicker), matching how /model works.
|
||||
func (m model) skillsText() string {
|
||||
return renderCommandOutput(commandOutput{
|
||||
Title: "Skills",
|
||||
Status: commandStatusInfo,
|
||||
Sections: []commandSection{{
|
||||
Lines: []string{"No skills installed."},
|
||||
}},
|
||||
Hints: []string{
|
||||
"install one: create <skills-dir>/<name>/SKILL.md (see `zero skills`)",
|
||||
},
|
||||
})
|
||||
}
|
||||
@@ -46,6 +46,7 @@ const (
|
||||
commandCopy
|
||||
commandExport
|
||||
commandNew
|
||||
commandSkills
|
||||
commandUnknown
|
||||
)
|
||||
|
||||
@@ -131,6 +132,13 @@ var commandDefinitions = []commandDefinition{
|
||||
description: "List registered tools.",
|
||||
kind: commandTools,
|
||||
},
|
||||
{
|
||||
name: "/skills",
|
||||
usage: "/skills",
|
||||
group: commandGroupTools,
|
||||
description: "List installed skills; run one directly with /<skill-name> [args].",
|
||||
kind: commandSkills,
|
||||
},
|
||||
{
|
||||
name: "/context",
|
||||
usage: "/context",
|
||||
|
||||
+25
-3
@@ -29,6 +29,7 @@ import (
|
||||
"github.com/Gitlawb/zero/internal/providers/providerio"
|
||||
"github.com/Gitlawb/zero/internal/sandbox"
|
||||
"github.com/Gitlawb/zero/internal/sessions"
|
||||
"github.com/Gitlawb/zero/internal/skills"
|
||||
"github.com/Gitlawb/zero/internal/streamjson"
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/usage"
|
||||
@@ -62,6 +63,7 @@ type model struct {
|
||||
ctx context.Context
|
||||
cwd string
|
||||
userCommands []usercommands.Command // file-sourced /commands (.zero/commands)
|
||||
loadSkills func() []skills.Skill // lazy installed-skills loader for /skills + /<skill-name>
|
||||
userConfigPath string
|
||||
doctorUserConfigPath string
|
||||
projectConfigPath string
|
||||
@@ -735,6 +737,7 @@ func newModel(ctx context.Context, options Options) model {
|
||||
cwd: cwd,
|
||||
swarmDoneAt: map[string]time.Time{},
|
||||
userCommands: loadedUserCommands,
|
||||
loadSkills: options.LoadSkills,
|
||||
composerCursorVisible: true,
|
||||
userConfigPath: options.UserConfigPath,
|
||||
doctorUserConfigPath: doctorUserConfigPath,
|
||||
@@ -3730,6 +3733,11 @@ func (m model) choosePicker() (tea.Model, tea.Cmd) {
|
||||
if text != "" {
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text})
|
||||
}
|
||||
case pickerSkill:
|
||||
// Fill the composer with "/name " so the user adds their request before
|
||||
// submitting (a bare second Enter runs it without one); names the slash
|
||||
// path cannot reach run immediately instead.
|
||||
m, cmd = m.chooseSkillFromPicker(item)
|
||||
case pickerTheme:
|
||||
// The hovered palette is already live from the preview; handleThemeCommand
|
||||
// records the choice (m.themeMode) and re-applies it, and reports the switch.
|
||||
@@ -3845,6 +3853,15 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) {
|
||||
case commandTools:
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: m.toolsText()})
|
||||
return m, nil
|
||||
case commandSkills:
|
||||
// With skills installed, /skills opens a searchable picker (like /model);
|
||||
// the text card remains only as the no-skills install hint.
|
||||
if picker := m.newSkillPicker(); picker != nil {
|
||||
m.picker = picker
|
||||
return m, nil
|
||||
}
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: m.skillsText()})
|
||||
return m, nil
|
||||
case commandMCP:
|
||||
if strings.TrimSpace(command.text) == "" {
|
||||
return m.openMCPManager(), nil
|
||||
@@ -4040,6 +4057,11 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) {
|
||||
if next, cmd, handled := m.handleUserCommand(command.text); handled {
|
||||
return next, cmd
|
||||
}
|
||||
// Then an installed skill: "/<skill-name> [args]" runs the skill directly
|
||||
// (deterministic invocation, vs waiting for the model to pull it in).
|
||||
if next, cmd, handled := m.handleSkillCommand(command.text); handled {
|
||||
return next, cmd
|
||||
}
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{
|
||||
kind: actionAppendError,
|
||||
text: "unknown command: " + command.text,
|
||||
@@ -4446,9 +4468,9 @@ func (m model) runAgentWithOptions(runID int, runCtx context.Context, prompt str
|
||||
IncludeLSP: true,
|
||||
Autonomy: selfCorrectAutonomyForMode(options.PermissionMode),
|
||||
})
|
||||
// Background post-edit diagnostics: the loop checks files changed by
|
||||
// edit_file/write_file off the tool-call path and nudges the model with
|
||||
// any errors before its next request. Shares the run's lazy manager.
|
||||
// Inline post-edit diagnostics: edit_file/write_file append error
|
||||
// diagnostics for the file they just wrote to their own output, so the
|
||||
// model sees a break in the same turn. Shares the run's lazy manager.
|
||||
options.FileDiagnostics = agent.NewFileDiagnostics(lspManager, options.Cwd)
|
||||
}
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
"github.com/Gitlawb/zero/internal/providermodeldiscovery"
|
||||
"github.com/Gitlawb/zero/internal/sandbox"
|
||||
"github.com/Gitlawb/zero/internal/sessions"
|
||||
"github.com/Gitlawb/zero/internal/skills"
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/usage"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
@@ -48,7 +49,13 @@ type Options struct {
|
||||
SessionCompactor SessionCompactor
|
||||
PrService *PrService
|
||||
|
||||
AgentOptions agent.Options
|
||||
AgentOptions agent.Options
|
||||
// LoadSkills returns the installed skills (default skills dir merged with any
|
||||
// plugin skill roots), bodies included, for /skills and direct /<skill-name>
|
||||
// invocation. Called lazily per use so newly installed skills are picked up
|
||||
// without a restart. Nil means the session has no skills wiring (skills stay
|
||||
// model-pulled via the skill tool only).
|
||||
LoadSkills func() []skills.Skill
|
||||
PermissionMode agent.PermissionMode
|
||||
ReasoningEffort modelregistry.ReasoningEffort
|
||||
ResponseStyle string
|
||||
|
||||
@@ -24,6 +24,7 @@ const (
|
||||
pickerEffort
|
||||
pickerSession
|
||||
pickerTheme
|
||||
pickerSkill
|
||||
)
|
||||
|
||||
// pickerItem is one selectable row: Label is shown, Value is passed to the
|
||||
@@ -225,6 +226,35 @@ func (m model) newModelPicker() *commandPicker {
|
||||
return &commandPicker{kind: pickerModel, title: "Choose a model", items: items, allItems: append([]pickerItem{}, items...), selected: 0}
|
||||
}
|
||||
|
||||
// newSkillPicker lists the installed skills for browse-and-run: type to filter,
|
||||
// arrow keys to select, Enter runs the chosen skill immediately. Selection is by
|
||||
// the skill's EXACT name (item.Value), so even skills the slash path cannot
|
||||
// reach — names shadowed by a builtin or user command, or names that don't fit
|
||||
// a slash token — are runnable here: picking from the Skills picker is
|
||||
// unambiguous. Returns nil when no skills are installed (the caller falls back
|
||||
// to the install-hint card).
|
||||
func (m model) newSkillPicker() *commandPicker {
|
||||
installed := m.installedSkills()
|
||||
items := make([]pickerItem, 0, len(installed))
|
||||
for _, skill := range installed {
|
||||
name := strings.TrimSpace(skill.Name)
|
||||
if name == "" {
|
||||
continue
|
||||
}
|
||||
label := name
|
||||
if slash := skillSlashName(name); slash != "" {
|
||||
// Show the slash form where one exists — it doubles as documentation
|
||||
// of the direct "/name [args]" invocation.
|
||||
label = "/" + slash
|
||||
}
|
||||
items = append(items, pickerItem{Label: label, Value: name, Meta: strings.TrimSpace(skill.Description)})
|
||||
}
|
||||
if len(items) == 0 {
|
||||
return nil
|
||||
}
|
||||
return &commandPicker{kind: pickerSkill, title: "pick a skill · ⏎ fills /name — add your request", items: items, allItems: append([]pickerItem{}, items...), selected: 0}
|
||||
}
|
||||
|
||||
// modelPickerProviders returns the providers to list in /model: all saved
|
||||
// providers, falling back to the active profile when none were threaded in.
|
||||
func (m model) modelPickerProviders() []config.ProviderProfile {
|
||||
|
||||
@@ -0,0 +1,205 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"strings"
|
||||
|
||||
tea "charm.land/bubbletea/v2"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/skills"
|
||||
)
|
||||
|
||||
// Skill slash invocation: "/<skill-name> [args]" runs an installed skill
|
||||
// directly, mirroring user commands (.zero/commands). Skills were previously
|
||||
// model-pulled only (the skill tool), which made invocation a matter of model
|
||||
// discretion; typing the skill's name makes it deterministic. Precedence is
|
||||
// builtin > user command > skill: parseCommand resolves builtins first, and the
|
||||
// commandUnknown fallback tries user commands before skills.
|
||||
|
||||
// handleSkillCommand resolves a "/name args" that wasn't a builtin or a user
|
||||
// command against the installed skills. On a match it launches a normal agent
|
||||
// turn whose prompt inlines the skill body (its instructions) followed by the
|
||||
// typed args, returning handled=true. handled=false means no skill matched, so
|
||||
// the caller falls through to "unknown command".
|
||||
func (m model) handleSkillCommand(raw string) (model, tea.Cmd, bool) {
|
||||
name, args := splitUserCommand(raw)
|
||||
if name == "" {
|
||||
return m, nil, false
|
||||
}
|
||||
skill, ok := m.lookupSkillCommand(name)
|
||||
if !ok {
|
||||
return m, nil, false
|
||||
}
|
||||
body := strings.TrimSpace(skill.Content)
|
||||
if body == "" {
|
||||
// The name matched a real skill, so falling through to "unknown command"
|
||||
// would mislead; surface the actual problem instead.
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{
|
||||
kind: actionAppendError,
|
||||
text: "skill /" + name + " has an empty SKILL.md body (" + skill.Path + ")",
|
||||
})
|
||||
return m, nil, true
|
||||
}
|
||||
return m.launchOrDeferExpandedPrompt(raw, skillInvocationPrompt(body, args))
|
||||
}
|
||||
|
||||
// bareSkillInvocationNote is appended when a skill is invoked with no request.
|
||||
// The body alone is instructions with no target ("review the PR" — which PR?),
|
||||
// and without this note the model improvises one instead of asking. The wording
|
||||
// is conditional so self-contained skills (no target needed) still just run.
|
||||
const bareSkillInvocationNote = "The user invoked this skill directly without providing a request. " +
|
||||
"If these instructions need a target or details that are not already clear from the conversation " +
|
||||
"(which pull request, file, branch, topic, …), ask for them first — do not guess or pick one yourself. " +
|
||||
"If the instructions are self-contained, proceed."
|
||||
|
||||
// skillInvocationPrompt builds the agent prompt for a skill invocation: the
|
||||
// skill body (its instructions), then either the user's request or — for a bare
|
||||
// invocation — the ask-first note above. Mirrors usercommands.Expand's
|
||||
// no-placeholder behavior for the args case.
|
||||
func skillInvocationPrompt(body, args string) string {
|
||||
if args != "" {
|
||||
return body + "\n\n" + args
|
||||
}
|
||||
return body + "\n\n" + bareSkillInvocationNote
|
||||
}
|
||||
|
||||
// launchOrDeferExpandedPrompt applies the same run-state guards the plain
|
||||
// commandPrompt path has to an expanded (user command / skill) prompt: while
|
||||
// exiting nothing may start, a prompt submitted mid-run is queued for the next
|
||||
// turn, and compaction-in-flight warns instead of racing the compactor. The
|
||||
// EXPANDED prompt is what gets queued — the queue flush path resubmits text as
|
||||
// a literal prompt, so queuing the raw "/name args" would send it to the model
|
||||
// as prose instead of re-dispatching it. raw is the typed invocation; on the
|
||||
// compaction warning it is restored into the composer for an easy re-submit
|
||||
// (the commandUnknown dispatch cleared the composer before we got here, unlike
|
||||
// the plain-prompt path whose guard fires before the clear). Empty raw (the
|
||||
// picker path has no typed form) restores nothing.
|
||||
func (m model) launchOrDeferExpandedPrompt(raw, prompt string) (model, tea.Cmd, bool) {
|
||||
if m.exiting {
|
||||
return m, nil, true
|
||||
}
|
||||
if m.pending {
|
||||
return m.queueMessage(prompt), nil, true
|
||||
}
|
||||
if m.compactInFlight {
|
||||
if strings.TrimSpace(raw) != "" {
|
||||
m.input.SetValue(raw)
|
||||
m.input.CursorEnd()
|
||||
}
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{
|
||||
kind: actionAppendSystem,
|
||||
text: "Compact\nstatus: warning\nCompaction is running. Re-run the command when it finishes.",
|
||||
})
|
||||
return m, nil, true
|
||||
}
|
||||
next, teaCmd := m.launchPrompt(prompt)
|
||||
return next, teaCmd, true
|
||||
}
|
||||
|
||||
// takenSlashNames returns every slash token already claimed by a builtin
|
||||
// command (or alias) or a user command — the names a typed "/x" would dispatch
|
||||
// to before ever reaching a skill.
|
||||
func (m model) takenSlashNames() map[string]bool {
|
||||
taken := map[string]bool{}
|
||||
for _, command := range commandDefinitions {
|
||||
taken[strings.TrimPrefix(command.name, "/")] = true
|
||||
for _, alias := range command.aliases {
|
||||
taken[strings.TrimPrefix(alias, "/")] = true
|
||||
}
|
||||
}
|
||||
for _, cmd := range m.userCommands {
|
||||
taken[cmd.Name] = true
|
||||
}
|
||||
return taken
|
||||
}
|
||||
|
||||
// chooseSkillFromPicker handles Enter on a skill-picker row. Most skills want a
|
||||
// request ("review WHICH pr?"), so selection fills the composer with
|
||||
// "/name " — cursor at the end — for the user to complete and submit (Enter
|
||||
// again runs it bare). Skills whose names cannot be typed as a slash command
|
||||
// (non-slash shapes, or names shadowed by a builtin or user command) fall back
|
||||
// to running immediately: the picker is the only path that reaches them, and an
|
||||
// unfillable composer would be a dead end.
|
||||
func (m model) chooseSkillFromPicker(item pickerItem) (model, tea.Cmd) {
|
||||
slash := skillSlashName(item.Value)
|
||||
if slash == "" || m.takenSlashNames()[slash] {
|
||||
return m.invokeSkillByName(item.Value)
|
||||
}
|
||||
m.input.SetValue("/" + slash + " ")
|
||||
m.input.CursorEnd()
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// invokeSkillByName runs the installed skill with the given EXACT raw name (the
|
||||
// skill-picker path). The same run-state guards as slash invocation apply; the
|
||||
// picker offers no args affordance, so the prompt is the skill body alone.
|
||||
func (m model) invokeSkillByName(name string) (model, tea.Cmd) {
|
||||
for _, skill := range m.installedSkills() {
|
||||
if strings.TrimSpace(skill.Name) != name {
|
||||
continue
|
||||
}
|
||||
body := strings.TrimSpace(skill.Content)
|
||||
if body == "" {
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{
|
||||
kind: actionAppendError,
|
||||
text: "skill " + name + " has an empty SKILL.md body (" + skill.Path + ")",
|
||||
})
|
||||
return m, nil
|
||||
}
|
||||
next, teaCmd, _ := m.launchOrDeferExpandedPrompt("", skillInvocationPrompt(body, ""))
|
||||
return next, teaCmd
|
||||
}
|
||||
// The picker row came from a slightly older load (TTL cache) and the skill
|
||||
// has since been removed.
|
||||
m.transcript = reduceTranscript(m.transcript, transcriptAction{
|
||||
kind: actionAppendError,
|
||||
text: "skill " + name + " is no longer installed",
|
||||
})
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// lookupSkillCommand returns the installed skill whose slash name matches the
|
||||
// given (lowercased) name. Linear scan over a fresh load — skills are re-read
|
||||
// per invocation so a skill installed mid-session is invocable without a
|
||||
// restart, and the body is never held on the model.
|
||||
func (m model) lookupSkillCommand(name string) (skills.Skill, bool) {
|
||||
name = strings.ToLower(strings.TrimSpace(name))
|
||||
if name == "" {
|
||||
return skills.Skill{}, false
|
||||
}
|
||||
for _, skill := range m.installedSkills() {
|
||||
if skillSlashName(skill.Name) == name {
|
||||
return skill, true
|
||||
}
|
||||
}
|
||||
return skills.Skill{}, false
|
||||
}
|
||||
|
||||
// installedSkills returns the session's installed skills (default dir merged
|
||||
// with plugin skill roots) via the injected loader, or nil when the session has
|
||||
// no skills wiring (e.g. bare test models).
|
||||
func (m model) installedSkills() []skills.Skill {
|
||||
if m.loadSkills == nil {
|
||||
return nil
|
||||
}
|
||||
return m.loadSkills()
|
||||
}
|
||||
|
||||
// skillSlashName maps a skill's frontmatter name to its slash-command form:
|
||||
// lowercased, and only if it fits the slash-token shape (letters, digits,
|
||||
// dot/underscore/hyphen — a superset of user-command names, since skill names
|
||||
// are free-form frontmatter). Returns "" for names that cannot be typed as a
|
||||
// /command (e.g. containing spaces); those skills remain loadable by the model
|
||||
// via the skill tool and are still listed by /skills.
|
||||
func skillSlashName(name string) string {
|
||||
name = strings.ToLower(strings.TrimSpace(name))
|
||||
if name == "" {
|
||||
return ""
|
||||
}
|
||||
for _, r := range name {
|
||||
valid := (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' || r == '.'
|
||||
if !valid {
|
||||
return ""
|
||||
}
|
||||
}
|
||||
return name
|
||||
}
|
||||
@@ -0,0 +1,412 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
tea "charm.land/bubbletea/v2"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/agent"
|
||||
"github.com/Gitlawb/zero/internal/skills"
|
||||
)
|
||||
|
||||
// newSkillTestModel builds a model with an injected skills loader (and the
|
||||
// matching palette metadata), no filesystem involved.
|
||||
func newSkillTestModel(t *testing.T, installed ...skills.Skill) model {
|
||||
t.Helper()
|
||||
infos := make([]agent.SkillInfo, 0, len(installed))
|
||||
for _, s := range installed {
|
||||
infos = append(infos, agent.SkillInfo{Name: s.Name, Description: s.Description})
|
||||
}
|
||||
return newModel(context.Background(), Options{
|
||||
Cwd: t.TempDir(),
|
||||
LoadSkills: func() []skills.Skill { return installed },
|
||||
AgentOptions: agent.Options{Skills: infos},
|
||||
})
|
||||
}
|
||||
|
||||
func submitInput(t *testing.T, m model, input string) model {
|
||||
t.Helper()
|
||||
m.input.SetValue(input)
|
||||
updated, _ := m.Update(testKey(tea.KeyEnter))
|
||||
return updated.(model)
|
||||
}
|
||||
|
||||
func TestSkillCommandInvokesAndSubmits(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{
|
||||
Name: "deploy-checks",
|
||||
Description: "Run the pre-deploy verification suite.",
|
||||
Content: "Run every verification step and summarize failures.",
|
||||
Path: "/skills/deploy-checks/SKILL.md",
|
||||
})
|
||||
|
||||
next := submitInput(t, m, "/deploy-checks ship v2 to production")
|
||||
|
||||
if transcriptContains(next.transcript, "unknown command") {
|
||||
t.Fatalf("an installed skill must not be 'unknown', got %#v", next.transcript)
|
||||
}
|
||||
if !transcriptContains(next.transcript, "Run every verification step and summarize failures.") {
|
||||
t.Fatalf("skill body should be inlined into the prompt, got %#v", next.transcript)
|
||||
}
|
||||
if !transcriptContains(next.transcript, "ship v2 to production") {
|
||||
t.Fatalf("typed args should follow the skill body, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillCommandWithoutArgs(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review the diff for correctness."})
|
||||
|
||||
next := submitInput(t, m, "/reviewer")
|
||||
|
||||
if !transcriptContains(next.transcript, "Review the diff for correctness.") {
|
||||
t.Fatalf("bare skill invocation should submit the body, got %#v", next.transcript)
|
||||
}
|
||||
// A bare invocation carries the ask-first note: instructions with no target
|
||||
// must make the model ask ("which PR?") instead of improvising one.
|
||||
if !transcriptContains(next.transcript, "ask for them first") {
|
||||
t.Fatalf("bare invocation should carry the clarify-first note, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
// An invocation WITH a request must not carry the ask-first note — the user
|
||||
// already said what to apply the skill to.
|
||||
func TestSkillCommandWithArgsHasNoClarifyNote(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review the diff."})
|
||||
|
||||
next := submitInput(t, m, "/reviewer PR 484 reconnect changes")
|
||||
|
||||
if transcriptContains(next.transcript, "ask for them first") {
|
||||
t.Fatalf("args invocation must not carry the clarify note, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
// A skill's frontmatter name is free-form; invocation matches its lowercased
|
||||
// slash form.
|
||||
func TestSkillCommandNameIsCaseInsensitive(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "Deploy-Checks", Content: "Check things."})
|
||||
|
||||
next := submitInput(t, m, "/deploy-checks")
|
||||
|
||||
if !transcriptContains(next.transcript, "Check things.") {
|
||||
t.Fatalf("uppercase frontmatter name should be invocable lowercased, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
// Precedence: a user command (.zero/commands) shadows a same-named skill.
|
||||
func TestUserCommandShadowsSkill(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeUserCommand(t, root, "greet.md", "Say hello from the user command.")
|
||||
m := newModel(context.Background(), Options{
|
||||
Cwd: root,
|
||||
LoadSkills: func() []skills.Skill { return []skills.Skill{{Name: "greet", Content: "Skill body must not run."}} },
|
||||
})
|
||||
|
||||
next := submitInput(t, m, "/greet")
|
||||
|
||||
if !transcriptContains(next.transcript, "Say hello from the user command.") {
|
||||
t.Fatalf("user command should win the name collision, got %#v", next.transcript)
|
||||
}
|
||||
if transcriptContains(next.transcript, "Skill body must not run.") {
|
||||
t.Fatalf("shadowed skill must not run, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
// An empty skill body is a real, named match — surface the problem instead of
|
||||
// falling through to a misleading "unknown command".
|
||||
func TestSkillCommandEmptyBody(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "hollow", Path: "/skills/hollow/SKILL.md"})
|
||||
|
||||
next := submitInput(t, m, "/hollow")
|
||||
|
||||
if transcriptContains(next.transcript, "unknown command") {
|
||||
t.Fatalf("an empty skill must not be 'unknown', got %#v", next.transcript)
|
||||
}
|
||||
if !transcriptContains(next.transcript, "empty SKILL.md body") {
|
||||
t.Fatalf("empty-body error should be surfaced, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnknownSlashStillUnknownWithSkillsInstalled(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review."})
|
||||
|
||||
next := submitInput(t, m, "/definitelynotaskill")
|
||||
|
||||
if !transcriptContains(next.transcript, "unknown command") {
|
||||
t.Fatalf("non-matching name should still be unknown, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillSlashName(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"deploy-checks": "deploy-checks",
|
||||
"Deploy-Checks": "deploy-checks",
|
||||
" pdf_tools ": "pdf_tools",
|
||||
"v2.migrate": "v2.migrate",
|
||||
"has space": "",
|
||||
"emoji✨": "",
|
||||
"": "",
|
||||
"slash/inside": "",
|
||||
"tab\tseparated": "",
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := skillSlashName(in); got != want {
|
||||
t.Errorf("skillSlashName(%q) = %q, want %q", in, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillAppearsInAutocomplete(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "deploy-checks", Description: "Run the pre-deploy suite.", Content: "x"})
|
||||
|
||||
got := m.matchCommandSuggestions("/dep")
|
||||
found := false
|
||||
for _, s := range got {
|
||||
if s.Name == "/deploy-checks" {
|
||||
found = true
|
||||
if s.Desc != "Run the pre-deploy suite. (skill)" {
|
||||
t.Fatalf("skill suggestion should be labeled, got %q", s.Desc)
|
||||
}
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("skill /deploy-checks should appear for '/dep', got %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A skill shadowed by a builtin (or its alias) or a user command would be dead
|
||||
// at dispatch time, so the palette must not advertise it.
|
||||
func TestShadowedSkillNotSuggested(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeUserCommand(t, root, "deploy.md", "User deploy.")
|
||||
installed := []skills.Skill{
|
||||
{Name: "help", Content: "x"}, // builtin collision
|
||||
{Name: "sessions", Content: "x"}, // builtin ALIAS collision (/resume alias)
|
||||
{Name: "deploy", Content: "x"}, // user-command collision
|
||||
}
|
||||
infos := make([]agent.SkillInfo, 0, len(installed))
|
||||
for _, s := range installed {
|
||||
infos = append(infos, agent.SkillInfo{Name: s.Name})
|
||||
}
|
||||
m := newModel(context.Background(), Options{
|
||||
Cwd: root,
|
||||
LoadSkills: func() []skills.Skill { return installed },
|
||||
AgentOptions: agent.Options{Skills: infos},
|
||||
})
|
||||
|
||||
// Assert on the advertised NAME with a skill-marked Desc, not on the exact
|
||||
// fallback Desc format — the invariant is that no skill row carries a
|
||||
// builtin/user-command name, regardless of description wording.
|
||||
for _, token := range []string{"/hel", "/ses", "/dep"} {
|
||||
for _, s := range m.matchSkillSuggestions(token) {
|
||||
if s.Name == "/help" || s.Name == "/sessions" || s.Name == "/deploy" {
|
||||
t.Fatalf("shadowed skill advertised for %q: %#v", token, s)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Invalid slash shapes (spaces etc.) and a bare "/" must not surface skills.
|
||||
func TestSkillSuggestionEdgeCases(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "has space", Description: "not invocable", Content: "x"})
|
||||
|
||||
if got := m.matchSkillSuggestions("/has"); len(got) != 0 {
|
||||
t.Fatalf("invalid slash-shape skill must not be suggested, got %#v", got)
|
||||
}
|
||||
if got := m.matchSkillSuggestions("/"); got != nil {
|
||||
t.Fatalf("bare '/' should not list skills, got %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillsCommandResolves(t *testing.T) {
|
||||
command, ok := resolveCommand("/skills")
|
||||
if !ok || command.kind != commandSkills {
|
||||
t.Fatalf("resolveCommand(/skills) = %#v, %v", command, ok)
|
||||
}
|
||||
}
|
||||
|
||||
// With skills installed, /skills opens a searchable picker (like /model).
|
||||
func TestSkillsCommandOpensPicker(t *testing.T) {
|
||||
m := newSkillTestModel(t,
|
||||
skills.Skill{Name: "deploy-checks", Description: "Run the pre-deploy suite.", Content: "x"},
|
||||
skills.Skill{Name: "has space", Description: "tool-only skill", Content: "x"},
|
||||
)
|
||||
|
||||
next := submitInput(t, m, "/skills")
|
||||
|
||||
if next.picker == nil || next.picker.kind != pickerSkill {
|
||||
t.Fatalf("/skills should open the skill picker, got %#v", next.picker)
|
||||
}
|
||||
labels := map[string]string{}
|
||||
for _, item := range next.picker.items {
|
||||
labels[item.Label] = item.Value
|
||||
}
|
||||
if labels["/deploy-checks"] != "deploy-checks" {
|
||||
t.Fatalf("slash-invocable skill should show its slash form, got %#v", labels)
|
||||
}
|
||||
// A non-slash-shaped name is still selectable (bare label, exact-name value).
|
||||
if labels["has space"] != "has space" {
|
||||
t.Fatalf("non-slash skill should be listed bare but selectable, got %#v", labels)
|
||||
}
|
||||
}
|
||||
|
||||
// Enter on a picker row fills the composer with "/name " so the user can add
|
||||
// their request (which PR? which file?) before submitting — it must NOT fire
|
||||
// the skill with no request attached.
|
||||
func TestSkillPickerEnterFillsComposer(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review the diff for correctness."})
|
||||
|
||||
next := submitInput(t, m, "/skills")
|
||||
if next.picker == nil {
|
||||
t.Fatal("picker should be open")
|
||||
}
|
||||
updated, _ := next.Update(testKey(tea.KeyEnter))
|
||||
after := updated.(model)
|
||||
|
||||
if after.picker != nil {
|
||||
t.Fatal("picker should close on Enter")
|
||||
}
|
||||
if got := after.input.Value(); got != "/reviewer " {
|
||||
t.Fatalf("composer should be filled with the invocation, got %q", got)
|
||||
}
|
||||
if transcriptContains(after.transcript, "Review the diff for correctness.") {
|
||||
t.Fatalf("skill must not run before the user submits, got %#v", after.transcript)
|
||||
}
|
||||
|
||||
// A second Enter submits the (bare) invocation and runs the skill.
|
||||
final, _ := after.Update(testKey(tea.KeyEnter))
|
||||
done := final.(model)
|
||||
if !transcriptContains(done.transcript, "Review the diff for correctness.") {
|
||||
t.Fatalf("submitting the filled composer should run the skill, got %#v", done.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
// Picker selection is by exact name, so a skill shadowed by a builtin (dead on
|
||||
// the slash path) is still runnable from the picker.
|
||||
func TestSkillPickerRunsBuiltinShadowedSkill(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "help", Description: "shadowed", Content: "Shadowed skill body."})
|
||||
|
||||
next := submitInput(t, m, "/skills")
|
||||
if next.picker == nil {
|
||||
t.Fatal("picker should be open")
|
||||
}
|
||||
updated, _ := next.Update(testKey(tea.KeyEnter))
|
||||
after := updated.(model)
|
||||
|
||||
if !transcriptContains(after.transcript, "Shadowed skill body.") {
|
||||
t.Fatalf("picker must run the shadowed skill directly, got %#v", after.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillsCommandEmptyState(t *testing.T) {
|
||||
m := newModel(context.Background(), Options{Cwd: t.TempDir()})
|
||||
|
||||
next := submitInput(t, m, "/skills")
|
||||
|
||||
if !transcriptContains(next.transcript, "No skills installed.") {
|
||||
t.Fatalf("empty state should say no skills, got %#v", next.transcript)
|
||||
}
|
||||
}
|
||||
|
||||
// Run-state guards: an expanded skill prompt must not race an active run — it
|
||||
// queues (as the EXPANDED prompt, since the queue flush submits literal text).
|
||||
func TestSkillInvocationQueuedWhileRunPending(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review the diff."})
|
||||
m.pending = true
|
||||
|
||||
next, _, handled := m.handleSkillCommand("/reviewer focus on tests")
|
||||
if !handled {
|
||||
t.Fatal("skill must be handled even while a run is pending")
|
||||
}
|
||||
if next.queuedMessage == "" || !strings.Contains(next.queuedMessage, "Review the diff.") {
|
||||
t.Fatalf("expanded prompt should be queued, got %q", next.queuedMessage)
|
||||
}
|
||||
if !strings.Contains(next.queuedMessage, "focus on tests") {
|
||||
t.Fatalf("queued prompt should carry the args, got %q", next.queuedMessage)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillInvocationDroppedWhileExiting(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review."})
|
||||
m.exiting = true
|
||||
|
||||
next, cmd, handled := m.handleSkillCommand("/reviewer")
|
||||
if !handled || cmd != nil || next.queuedMessage != "" {
|
||||
t.Fatalf("exiting must swallow the invocation: handled=%v cmd=%v queued=%q", handled, cmd, next.queuedMessage)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillInvocationWarnsDuringCompaction(t *testing.T) {
|
||||
m := newSkillTestModel(t, skills.Skill{Name: "reviewer", Content: "Review."})
|
||||
m.compactInFlight = true
|
||||
|
||||
next, _, handled := m.handleSkillCommand("/reviewer")
|
||||
if !handled {
|
||||
t.Fatal("skill must be handled during compaction")
|
||||
}
|
||||
if !transcriptContains(next.transcript, "Compaction is running") {
|
||||
t.Fatalf("compaction warning expected, got %#v", next.transcript)
|
||||
}
|
||||
// The typed invocation is restored for an easy re-submit (the dispatch path
|
||||
// cleared the composer before the guard could fire).
|
||||
if got := next.input.Value(); got != "/reviewer" {
|
||||
t.Fatalf("typed invocation should be restored into the composer, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The same guard now protects user commands (previously they could start a
|
||||
// second concurrent run).
|
||||
func TestUserCommandQueuedWhileRunPending(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeUserCommand(t, root, "greet.md", "Say hello to $1.")
|
||||
m := newModel(context.Background(), Options{Cwd: root})
|
||||
m.pending = true
|
||||
|
||||
next, _, handled := m.handleUserCommand("/greet world")
|
||||
if !handled {
|
||||
t.Fatal("user command must be handled while pending")
|
||||
}
|
||||
if !strings.Contains(next.queuedMessage, "Say hello to world.") {
|
||||
t.Fatalf("expanded user-command prompt should be queued, got %q", next.queuedMessage)
|
||||
}
|
||||
}
|
||||
|
||||
// Two skills whose names collide after lowercasing map to one slash name; only
|
||||
// the one dispatch will run may be advertised.
|
||||
func TestCaseCollidingSkillsAdvertisedOnce(t *testing.T) {
|
||||
m := newSkillTestModel(t,
|
||||
skills.Skill{Name: "Deploy", Description: "wins", Content: "a"},
|
||||
skills.Skill{Name: "deploy", Description: "loses", Content: "b"},
|
||||
)
|
||||
|
||||
rows := 0
|
||||
for _, s := range m.matchSkillSuggestions("/dep") {
|
||||
if s.Name == "/deploy" {
|
||||
rows++
|
||||
if !strings.Contains(s.Desc, "wins") {
|
||||
t.Fatalf("advertised row must describe the skill dispatch runs, got %q", s.Desc)
|
||||
}
|
||||
}
|
||||
}
|
||||
if rows != 1 {
|
||||
t.Fatalf("case-colliding skills must yield exactly one /deploy row, got %d", rows)
|
||||
}
|
||||
}
|
||||
|
||||
// The palette reads through the loader, so a skill installed mid-session (the
|
||||
// loader's next result) appears without a restart.
|
||||
func TestSkillPaletteReadsThroughLoader(t *testing.T) {
|
||||
installed := []skills.Skill{}
|
||||
m := newModel(context.Background(), Options{
|
||||
Cwd: t.TempDir(),
|
||||
LoadSkills: func() []skills.Skill { return installed },
|
||||
})
|
||||
if got := m.matchSkillSuggestions("/lat"); len(got) != 0 {
|
||||
t.Fatalf("no skills installed yet, got %#v", got)
|
||||
}
|
||||
installed = append(installed, skills.Skill{Name: "late-arrival", Description: "installed mid-session", Content: "x"})
|
||||
got := m.matchSkillSuggestions("/lat")
|
||||
if len(got) != 1 || got[0].Name != "/late-arrival" {
|
||||
t.Fatalf("mid-session skill should appear via the loader, got %#v", got)
|
||||
}
|
||||
}
|
||||
@@ -30,8 +30,9 @@ func (m model) handleUserCommand(raw string) (model, tea.Cmd, bool) {
|
||||
if strings.TrimSpace(prompt) == "" {
|
||||
return m, nil, false
|
||||
}
|
||||
next, teaCmd := m.launchPrompt(prompt)
|
||||
return next, teaCmd, true
|
||||
// Same run-state guards as a plain prompt: a user command invoked mid-run is
|
||||
// queued (as its expanded prompt), not raced into a second concurrent turn.
|
||||
return m.launchOrDeferExpandedPrompt(raw, prompt)
|
||||
}
|
||||
|
||||
// lookupUserCommand returns the loaded user command with the given (lowercased)
|
||||
|
||||
Reference in new issue
Block a user