Skip to content

Commit 32e97ad

Browse files
Patel230claude
Patel230
and
claude
committed
feat: adopt QwenLM engineering patterns (MCP schema pruning, inline tool calls, read-only validator)
Patterns ported from a deep-research scan of the QwenLM org (Qwen-Agent / qwen-code, both Apache-2.0); no code copied. Four changes: - engine: route Hermes/Nous <tool_call> inline tool calls through ParseInlineToolCalls (the eyrie side lands in eyrie fd2d49b), so Qwen and OpenAI-compatible local models that inline tool calls are parsed. - mcp: SanitizeToolSchema prunes MCP tool JSON Schema to the {type,properties,required} subset strict function-calling endpoints accept; wired into PluginToolAdapter.Parameters() (was passing the raw upstream schema straight to the model). Transports already covered stdio/HTTP/SSE/WS, so only schema pruning was missing. - multiagent: add a read-only validation worker (readOnlyWorkerTools + ReadOnlyValidationWorker) and a `validator` built-in persona with an enforceable ReadOnly contract (no Write/Edit/Bash) — the implement-then-validate pair where the agent that writes the code is not the one that signs off on it. - personas: stop pinning built-in personas to specific model names (reviewer/architect/speed/critic/security-reviewer/verifier); they now inherit the session model so a name absent on the user's provider can't break them. Bumps external/eyrie -> fd2d49b and external/yaad -> e9d7df3. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent eb1c8cb commit 32e97ad

10 files changed

Lines changed: 411 additions & 28 deletions

File tree

‎external/eyrie‎

‎external/yaad‎

‎internal/engine/stream.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,8 +499,10 @@ func (s *Session) agentLoop(ctx context.Context, ch chan<- StreamEvent) {
499499
return
500500
}
501501

502-
// Check for inline tool calls in text (some providers embed tool calls in text)
503-
if len(toolCalls) == 0 && strings.Contains(textContent.String(), "<|tool_calls_section_begin|>") {
502+
// Check for inline tool calls in text (some providers embed tool calls in
503+
// text): Moonshot/kimi <|tool_calls_section_begin|> or Hermes/Nous
504+
// <tool_call> (Qwen and most OpenAI-compatible local models).
505+
if len(toolCalls) == 0 && (strings.Contains(textContent.String(), "<|tool_calls_section_begin|>") || strings.Contains(textContent.String(), "<tool_call>")) {
504506
cleanText, inlineCalls := types.ParseInlineToolCalls(textContent.String())
505507
if len(inlineCalls) > 0 {
506508
textContent.Reset()

‎internal/mcp/schema.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package mcp
2+
3+
// SanitizeToolSchema prunes an MCP tool's JSON Schema down to the subset that
4+
// strict OpenAI-compatible function-calling endpoints accept. Many MCP servers
5+
// publish full JSON Schema documents (with "$schema", "title", "definitions",
6+
// "additionalProperties", "examples", etc.); several providers (and some local
7+
// vLLM/SGLang routes) reject a function's parameter schema outright when it
8+
// carries keys outside the function-calling subset. Qwen-Agent solves this by
9+
// keeping only {type, properties, required} per tool — hawk does the same.
10+
//
11+
// The function is conservative: it never invents structure. If the input does
12+
// not look like an object schema (no "type":"object" and no "properties"), it is
13+
// returned unchanged so non-object tools (rare, but valid) are not corrupted. A
14+
// nil input yields the canonical empty-object schema so the model still sees a
15+
// well-formed parameter block.
16+
func SanitizeToolSchema(schema map[string]interface{}) map[string]interface{} {
17+
if schema == nil {
18+
return map[string]interface{}{
19+
"type": "object",
20+
"properties": map[string]interface{}{},
21+
}
22+
}
23+
24+
_, hasProps := schema["properties"]
25+
if schema["type"] != "object" && !hasProps {
26+
// Not an object schema — leave it alone.
27+
return schema
28+
}
29+
30+
out := map[string]interface{}{"type": "object"}
31+
32+
if props, ok := schema["properties"].(map[string]interface{}); ok {
33+
out["properties"] = sanitizeProperties(props)
34+
} else {
35+
out["properties"] = map[string]interface{}{}
36+
}
37+
38+
// "required" must be a list of strings; copy it only when well-formed and
39+
// non-empty (an empty required list is the default and adds nothing).
40+
if req, ok := schema["required"].([]interface{}); ok && len(req) > 0 {
41+
out["required"] = req
42+
}
43+
44+
return out
45+
}
46+
47+
// allowedPropKeys is the per-property keyword subset preserved during pruning.
48+
// These are the keywords function-calling endpoints actually consume to build a
49+
// prompt-time tool description; everything else (validation-only or
50+
// documentation-only keywords) is dropped.
51+
var allowedPropKeys = map[string]struct{}{
52+
"type": {},
53+
"description": {},
54+
"enum": {},
55+
"items": {},
56+
"properties": {},
57+
"required": {},
58+
"default": {},
59+
}
60+
61+
// sanitizeProperties prunes each property to allowedPropKeys, recursing into
62+
// nested object/array schemas so the subset is applied at every depth.
63+
func sanitizeProperties(props map[string]interface{}) map[string]interface{} {
64+
cleaned := make(map[string]interface{}, len(props))
65+
for name, raw := range props {
66+
spec, ok := raw.(map[string]interface{})
67+
if !ok {
68+
cleaned[name] = raw
69+
continue
70+
}
71+
cp := make(map[string]interface{}, len(spec))
72+
for k, v := range spec {
73+
if _, allow := allowedPropKeys[k]; !allow {
74+
continue
75+
}
76+
switch k {
77+
case "properties":
78+
if nested, ok := v.(map[string]interface{}); ok {
79+
cp[k] = sanitizeProperties(nested)
80+
continue
81+
}
82+
case "items":
83+
if item, ok := v.(map[string]interface{}); ok {
84+
cp[k] = sanitizeItems(item)
85+
continue
86+
}
87+
}
88+
cp[k] = v
89+
}
90+
cleaned[name] = cp
91+
}
92+
return cleaned
93+
}
94+
95+
// sanitizeItems prunes an array "items" sub-schema, recursing into its own
96+
// properties when it describes objects.
97+
func sanitizeItems(item map[string]interface{}) map[string]interface{} {
98+
cp := make(map[string]interface{}, len(item))
99+
for k, v := range item {
100+
if _, allow := allowedPropKeys[k]; !allow {
101+
continue
102+
}
103+
if k == "properties" {
104+
if nested, ok := v.(map[string]interface{}); ok {
105+
cp[k] = sanitizeProperties(nested)
106+
continue
107+
}
108+
}
109+
cp[k] = v
110+
}
111+
return cp
112+
}

‎internal/mcp/schema_test.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package mcp
2+
3+
import "testing"
4+
5+
func TestSanitizeToolSchema_PrunesTopLevelKeys(t *testing.T) {
6+
in := map[string]interface{}{
7+
"$schema": "https://json-schema.org/draft/2020-12/schema",
8+
"title": "DoThing",
9+
"description": "does a thing",
10+
"additionalProperties": false,
11+
"type": "object",
12+
"properties": map[string]interface{}{
13+
"path": map[string]interface{}{"type": "string", "description": "a path"},
14+
},
15+
"required": []interface{}{"path"},
16+
}
17+
out := SanitizeToolSchema(in)
18+
19+
for _, banned := range []string{"$schema", "title", "additionalProperties", "description"} {
20+
if _, ok := out[banned]; ok {
21+
t.Errorf("expected %q to be pruned, got %v", banned, out[banned])
22+
}
23+
}
24+
if out["type"] != "object" {
25+
t.Errorf("type = %v, want object", out["type"])
26+
}
27+
if _, ok := out["properties"].(map[string]interface{})["path"]; !ok {
28+
t.Errorf("properties.path dropped: %v", out["properties"])
29+
}
30+
req, ok := out["required"].([]interface{})
31+
if !ok || len(req) != 1 || req[0] != "path" {
32+
t.Errorf("required = %v, want [path]", out["required"])
33+
}
34+
}
35+
36+
func TestSanitizeToolSchema_PrunesPerPropertyKeys(t *testing.T) {
37+
in := map[string]interface{}{
38+
"type": "object",
39+
"properties": map[string]interface{}{
40+
"q": map[string]interface{}{
41+
"type": "string",
42+
"description": "query",
43+
"minLength": 1, // validation-only — should be dropped
44+
"examples": []any{}, // doc-only — should be dropped
45+
"enum": []interface{}{"a", "b"},
46+
},
47+
},
48+
}
49+
out := SanitizeToolSchema(in)
50+
prop := out["properties"].(map[string]interface{})["q"].(map[string]interface{})
51+
if _, ok := prop["minLength"]; ok {
52+
t.Error("minLength should be pruned")
53+
}
54+
if _, ok := prop["examples"]; ok {
55+
t.Error("examples should be pruned")
56+
}
57+
if prop["type"] != "string" || prop["description"] != "query" {
58+
t.Errorf("kept keys lost: %v", prop)
59+
}
60+
if _, ok := prop["enum"]; !ok {
61+
t.Error("enum should be kept")
62+
}
63+
}
64+
65+
func TestSanitizeToolSchema_RecursesNestedAndItems(t *testing.T) {
66+
in := map[string]interface{}{
67+
"type": "object",
68+
"properties": map[string]interface{}{
69+
"filter": map[string]interface{}{
70+
"type": "object",
71+
"title": "Filter", // dropped at nested level
72+
"properties": map[string]interface{}{
73+
"tags": map[string]interface{}{
74+
"type": "array",
75+
"items": map[string]interface{}{"type": "string", "pattern": "x"}, // pattern dropped
76+
},
77+
},
78+
},
79+
},
80+
}
81+
out := SanitizeToolSchema(in)
82+
filter := out["properties"].(map[string]interface{})["filter"].(map[string]interface{})
83+
if _, ok := filter["title"]; ok {
84+
t.Error("nested title should be pruned")
85+
}
86+
tags := filter["properties"].(map[string]interface{})["tags"].(map[string]interface{})
87+
items := tags["items"].(map[string]interface{})
88+
if _, ok := items["pattern"]; ok {
89+
t.Error("items.pattern should be pruned")
90+
}
91+
if items["type"] != "string" {
92+
t.Errorf("items.type lost: %v", items)
93+
}
94+
}
95+
96+
func TestSanitizeToolSchema_NilYieldsEmptyObject(t *testing.T) {
97+
out := SanitizeToolSchema(nil)
98+
if out["type"] != "object" {
99+
t.Fatalf("nil schema = %v, want empty object schema", out)
100+
}
101+
if _, ok := out["properties"].(map[string]interface{}); !ok {
102+
t.Errorf("expected empty properties map, got %v", out["properties"])
103+
}
104+
}
105+
106+
func TestSanitizeToolSchema_NonObjectLeftAlone(t *testing.T) {
107+
in := map[string]interface{}{"type": "string", "minLength": 3}
108+
out := SanitizeToolSchema(in)
109+
if out["minLength"] != 3 {
110+
t.Errorf("non-object schema should be returned unchanged, got %v", out)
111+
}
112+
}

‎internal/multiagent/agents/persona.go‎

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,19 @@ import (
1717
// Persona represents an enhanced agent definition with specific skills,
1818
// model preferences, and behavioral configuration.
1919
type Persona struct {
20-
Name string `json:"name"`
21-
Description string `json:"description"`
22-
Model string `json:"model"`
23-
Provider string `json:"provider"`
24-
SystemPrompt string `json:"system_prompt"`
25-
Tools []string `json:"tools"`
26-
ExcludedTools []string `json:"excluded_tools"`
20+
Name string `json:"name"`
21+
Description string `json:"description"`
22+
Model string `json:"model"`
23+
Provider string `json:"provider"`
24+
SystemPrompt string `json:"system_prompt"`
25+
Tools []string `json:"tools"`
26+
ExcludedTools []string `json:"excluded_tools"`
27+
// ReadOnly declares that this persona must never mutate the workspace. It is
28+
// a machine-readable contract (stronger than listing Edit/Write in
29+
// ExcludedTools, since it also forbids mutation via Bash): orchestrators
30+
// should run a ReadOnly persona with a read-only tool registry. Used by the
31+
// validation half of an implement-then-validate agent pair.
32+
ReadOnly bool `json:"read_only,omitempty"`
2733
Temperature float64 `json:"temperature"`
2834
MaxTokens int `json:"max_tokens"`
2935
Expertise []string `json:"expertise"`
@@ -408,6 +414,8 @@ func parsePersonaContent(content, path string) (*Persona, error) {
408414
p.Tools = parseYAMLList(val)
409415
case "excluded_tools":
410416
p.ExcludedTools = parseYAMLList(val)
417+
case "read_only":
418+
p.ReadOnly = val == "true" || val == "yes"
411419
case "rules":
412420
p.Rules = parseYAMLList(val)
413421
case "created_at":
@@ -481,6 +489,9 @@ func RenderPersonaFile(persona *Persona) string {
481489
if len(persona.ExcludedTools) > 0 {
482490
sb.WriteString(fmt.Sprintf("excluded_tools: [%s]\n", strings.Join(persona.ExcludedTools, ", ")))
483491
}
492+
if persona.ReadOnly {
493+
sb.WriteString("read_only: true\n")
494+
}
484495
if persona.Color != "" {
485496
sb.WriteString(fmt.Sprintf("color: %s\n", persona.Color))
486497
}
@@ -556,7 +567,7 @@ func BuiltinPersonas() []*Persona {
556567
{
557568
Name: "reviewer",
558569
Description: "Security and correctness focused code reviewer",
559-
Model: "claude-sonnet-4-6",
570+
Model: "", // inherit session model (was claude-sonnet-4-6)
560571
Temperature: 0.2,
561572
Expertise: []string{"security", "backend", "testing"},
562573
CommunicationStyle: "concise",
@@ -574,7 +585,7 @@ func BuiltinPersonas() []*Persona {
574585
{
575586
Name: "architect",
576587
Description: "High-level system design with minimal code",
577-
Model: "claude-opus-4-6",
588+
Model: "", // inherit session model (was claude-opus-4-6)
578589
Temperature: 0.7,
579590
MaxTokens: 16384,
580591
Expertise: []string{"backend", "devops"},
@@ -634,7 +645,7 @@ func BuiltinPersonas() []*Persona {
634645
{
635646
Name: "speed",
636647
Description: "Fast and concise, uses cheapest model",
637-
Model: "claude-haiku-3-5",
648+
Model: "", // inherit session model (was claude-haiku-3-5)
638649
Temperature: 0.3,
639650
MaxTokens: 4096,
640651
Expertise: []string{"backend", "frontend"},
@@ -684,7 +695,7 @@ func BuiltinPersonas() []*Persona {
684695
{
685696
Name: "critic",
686697
Description: "Reviews plans and code for flaws before commitment",
687-
Model: "claude-sonnet-4-6",
698+
Model: "", // inherit session model (was claude-sonnet-4-6)
688699
Temperature: 0.2,
689700
Expertise: []string{"backend", "testing", "security"},
690701
CommunicationStyle: "concise",
@@ -701,7 +712,7 @@ func BuiltinPersonas() []*Persona {
701712
{
702713
Name: "security-reviewer",
703714
Description: "Deep security-focused code reviewer",
704-
Model: "claude-sonnet-4-6",
715+
Model: "", // inherit session model (was claude-sonnet-4-6)
705716
Temperature: 0.2,
706717
MaxTokens: 8192,
707718
Expertise: []string{"security", "backend"},
@@ -751,7 +762,7 @@ func BuiltinPersonas() []*Persona {
751762
{
752763
Name: "verifier",
753764
Description: "Validates implementations against specifications",
754-
Model: "claude-sonnet-4-6",
765+
Model: "", // inherit session model (was claude-sonnet-4-6)
755766
Temperature: 0.2,
756767
Expertise: []string{"testing", "backend"},
757768
CommunicationStyle: "concise",
@@ -765,6 +776,34 @@ func BuiltinPersonas() []*Persona {
765776
},
766777
CreatedAt: now,
767778
},
779+
{
780+
// validator is the read-only half of an implement-then-validate
781+
// agent pair: a separate agent reviews the implementation worker's
782+
// output without the ability to change it. Unlike verifier it is
783+
// ReadOnly (no Bash), so its sign-off cannot be tainted by mutating
784+
// the very code it judges.
785+
Name: "validator",
786+
Description: "Read-only validator of an implementation it did not write",
787+
// Model intentionally left empty: a validator should run on whatever
788+
// model the user has configured for the session rather than pinning a
789+
// specific name that may not exist on their provider. (Several
790+
// built-ins pin claude-sonnet-4-6; this one deliberately inherits.)
791+
Model: "",
792+
Temperature: 0.1,
793+
Expertise: []string{"testing", "backend", "security"},
794+
CommunicationStyle: "concise",
795+
ReadOnly: true,
796+
Tools: []string{"Read", "Grep", "Glob", "LS"},
797+
ExcludedTools: []string{"Edit", "Write", "Bash"},
798+
SystemPrompt: "You are a read-only validation agent. You did not write the code under review and you cannot modify it. Inspect the implementation against the stated expected behavior and report, per acceptance criterion, a concrete PASS or FAIL with file:line evidence. Never assume — cite what you actually read.",
799+
Rules: []string{
800+
"You are read-only: never propose to edit, write, or run shell commands",
801+
"Cite file:line evidence for every PASS or FAIL",
802+
"Judge against the expected behavior, not your own preferences",
803+
"Report partial or unclear completion honestly rather than rounding up",
804+
},
805+
CreatedAt: now,
806+
},
768807
{
769808
Name: "integrator",
770809
Description: "Handles merges, integration, and compatibility",

‎internal/multiagent/agents/persona_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,7 @@ func TestBuiltinPersonas_AreValid(t *testing.T) {
470470
"test-engineer": false,
471471
"tracer": false,
472472
"verifier": false,
473+
"validator": false,
473474
"integrator": false,
474475
"documenter": false,
475476
"devops": false,
@@ -549,8 +550,8 @@ func TestSelectPersona_NewDomains(t *testing.T) {
549550
}
550551

551552
func TestBuiltinPersonas_Count(t *testing.T) {
552-
if got := len(BuiltinPersonas()); got != 21 {
553-
t.Errorf("expected 21 built-in personas, got %d", got)
553+
if got := len(BuiltinPersonas()); got != 22 {
554+
t.Errorf("expected 22 built-in personas, got %d", got)
554555
}
555556
}
556557

0 commit comments

Comments
 (0)