diff --git a/pkg/debugcmd/debug.go b/pkg/debugcmd/debug.go index ed43f853..645910a4 100644 --- a/pkg/debugcmd/debug.go +++ b/pkg/debugcmd/debug.go @@ -2,19 +2,99 @@ package debugcmd import ( "context" + "fmt" "os" "os/exec" + "strings" + "sync" ) -func New(ctx context.Context, arg string, args ...string) *exec.Cmd { - cmd := exec.CommandContext(ctx, arg, args...) - SetupDebug(cmd) - return cmd +type WrappedCmd struct { + c *exec.Cmd + r recorder + Env []string + Dir string } -func SetupDebug(cmd *exec.Cmd) { +func (w *WrappedCmd) Run() error { + if len(w.Env) > 0 { + w.c.Env = w.Env + } + if w.Dir != "" { + w.c.Dir = w.Dir + } + if err := w.c.Run(); err != nil { + msg := w.r.dump() + if msg != "" { + return fmt.Errorf("%w: %s", err, msg) + } + return err + } + return nil +} + +func New(ctx context.Context, arg string, args ...string) *WrappedCmd { + w := &WrappedCmd{ + c: exec.CommandContext(ctx, arg, args...), + } + setupDebug(w) + return w +} + +type entry struct { + err bool + data []byte +} + +type recorder struct { + lock sync.Mutex + entries []entry +} + +func (r *recorder) dump() string { + var errMessage strings.Builder + for _, entry := range r.entries { + if entry.err { + errMessage.Write(entry.data) + _, _ = os.Stderr.Write(entry.data) + } else { + _, _ = os.Stdout.Write(entry.data) + } + } + return errMessage.String() +} + +type writer struct { + err bool + r *recorder +} + +func (w *writer) Write(data []byte) (int, error) { + w.r.lock.Lock() + defer w.r.lock.Unlock() + + cp := make([]byte, len(data)) + copy(cp, data) + + w.r.entries = append(w.r.entries, entry{ + err: w.err, + data: cp, + }) + + return len(data), nil +} + +func setupDebug(w *WrappedCmd) { if log.IsDebug() { - cmd.Stdout = os.Stdout + w.c.Stdout = os.Stdout + w.c.Stderr = os.Stderr + } else { + w.c.Stdout = &writer{ + r: &w.r, + } + w.c.Stderr = &writer{ + err: true, + r: &w.r, + } } - cmd.Stderr = os.Stderr } diff --git a/pkg/engine/daemon.go b/pkg/engine/daemon.go index fe884c52..607b7870 100644 --- a/pkg/engine/daemon.go +++ b/pkg/engine/daemon.go @@ -100,6 +100,12 @@ func (e *Engine) startDaemon(_ context.Context, tool types.Tool) (string, error) return url, err } + r, w, err := os.Pipe() + if err != nil { + return "", err + } + + cmd.Stdin = r cmd.Stderr = os.Stderr cmd.Stdout = os.Stdout log.Infof("launched [%s][%s] port [%d] %v", tool.Parameters.Name, tool.ID, port, cmd.Args) @@ -121,6 +127,8 @@ func (e *Engine) startDaemon(_ context.Context, tool types.Tool) (string, error) if err != nil { log.Errorf("daemon exited tool [%s] %v: %v", tool.Parameters.Name, cmd.Args, err) } + _ = r.Close() + _ = w.Close() cancel(err) stop() @@ -133,7 +141,7 @@ func (e *Engine) startDaemon(_ context.Context, tool types.Tool) (string, error) daemonWG.Add(1) context.AfterFunc(ctx, func() { if err := cmd.Process.Kill(); err != nil { - log.Errorf("daemon failed to kill tool [%s] process: %v", tool.Parameters.Name, err) + log.Debugf("daemon failed to kill tool [%s] process: %v", tool.Parameters.Name, err) } daemonWG.Done() }) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 588ee3ef..3a708d79 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -155,14 +155,18 @@ func (c *Context) appendTool(completion *types.CompletionRequest, parentTool typ c.toolNames = map[string]struct{}{} } - completion.Tools = append(completion.Tools, types.CompletionTool{ - Function: types.CompletionFunctionDefinition{ - ToolID: subTool.ID, - Name: PickToolName(subToolName, c.toolNames), - Description: subTool.Parameters.Description, - Parameters: args, - }, - }) + if subTool.Instructions == "" { + log.Debugf("Skipping zero instruction tool %s (%s)", subToolName, subTool.ID) + } else { + completion.Tools = append(completion.Tools, types.CompletionTool{ + Function: types.CompletionFunctionDefinition{ + ToolID: subTool.ID, + Name: PickToolName(subToolName, c.toolNames), + Description: subTool.Parameters.Description, + Parameters: args, + }, + }) + } for _, export := range subTool.Export { if err := c.appendTool(completion, subTool, export); err != nil { diff --git a/pkg/repos/git/cmd.go b/pkg/repos/git/cmd.go index 7bbf0ecb..e3662771 100644 --- a/pkg/repos/git/cmd.go +++ b/pkg/repos/git/cmd.go @@ -2,14 +2,12 @@ package git import ( "context" - "os/exec" "github.com/gptscript-ai/gptscript/pkg/debugcmd" ) -func newGitCommand(ctx context.Context, args ...string) *exec.Cmd { - cmd := exec.CommandContext(ctx, "git", args...) - debugcmd.SetupDebug(cmd) +func newGitCommand(ctx context.Context, args ...string) *debugcmd.WrappedCmd { + cmd := debugcmd.New(ctx, "git", args...) return cmd } diff --git a/pkg/tests/testdata/TestExport/call1.golden b/pkg/tests/testdata/TestExport/call1.golden index b1d0c9f0..b6083d9e 100644 --- a/pkg/tests/testdata/TestExport/call1.golden +++ b/pkg/tests/testdata/TestExport/call1.golden @@ -4,7 +4,7 @@ "Tools": [ { "function": { - "toolID": "testdata/TestExport/parent.gpt:4", + "toolID": "testdata/TestExport/parent.gpt:5", "name": "frommain", "parameters": { "type": "object", @@ -22,7 +22,7 @@ }, { "function": { - "toolID": "testdata/TestExport/parent.gpt:8", + "toolID": "testdata/TestExport/parent.gpt:11", "name": "parent-local", "parameters": { "type": "object", @@ -38,24 +38,6 @@ } } }, - { - "function": { - "toolID": "testdata/TestExport/sub/child.gpt:4", - "name": "child", - "parameters": { - "type": "object", - "properties": { - "defaultPromptParameter": { - "description": "Prompt to send to the tool or assistant. This may be instructions or question.", - "type": "string" - } - }, - "required": [ - "defaultPromptParameter" - ] - } - } - }, { "function": { "toolID": "testdata/TestExport/sub/child.gpt:8", @@ -75,7 +57,16 @@ } } ], - "Messages": null, + "Messages": [ + { + "role": "system", + "content": [ + { + "text": "the default" + } + ] + } + ], "MaxTokens": 0, "Temperature": null, "JSONResponse": false, diff --git a/pkg/tests/testdata/TestExport/call2.golden b/pkg/tests/testdata/TestExport/call2.golden index babfc6da..87303d09 100644 --- a/pkg/tests/testdata/TestExport/call2.golden +++ b/pkg/tests/testdata/TestExport/call2.golden @@ -2,7 +2,16 @@ "Model": "gpt-4-turbo-preview", "InternalSystemPrompt": null, "Tools": null, - "Messages": null, + "Messages": [ + { + "role": "system", + "content": [ + { + "text": "the transient" + } + ] + } + ], "MaxTokens": 0, "Temperature": null, "JSONResponse": false, diff --git a/pkg/tests/testdata/TestExport/call3.golden b/pkg/tests/testdata/TestExport/call3.golden index 2ca34642..f5f62eda 100644 --- a/pkg/tests/testdata/TestExport/call3.golden +++ b/pkg/tests/testdata/TestExport/call3.golden @@ -4,7 +4,7 @@ "Tools": [ { "function": { - "toolID": "testdata/TestExport/parent.gpt:4", + "toolID": "testdata/TestExport/parent.gpt:5", "name": "frommain", "parameters": { "type": "object", @@ -22,7 +22,7 @@ }, { "function": { - "toolID": "testdata/TestExport/parent.gpt:8", + "toolID": "testdata/TestExport/parent.gpt:11", "name": "parent-local", "parameters": { "type": "object", @@ -38,24 +38,6 @@ } } }, - { - "function": { - "toolID": "testdata/TestExport/sub/child.gpt:4", - "name": "child", - "parameters": { - "type": "object", - "properties": { - "defaultPromptParameter": { - "description": "Prompt to send to the tool or assistant. This may be instructions or question.", - "type": "string" - } - }, - "required": [ - "defaultPromptParameter" - ] - } - } - }, { "function": { "toolID": "testdata/TestExport/sub/child.gpt:8", @@ -76,12 +58,20 @@ } ], "Messages": [ + { + "role": "system", + "content": [ + { + "text": "the default" + } + ] + }, { "role": "assistant", "content": [ { "toolCall": { - "index": 3, + "index": 2, "id": "call_1", "function": { "name": "transient" @@ -98,7 +88,7 @@ } ], "toolCall": { - "index": 3, + "index": 2, "id": "call_1", "function": { "name": "transient" diff --git a/pkg/tests/testdata/TestExport/parent.gpt b/pkg/tests/testdata/TestExport/parent.gpt index 212b8cf2..7588755b 100644 --- a/pkg/tests/testdata/TestExport/parent.gpt +++ b/pkg/tests/testdata/TestExport/parent.gpt @@ -1,8 +1,13 @@ tools: frommain, sub/child.gpt +the default --- name: frommain export: parent-local, fromchild from sub/child.gpt +the frommain + --- name: parent-local + +the parent-local diff --git a/pkg/tests/testdata/TestExport/sub/child.gpt b/pkg/tests/testdata/TestExport/sub/child.gpt index 2f591ace..2d2d4c3a 100644 --- a/pkg/tests/testdata/TestExport/sub/child.gpt +++ b/pkg/tests/testdata/TestExport/sub/child.gpt @@ -7,5 +7,9 @@ export: transient --- name: transient +the transient + --- name: miss + +the miss