quirks: only reject shell calls with tools in command position
The old word-boundary check falsely triggered when tool names appeared in arguments (e.g., git commit messages mentioning native tools). New logic: split on shell separators and only flag when a tool is the first token of a sub-command or a path ending with /toolname.
This commit is contained in:
parent
89dd021a93
commit
fa219e06aa
|
|
@ -3,49 +3,102 @@ package quirks
|
||||||
|
|
||||||
import "strings"
|
import "strings"
|
||||||
|
|
||||||
// ShellInvokesNativeTool checks if a shell command contains a native tool name.
|
// ShellInvokesNativeTool checks if a shell command attempts to execute a native
|
||||||
// Returns the tool name if found, empty string otherwise.
|
// tool. Returns the tool name if found, empty string otherwise.
|
||||||
//
|
//
|
||||||
// This is a hack for broken models that ignore prompt instructions and shell
|
// Only matches when the tool name appears in command position:
|
||||||
// out to tools instead of calling them natively.
|
// - First word of the command
|
||||||
|
// - After a command separator (; | && ||)
|
||||||
|
// - As a path ending with the tool name (./tool, /usr/bin/tool)
|
||||||
|
//
|
||||||
|
// Does NOT match tool names that merely appear in arguments (e.g., commit messages).
|
||||||
func ShellInvokesNativeTool(cmd string, tools []string) string {
|
func ShellInvokesNativeTool(cmd string, tools []string) string {
|
||||||
// Reject ollie-9p in shell — use client_9p native tool instead.
|
// Reject ollie-9p in command position.
|
||||||
if strings.Contains(cmd, "ollie-9p") {
|
if isInCommandPosition(cmd, "ollie-9p") {
|
||||||
return "client_9p"
|
return "client_9p"
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if cmd contains any tool name as a word boundary match.
|
|
||||||
// Match: "skill_list", "echo foo | skill_list", "skill_list arg"
|
|
||||||
// Don't match: "my_skill_list" (substring)
|
|
||||||
for _, name := range tools {
|
for _, name := range tools {
|
||||||
if name == "shell" {
|
if name == "shell" {
|
||||||
continue // don't match shell itself
|
continue
|
||||||
}
|
}
|
||||||
if matchesWordBoundary(cmd, name) {
|
if isInCommandPosition(cmd, name) {
|
||||||
return name
|
return name
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// matchesWordBoundary checks if name appears in cmd at word boundaries.
|
// isInCommandPosition returns true if name appears as an executable in cmd.
|
||||||
func matchesWordBoundary(cmd, name string) bool {
|
// It checks whether name is the first token of a (sub-)command or appears as a
|
||||||
idx := strings.Index(cmd, name)
|
// path ending with /name.
|
||||||
if idx < 0 {
|
func isInCommandPosition(cmd, name string) bool {
|
||||||
return false
|
// Check each sub-command (split on shell separators).
|
||||||
|
for _, sub := range splitCommands(cmd) {
|
||||||
|
sub = strings.TrimSpace(sub)
|
||||||
|
if sub == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// Get the first token (the command being executed).
|
||||||
|
first := firstToken(sub)
|
||||||
|
// Exact match: "tool_name args..."
|
||||||
|
if first == name {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
// Path match: "./tool_name", "../tool_name", "/usr/bin/tool_name"
|
||||||
|
if strings.HasSuffix(first, "/"+name) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
// Check left boundary: start of string or non-word char
|
return false
|
||||||
if idx > 0 && isWordChar(cmd[idx-1]) {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
// Check right boundary: end of string or non-word char
|
|
||||||
end := idx + len(name)
|
|
||||||
if end < len(cmd) && isWordChar(cmd[end]) {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
return true
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func isWordChar(c byte) bool {
|
// splitCommands splits a shell command line on command separators: ;, |, &&, ||
|
||||||
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_'
|
// This is a rough heuristic — it doesn't handle quoting, but it's good enough
|
||||||
|
// to identify command positions in typical AI-generated shell commands.
|
||||||
|
func splitCommands(cmd string) []string {
|
||||||
|
var parts []string
|
||||||
|
var current strings.Builder
|
||||||
|
i := 0
|
||||||
|
for i < len(cmd) {
|
||||||
|
switch {
|
||||||
|
case cmd[i] == ';' || cmd[i] == '|':
|
||||||
|
parts = append(parts, current.String())
|
||||||
|
current.Reset()
|
||||||
|
if cmd[i] == '|' && i+1 < len(cmd) && cmd[i+1] == '|' {
|
||||||
|
i++ // skip ||
|
||||||
|
}
|
||||||
|
i++
|
||||||
|
case cmd[i] == '&' && i+1 < len(cmd) && cmd[i+1] == '&':
|
||||||
|
parts = append(parts, current.String())
|
||||||
|
current.Reset()
|
||||||
|
i += 2
|
||||||
|
case cmd[i] == '$' && i+1 < len(cmd) && cmd[i+1] == '(':
|
||||||
|
// Command substitution — the content after $( is a new command
|
||||||
|
parts = append(parts, current.String())
|
||||||
|
current.Reset()
|
||||||
|
i += 2
|
||||||
|
case cmd[i] == '`':
|
||||||
|
// Backtick substitution
|
||||||
|
parts = append(parts, current.String())
|
||||||
|
current.Reset()
|
||||||
|
i++
|
||||||
|
default:
|
||||||
|
current.WriteByte(cmd[i])
|
||||||
|
i++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if current.Len() > 0 {
|
||||||
|
parts = append(parts, current.String())
|
||||||
|
}
|
||||||
|
return parts
|
||||||
|
}
|
||||||
|
|
||||||
|
// firstToken returns the first whitespace-delimited token from s.
|
||||||
|
func firstToken(s string) string {
|
||||||
|
s = strings.TrimSpace(s)
|
||||||
|
if i := strings.IndexAny(s, " \t"); i >= 0 {
|
||||||
|
return s[:i]
|
||||||
|
}
|
||||||
|
return s
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,56 @@
|
||||||
|
package quirks
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
func TestShellInvokesNativeTool(t *testing.T) {
|
||||||
|
tools := []string{"shell", "client_9p", "file_read", "file_edit", "skill_list", "memory_wake"}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
cmd string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
// Should match: tool in command position
|
||||||
|
{"client_9p read foo", "client_9p"},
|
||||||
|
{"file_read path=/tmp/x", "file_read"},
|
||||||
|
{"skill_list", "skill_list"},
|
||||||
|
{"echo hello | skill_list", "skill_list"},
|
||||||
|
{"echo hello && file_read foo", "file_read"},
|
||||||
|
{"echo hello || file_edit bar", "file_edit"},
|
||||||
|
{"echo hello; memory_wake", "memory_wake"},
|
||||||
|
{"./client_9p read foo", "client_9p"},
|
||||||
|
{"/usr/local/bin/client_9p read", "client_9p"},
|
||||||
|
{"../bin/file_read foo", "file_read"},
|
||||||
|
{"$(file_read foo)", "file_read"},
|
||||||
|
{"`skill_list`", "skill_list"},
|
||||||
|
|
||||||
|
// Should match: ollie-9p -> client_9p
|
||||||
|
{"ollie-9p read session/foo", "client_9p"},
|
||||||
|
{"echo x | ollie-9p write foo", "client_9p"},
|
||||||
|
|
||||||
|
// Should NOT match: tool name in arguments (not command position)
|
||||||
|
{"git commit -m 'use client_9p interface'", ""},
|
||||||
|
{"echo 'client_9p is great'", ""},
|
||||||
|
{"git commit -m 'add file_read support'", ""},
|
||||||
|
{"grep skill_list README.md", ""},
|
||||||
|
{"cat file_read.go", ""},
|
||||||
|
{"echo ollie-9p is the CLI", ""},
|
||||||
|
|
||||||
|
// Should NOT match: substring
|
||||||
|
{"my_client_9p_wrapper foo", ""},
|
||||||
|
|
||||||
|
// Should not match shell itself
|
||||||
|
{"shell command", ""},
|
||||||
|
|
||||||
|
// Edge cases
|
||||||
|
{"", ""},
|
||||||
|
{"ls -la", ""},
|
||||||
|
{"go build ./...", ""},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
got := ShellInvokesNativeTool(tt.cmd, tools)
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("ShellInvokesNativeTool(%q) = %q, want %q", tt.cmd, got, tt.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue