From a66c716c0a43cb0f7952932be44eb19cb42e2cfc Mon Sep 17 00:00:00 2001 From: kiserufetch <38003008+kiserufetch@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:05:03 +0300 Subject: [PATCH 1/2] fix(tools): drop empty required entry from list_task_credentials schema list_task_credentials takes no parameters but built its JSON Schema with obj(map[string]any{}, ""), emitting {"type":"object","required":[""], "properties":{}}. The required array contains an empty string, which strict function-call validators reject. Observed with DeepSeek through an OpenAI-compatible gateway (format "openai"): openai: status 400 invalid_tool_spec - "function.parameters.required must be an array of non-empty strings" Because the tool is in the worker toolset, every worker run fails before the first tool call (terminal state model_error); the planner and main agent are unaffected since they don't load it. Anthropic/OpenAI accept the malformed schema silently, so it only surfaces on stricter providers. Pass no required args so obj omits the required key entirely: {"type":"object","properties":{}}. Co-Authored-By: Claude Opus 4.8 --- agent/tools_creds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/tools_creds.go b/agent/tools_creds.go index b8f4cd5..5b07df1 100644 --- a/agent/tools_creds.go +++ b/agent/tools_creds.go @@ -64,7 +64,7 @@ func (t *ToolSet) listTaskCredentials() actool.CoreTool { return writeTool( "list_task_credentials", "List this task's test accounts: user-declared accounts (e.g. program-provided aliases; a user-declared entry with an empty password means: register an account on that domain with a strong password, then record it via save_task_credential) plus accounts recorded by earlier runs. Check this BEFORE registering a new account, and use matching entries for authenticated testing.", - obj(map[string]any{}, ""), + obj(map[string]any{}), func(_ context.Context, _ json.RawMessage) (actool.Result, error) { if t.ts == nil { return actool.Errorf("list_task_credentials is not enabled: task store is not initialized"), nil From a44af3ca2b389688452f4c92ff7019416a375a0d Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:46:49 +0100 Subject: [PATCH 2/2] fix(tools): harden obj against blank required names and heal seeded list_task_credentials rows The previous commit stops list_task_credentials from building its schema with obj(map[string]any{}, ""). Two gaps remained: - obj still emitted whatever it was given, so the same typo on another tool would again produce "required": [""]. obj now drops names that are blank after strings.TrimSpace and omits the "required" key when nothing is left. - Tool schemas are cached in the tools table and SeedTool is insert-only, so the code fix only reached fresh installs. Existing databases kept the malformed row and strict providers kept rejecting every worker run. healListTaskCredentialsSchema runs at startup next to the other tool seeds and removes the "required" key from that one row, only when it is exactly [""]: UPDATE tools SET schema = schema - 'required', updated_at=now() WHERE key='list_task_credentials' AND schema->'required' = '[""]'::jsonb It is idempotent (a healed row no longer matches), needs no settings flag, and leaves the description, bindings, enabled flag and every other tool untouched. tool_schema_refresh_v7 is not bumped. Tests: TestObjDropsBlankRequired and TestListTaskCredentialsSchemaHasNoBlankRequired (agent), TestHealListTaskCredentialsSchema (server, Postgres). --- agent/toolcatalog_test.go | 41 ++++++++++++++++++ agent/tools.go | 12 +++++- server/orchestration.go | 14 ++++++ server/tools_wire_test.go | 90 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 2 deletions(-) diff --git a/agent/toolcatalog_test.go b/agent/toolcatalog_test.go index 1cfc3f7..0419ef8 100644 --- a/agent/toolcatalog_test.go +++ b/agent/toolcatalog_test.go @@ -3,6 +3,7 @@ package agent import ( "context" "encoding/json" + "strings" "testing" actool "github.com/Autumn-27/norma/tool" @@ -46,6 +47,46 @@ func TestBuiltinToolSeeds(t *testing.T) { } } +// TestObjDropsBlankRequired: obj never emits a blank required name, and omits the +// "required" key entirely when no real name is left. +func TestObjDropsBlankRequired(t *testing.T) { + if got, ok := obj(map[string]any{}, "")["required"]; ok { + t.Errorf(`obj(props, "") required = %v, want no required key`, got) + } + got, _ := obj(map[string]any{}, " ", "x")["required"].([]string) + if len(got) != 1 || got[0] != "x" { + t.Errorf(`obj(props, " ", "x") required = %v, want [x]`, got) + } +} + +// TestListTaskCredentialsSchemaHasNoBlankRequired guards the seeded schema of the +// parameterless list_task_credentials tool: "required": [""] is rejected by strict +// providers, which fails every worker run. +func TestListTaskCredentialsSchemaHasNoBlankRequired(t *testing.T) { + for _, s := range BuiltinToolSeeds() { + if s.Key != "list_task_credentials" { + continue + } + raw, err := json.Marshal(s.Schema) + if err != nil { + t.Fatal(err) + } + var schema struct { + Required []string `json:"required"` + } + if err := json.Unmarshal(raw, &schema); err != nil { + t.Fatalf("schema %s: %v", raw, err) + } + for _, r := range schema.Required { + if strings.TrimSpace(r) == "" { + t.Errorf("blank required entry in schema %s", raw) + } + } + return + } + t.Fatal("list_task_credentials not seeded") +} + // TestDecorateToolInjectsDefaults verifies a schema "default" fills a missing param // before the underlying handler runs, and an explicitly-provided value is kept. func TestDecorateToolInjectsDefaults(t *testing.T) { diff --git a/agent/tools.go b/agent/tools.go index 815e014..62c25b4 100644 --- a/agent/tools.go +++ b/agent/tools.go @@ -351,8 +351,16 @@ func pidList(raw []json.RawMessage) []int64 { func obj(props map[string]any, required ...string) map[string]any { m := map[string]any{"type": "object", "properties": props} - if len(required) > 0 { - m["required"] = required + // Blank names are dropped, and "required" is omitted when nothing is left: + // strict providers reject "required": [""] and fail every run carrying the tool. + req := make([]string, 0, len(required)) + for _, r := range required { + if strings.TrimSpace(r) != "" { + req = append(req, r) + } + } + if len(req) > 0 { + m["required"] = req } return m } diff --git a/server/orchestration.go b/server/orchestration.go index 49941bd..a43dc2d 100644 --- a/server/orchestration.go +++ b/server/orchestration.go @@ -503,6 +503,7 @@ func (s *Server) seedOrchestrationTools() { s.seedWorkerReadToolsUnbind() // list_facts/list_companies/list_worker_traces unbound from worker's defaults (one-time) s.seedWorkerReadbackRebind() // repairs an old migration's wrong removal: re-binds search_all_worker_traces/get_worker_trace/node_detail back onto worker (one-time) s.seedTaskCredentialBindings() // save/list_task_credential onto worker + pentest (one-time, #3) + s.healListTaskCredentialsSchema() s.seedAutoReportFindingBinding() s.unbindGoalMetDefault() s.reseedGoalsPrompt() // goals prompt gains the "extract operation constraints" step → old DBs append a new default version (one-time) @@ -910,6 +911,19 @@ func (s *Server) seedTaskCredentialBindings() { _ = s.m.pg.SetSetting(flag, "true") } +// healListTaskCredentialsSchema repairs the list_task_credentials row seeded by +// older builds, whose schema carried "required": [""] — strict providers reject +// it, which fails every worker run. SeedTool is first-insert-only, so the code fix +// alone never reaches an already-seeded row. Only that exact malformed value on +// that one key matches, and only the "required" key is dropped: the rest of the +// schema, the description, bindings and enabled flag are untouched. A healed row +// no longer matches, so this needs no settings flag and is a no-op afterwards. +func (s *Server) healListTaskCredentialsSchema() { + if _, err := s.m.pg.Exec(`UPDATE tools SET schema = schema - 'required', updated_at=now() WHERE key='list_task_credentials' AND schema->'required' = '[""]'::jsonb`); err != nil { + log.Printf("[tools] heal list_task_credentials schema failed: %v", err) + } +} + func (s *Server) seedWorkerReadbackRebind() { const flag = "worker_readback_rebind_v2" // v2: adds node_detail if v, _, _ := s.m.pg.GetSetting(flag); v == "true" { diff --git a/server/tools_wire_test.go b/server/tools_wire_test.go index dd4850e..3e00bfb 100644 --- a/server/tools_wire_test.go +++ b/server/tools_wire_test.go @@ -103,6 +103,96 @@ func TestWireTools(t *testing.T) { } } +// TestHealListTaskCredentialsSchema: a list_task_credentials row seeded by an older +// build carries "required": [""]. The startup heal drops that key, leaves the rest of +// the row alone, changes nothing on a second run, and touches no other tool. +func TestHealListTaskCredentialsSchema(t *testing.T) { + dsn, _, err := db.DSN() + if err != nil { + t.Skipf("no database config (%v) — skipping", err) + } + pg, err := db.Open(dsn) + if err != nil { + t.Skipf("postgres unavailable (%v) — skipping", err) + } + t.Cleanup(func() { pg.Close() }) + s := &Server{m: &Manager{pg: pg}} + + const key = "list_task_credentials" + // Other tools the heal must never touch: a normal required list, and the same + // malformed value under a different key. + const probeRequired, probeBlank = "heal_probe_required", "heal_probe_blank" + const broken = `{"type":"object","properties":{},"required":[""]}` + + // The package test DB is shared: put back whatever it held for the real key. + old, err := pg.GetTool(key) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if old == nil { + pg.Exec(`DELETE FROM tools WHERE key=$1`, key) + } else { + pg.Exec(`UPDATE tools SET schema=$2::jsonb WHERE key=$1`, key, string(old.Schema)) + } + pg.Exec(`DELETE FROM tools WHERE key IN ($1,$2)`, probeRequired, probeBlank) + }) + for k, schema := range map[string]string{ + key: broken, + probeRequired: `{"type":"object","properties":{"x":{"type":"string"}},"required":["x"]}`, + probeBlank: broken, + } { + if _, err := pg.Exec(`INSERT INTO tools(key, description, schema, agents) VALUES ($1, 'legacy row', $2::jsonb, '["worker"]') +ON CONFLICT (key) DO UPDATE SET schema=EXCLUDED.schema`, k, schema); err != nil { + t.Fatal(err) + } + } + + type row struct { + schema, description, agents, updatedAt string + enabled bool + } + read := func(k string) row { + t.Helper() + var r row + if err := pg.QueryRow(`SELECT schema::text, description, agents::text, updated_at::text, enabled FROM tools WHERE key=$1`, k). + Scan(&r.schema, &r.description, &r.agents, &r.updatedAt, &r.enabled); err != nil { + t.Fatalf("read %s: %v", k, err) + } + return r + } + before, beforeRequired, beforeBlank := read(key), read(probeRequired), read(probeBlank) + + s.healListTaskCredentialsSchema() + + healed := read(key) + var schema map[string]any + if err := json.Unmarshal([]byte(healed.schema), &schema); err != nil { + t.Fatal(err) + } + if _, ok := schema["required"]; ok { + t.Fatalf("required survived the heal: %s", healed.schema) + } + if schema["type"] != "object" || schema["properties"] == nil { + t.Errorf("heal dropped more than required: %s", healed.schema) + } + if healed.description != before.description || healed.agents != before.agents || healed.enabled != before.enabled { + t.Errorf("heal changed description/bindings/enabled: %+v -> %+v", before, healed) + } + + s.healListTaskCredentialsSchema() + + if again := read(key); again != healed { + t.Errorf("second heal changed the row: %+v -> %+v", healed, again) + } + if got := read(probeRequired); got != beforeRequired { + t.Errorf("heal touched a tool with a non-empty required: %+v -> %+v", beforeRequired, got) + } + if got := read(probeBlank); got != beforeBlank { + t.Errorf("heal touched another tool key: %+v -> %+v", beforeBlank, got) + } +} + func mustJSON(v any) json.RawMessage { b, _ := json.Marshal(v) return b