diff --git a/pkg/functions/parse.go b/pkg/functions/parse.go index 9f6fe6b5e..d6be3b0f5 100644 --- a/pkg/functions/parse.go +++ b/pkg/functions/parse.go @@ -628,6 +628,36 @@ func buildContent(before string, parser *ChatMsgParser) string { // This provides better streaming and partial parsing support. // When format is nil or when format is set, tries "find scope/tool start, split, parse suffix" // first (llama.cpp PEG order) so that content before the tool block does not cause parse failure. +// validToolNameRe matches a plausible function name. OpenAI tool names are +// limited to letters, digits, underscores and hyphens; dots appear in some +// providers' namespaced names. Anything else (whitespace, braces, brackets, +// quotes, colons) signals the XML auto-detector grabbed a JSON blob or prose +// rather than a real name. +var validToolNameRe = regexp.MustCompile(`^[A-Za-z0-9_.\-]+$`) + +// plausibleToolName reports whether name looks like a real function name. +func plausibleToolName(name string) bool { + return validToolNameRe.MatchString(strings.TrimSpace(name)) +} + +// filterPlausibleToolCalls drops auto-detected tool calls whose name is not a +// plausible function name. This guards against a format (notably glm-4.5, whose +// tool block is name...) mis-claiming a Hermes-style +// JSON block and returning the whole JSON object — or +// any leading prose / array — as the function name. Dropping the misparse lets +// auto-detection fall through to the next format and ultimately to JSON +// parsing, which handles Hermes correctly. Replaces the narrower leading-"{" +// check (PR #9940); see issue #9722. +func filterPlausibleToolCalls(calls []FuncCallResults) []FuncCallResults { + out := calls[:0:0] + for _, c := range calls { + if plausibleToolName(c.Name) { + out = append(out, c) + } + } + return out +} + func ParseXMLIterative(s string, format *XMLToolCallFormat, isPartial bool) ([]FuncCallResults, error) { // Try split-on-scope first so reasoning/content before tool block is skipped if format != nil { @@ -639,7 +669,12 @@ func ParseXMLIterative(s string, format *XMLToolCallFormat, isPartial bool) ([]F for _, fmtPreset := range formats { if fmtPreset.format != nil { if pr, ok := tryParseXMLFromScopeStart(s, fmtPreset.format, isPartial); ok { - return pr.ToolCalls, nil + // Auto-detect: discard misparsed (non-name) results so a + // format that grabbed a JSON blob doesn't win; fall through + // to the next format. + if valid := filterPlausibleToolCalls(pr.ToolCalls); len(valid) > 0 { + return valid, nil + } } } } @@ -659,14 +694,19 @@ func ParseXMLIterative(s string, format *XMLToolCallFormat, isPartial bool) ([]F if err != nil { // Check if it's a partial exception (recoverable) if _, ok := err.(*ChatMsgPartialException); ok { - // Partial parse, return what we have - return parser.ToolCalls(), nil + // Partial parse, return what we have — unless every + // result is a misparse, in which case try the next format. + if valid := filterPlausibleToolCalls(parser.ToolCalls()); len(valid) > 0 { + return valid, nil + } } // Try next format continue } if success && len(parser.ToolCalls()) > 0 { - return parser.ToolCalls(), nil + if valid := filterPlausibleToolCalls(parser.ToolCalls()); len(valid) > 0 { + return valid, nil + } } } } diff --git a/pkg/functions/parse_glm_9722_test.go b/pkg/functions/parse_glm_9722_test.go new file mode 100644 index 000000000..fa8eecd4b --- /dev/null +++ b/pkg/functions/parse_glm_9722_test.go @@ -0,0 +1,56 @@ +package functions + +import ( + "regexp" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Robust fix for the glm-4.5 XML auto-detect false positive (relates to #9722 +// / supersedes the brittle leading-"{" filter in #9940). When the XML +// auto-detector mis-identifies a Hermes-style JSON block +// as glm-4.5, it extracts the block body as the function NAME. A real function +// name is [A-Za-z0-9_.-]+; anything with braces, brackets, whitespace, quotes +// or colons is a misparse and must not be returned (so JSON parsing can take +// over). This is stronger than checking only for a leading "{": it also rejects +// leading prose, JSON arrays, and brace-less garbage. +var _ = Describe("glm-4.5 auto-detect name validation (#9722/#9940)", func() { + // plausibleName mirrors the contract: a returned auto-detected tool name + // must look like a real function name. + plausible := regexp.MustCompile(`^[A-Za-z0-9_.\-]+$`) + + DescribeTable("auto-detect must not emit a misparsed tool name", + func(input string) { + results, err := ParseXMLIterative(input, nil, false) + Expect(err).ToNot(HaveOccurred()) + for _, r := range results { + Expect(plausible.MatchString(r.Name)).To(BeTrue(), + "auto-detected XML tool name must look like a function name, got: %q", r.Name) + } + }, + Entry("canonical Hermes JSON", "\n{\"name\": \"bash\", \"arguments\": {\"script\": \"ls\"}}\n"), + Entry("leading prose then JSON", "\nSure: {\"name\": \"bash\", \"arguments\": {\"script\": \"ls\"}}\n"), + Entry("JSON array (parallel calls)", "\n[{\"name\": \"bash\", \"arguments\": {}}]\n"), + Entry("brace-less garbage", "\nname: bash, arguments: {}\n"), + ) + + // No-regression: a genuine glm-4.5 tool call must still be auto-detected. + It("still parses a legitimate glm-4.5 tool call", func() { + legit := "get_weather\ncity\nNYC\n" + results, err := ParseXMLIterative(legit, nil, false) + Expect(err).ToNot(HaveOccurred()) + Expect(results).To(HaveLen(1)) + Expect(results[0].Name).To(Equal("get_weather")) + }) + + // A user who explicitly forces the glm-4.5 format keeps the raw behaviour + // (no name filtering) — only auto-detection is guarded. + It("does not filter when the glm-4.5 format is explicitly forced", func() { + input := "\n{\"name\": \"bash\", \"arguments\": {}}\n" + forced, err := ParseXMLIterative(input, GetXMLFormatPreset("glm-4.5"), false) + Expect(err).ToNot(HaveOccurred()) + Expect(forced).ToNot(BeEmpty(), + "explicit format must be trusted verbatim, even if it yields a JSON-blob name") + }) +})