From 337a0346013402eda33b3d506a7632d20d58f5f2 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Tue, 3 Nov 2026 14:45:42 +0000 Subject: [PATCH] feat(addons): declare document slots and legacy arguments in the manifest The four document skill names were code in the contract, the hint for binary files kept its own list of formats and libraries, and the chat boundary rewrote the old arguments of one add-on by its ID. A skill that shadows a name can now declare the slot it serves: kind, file extensions, name words and a library hint. A tool can declare legacyArgs, the arguments it refuses now, each with a hint. Both need manifest api 2. The four default slots move to a data file that the contract reads, so a manifest that declares nothing keeps working. The host takes the replaceable names, the files a skill covers and the hint for a script from the skills the add-ons list. It compares no format or add-on ID. A guard spec fails when it does again. The Office manifest declares its slots and legacy arguments and accepts api 2. Assisted-by: Claude:claude-sonnet-5-5 go gofmt --- addons/office/manifest.json | 77 +++++++++-- core/application/addon_gateway.go | 2 +- core/services/addons/tools/adapter.go | 9 +- ...ce_creation_test.go => legacyargs_test.go} | 23 +++- core/services/addons/tools/office_creation.go | 44 ------- core/services/studio/addonskills.go | 66 +++++++++- .../studio/addonskills_internal_test.go | 16 +++ core/services/studio/binaryhint.go | 48 ++++--- .../studio/binaryhint_internal_test.go | 15 +++ core/services/studio/loadskill.go | 6 +- core/services/studio/noaddonid_test.go | 17 ++- core/services/studio/toolsadmin.go | 3 +- docs/content/features/addons.md | 39 +++++- pkg/addons/contract/execution.go | 8 +- pkg/addons/contract/legacyargs.go | 79 ++++++++++++ pkg/addons/contract/manifest.go | 8 ++ pkg/addons/contract/skills.go | 45 +++++-- pkg/addons/contract/slots.go | 122 ++++++++++++++++++ pkg/addons/contract/slots.json | 6 + pkg/addons/contract/slots_test.go | 101 +++++++++++++++ pkg/addons/contract/tooldefs.go | 7 + scripts/addons/suites.json | 11 +- 22 files changed, 648 insertions(+), 104 deletions(-) rename core/services/addons/tools/{office_creation_test.go => legacyargs_test.go} (52%) delete mode 100644 core/services/addons/tools/office_creation.go create mode 100644 pkg/addons/contract/legacyargs.go create mode 100644 pkg/addons/contract/slots.go create mode 100644 pkg/addons/contract/slots.json create mode 100644 pkg/addons/contract/slots_test.go diff --git a/addons/office/manifest.json b/addons/office/manifest.json index 1f1fc24051..12398f394b 100644 --- a/addons/office/manifest.json +++ b/addons/office/manifest.json @@ -1,6 +1,6 @@ { "api": { - "max": 1, + "max": 2, "min": 1 }, "arch": "amd64", @@ -15,19 +15,62 @@ "skillDefinitions": [ { "name": "xlsx", - "shadows": "xlsx" + "shadows": "xlsx", + "slot": { + "extensions": [ + ".xlsx" + ], + "kind": "documents", + "library": "openpyxl", + "words": [ + "xlsx", + "excel" + ] + } }, { "name": "docx", - "shadows": "docx" + "shadows": "docx", + "slot": { + "extensions": [ + ".docx" + ], + "kind": "documents", + "library": "python-docx", + "words": [ + "docx", + "word" + ] + } }, { "name": "pptx", - "shadows": "pptx" + "shadows": "pptx", + "slot": { + "extensions": [ + ".pptx" + ], + "kind": "documents", + "library": "python-pptx", + "words": [ + "pptx", + "powerpoint" + ] + } }, { "name": "pdf", - "shadows": "pdf" + "shadows": "pdf", + "slot": { + "extensions": [ + ".pdf" + ], + "kind": "documents", + "library": "pypdf", + "words": [ + "pdf" + ] + } } ], "toolDefinitions": [ @@ -334,7 +377,13 @@ "markdown" ], "type": "object" - } + }, + "legacyArgs": [ + { + "hint": "use the markdown string", + "name": "blocks" + } + ] }, { "name": "create-xlsx", @@ -365,7 +414,13 @@ "csv" ], "type": "object" - } + }, + "legacyArgs": [ + { + "hint": "use the csv string", + "name": "sheets" + } + ] }, { "name": "create-pptx", @@ -441,7 +496,13 @@ "markdown" ], "type": "object" - } + }, + "legacyArgs": [ + { + "hint": "use the markdown string", + "name": "blocks" + } + ] } ], "tools": [ diff --git a/core/application/addon_gateway.go b/core/application/addon_gateway.go index fe96a83399..80c0e439c2 100644 --- a/core/application/addon_gateway.go +++ b/core/application/addon_gateway.go @@ -819,7 +819,7 @@ func (s *studioAddons) AddonSkills(ctx context.Context, user string) ([]studio.A continue } out = append(out, studio.AddonSkillSet{AddonID: a.AddonID, Title: a.Title, InstalledAt: a.InstalledAt, Skills: contract.SkillNames(a.Skills), Isolated: a.Isolated, - Shadows: contract.ShadowedSkills(a.Skills), Running: a.Running}) + Shadows: contract.ShadowedSkills(a.Skills), Slots: a.Slots, Running: a.Running}) } return out, nil } diff --git a/core/services/addons/tools/adapter.go b/core/services/addons/tools/adapter.go index c10208be1e..307c01e006 100644 --- a/core/services/addons/tools/adapter.go +++ b/core/services/addons/tools/adapter.go @@ -452,6 +452,8 @@ type AddonSkills struct { // Isolated are the skills that need the add-on's own commands tool to // report isolated execution (see contract.IsolatedSkills). Isolated []string + // Slots are the slots the add-on's skills serve (see contract.SlotsOf). + Slots []contract.Slot // Running is true when the add-on may serve now: enabled and running, as // Discover requires for tools. Running bool @@ -476,7 +478,7 @@ func (t *ToolAdapter) Skills(ctx context.Context, actor contract.Actor) ([]Addon } running := a.Enabled && a.State == "running" && a.Generation > 0 out = append(out, AddonSkills{AddonID: a.ID, Title: a.Title, InstalledAt: a.InstalledAt, Skills: slices.Clone(a.Skills), - Isolated: contract.IsolatedSkills(a.ToolDefinitions, a.Skills), Running: running}) + Isolated: contract.IsolatedSkills(a.ToolDefinitions, a.Skills), Slots: contract.SlotsOf(a.Skills), Running: running}) } slices.SortFunc(out, func(x, y AddonSkills) int { return strings.Compare(x.AddonID, y.AddonID) }) return out, nil @@ -709,6 +711,7 @@ func (t *ToolAdapter) execute(ctx context.Context, actor contract.Actor, convers var app apps.App var def ToolDefinition var found bool + var declared contract.ToolDefinition for _, a := range list { if a.ID != addonID { continue @@ -724,7 +727,7 @@ func (t *ToolAdapter) execute(ctx context.Context, actor contract.Actor, convers continue // not offered to this consumer, so not there } if x, ok := definition(a, d); ok { - app, def, found = a, x, true + app, def, found, declared = a, x, true, d } } } @@ -734,7 +737,7 @@ func (t *ToolAdapter) execute(ctx context.Context, actor contract.Actor, convers resource, body, err := toolargs.Bind(def.Tool, def.Parameters, arguments, conversationID != "") if err != nil { if errors.Is(err, ErrInvalidArguments) { - if message := officeCreationDiagnostic(addonID, tool, arguments); message != "" { + if message := contract.LegacyMessage(declared, arguments); message != "" { return ToolResult{Kind: ResultError, Tool: name, Code: CodeInvalidRequest, Message: message}, nil } } diff --git a/core/services/addons/tools/office_creation_test.go b/core/services/addons/tools/legacyargs_test.go similarity index 52% rename from core/services/addons/tools/office_creation_test.go rename to core/services/addons/tools/legacyargs_test.go index 1bc6cc5e59..90ac92e767 100644 --- a/core/services/addons/tools/office_creation_test.go +++ b/core/services/addons/tools/legacyargs_test.go @@ -13,8 +13,8 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("Office creation compatibility", func() { - It("names the replacement string for legacy arguments at the chat boundary without sending them", func() { +var _ = Describe("Legacy arguments declared by a tool", func() { + It("names the replacement string for the legacy arguments the shipped Office manifest declares, without sending them", func() { raw, err := os.ReadFile("../../../../addons/office/manifest.json") Expect(err).NotTo(HaveOccurred()) m, err := contract.ParseManifest(raw) @@ -41,4 +41,23 @@ var _ = Describe("Office creation compatibility", func() { Expect(f.acts).To(BeEmpty()) Expect(f.calls()).To(BeZero()) }) + + It("works for any add-on that declares legacyArgs, and for none that does not", func() { + schema := json.RawMessage(`{"type":"object","properties":{"path":{"type":"string"},"text":{"type":"string"}},"required":["path","text"],"additionalProperties":false}`) + def := func(legacy ...contract.LegacyArgument) contract.ToolDefinition { + return contract.ToolDefinition{Name: "make-note", Description: "Make a note.", Kind: contract.ToolKindAction, InputSchema: schema, LegacyArgs: legacy} + } + call := func(d contract.ToolDefinition) tools.ToolResult { + f := &actFake{fake: &fake{list: apps.List{Apps: []apps.App{{ID: "notes", Enabled: true, State: "running", Generation: 1, Tools: []string{"make-note"}, ToolDefinitions: []contract.ToolDefinition{d}}}}}} + res, err := tools.NewChatToolAdapter(f).ExecuteChat(context.Background(), member, tools.NewChatCall("bob", "conv1"), tools.ToolName("notes", "make-note"), json.RawMessage(`{"path":"/n","body":"PRIVATE"}`)) + if err != nil { + return tools.ToolResult{} + } + return res + } + res := call(def(contract.LegacyArgument{Name: "body", Hint: "use the text string"})) + Expect(res.Code).To(Equal(tools.CodeInvalidRequest)) + Expect(res.Message).To(Equal("body is no longer supported; use the text string")) + Expect(call(def()).Message).To(BeEmpty(), "a tool that declares nothing keeps the plain refusal") + }) }) diff --git a/core/services/addons/tools/office_creation.go b/core/services/addons/tools/office_creation.go deleted file mode 100644 index d664061f2f..0000000000 --- a/core/services/addons/tools/office_creation.go +++ /dev/null @@ -1,44 +0,0 @@ -// SPDX-License-Identifier: MIT -package tools - -import ( - "encoding/json" - "fmt" - - "github.com/mudler/LocalAI/core/services/addons/gateway" - "github.com/mudler/LocalAI/core/services/addons/toolargs" - "github.com/mudler/LocalAI/pkg/addons/contract" -) - -// Give old Office callers a migration hint without accepting their old schema. -// Structural and reserved-field refusals keep their existing semantics. -func officeCreationDiagnostic(addonID, tool string, raw json.RawMessage) string { - if addonID != "office" { - return "" - } - var legacy, replacement string - switch tool { - case "create-docx", "create-pdf": - legacy, replacement = "blocks", "markdown" - case "create-xlsx": - legacy, replacement = "sheets", "csv" - default: - return "" - } - if len(raw) == 0 || len(raw) > toolargs.MaxArgumentBytes || gateway.CheckToolArguments(raw) != nil { - return "" - } - var fields map[string]json.RawMessage - if json.Unmarshal(raw, &fields) != nil { - return "" - } - for key := range fields { - if contract.ForbiddenToolProperty(key) { - return "" - } - } - if _, ok := fields[legacy]; !ok { - return "" - } - return fmt.Sprintf("%s is no longer supported; use the %s string", legacy, replacement) -} diff --git a/core/services/studio/addonskills.go b/core/services/studio/addonskills.go index d088ea9902..ed9950ce08 100644 --- a/core/services/studio/addonskills.go +++ b/core/services/studio/addonskills.go @@ -19,7 +19,8 @@ import ( // chat switched off and one the chat's agent does not grant contribute none. // // A skill of an add-on may replace a built-in skill of the same name, when the -// name is in contract.ReplaceableSkills and the manifest says so (shadows), as +// name is a slot the add-on declares or a default one (contract.DefaultSlots) +// and the manifest says so (shadows), as // a tool replaces a built-in tool. Only one add-on serves such a name: the one // an administrator pinned in the same map as the tools (chats_tool_providers, // Settings > Tools), else the first by add-on ID. A pin to an add-on that @@ -43,6 +44,10 @@ type AddonSkillSet struct { Isolated []string // Shadows are the built-in skills it replaces (a subset of Skills). Shadows []string + // Slots are the slots its skills serve, as the add-on declares them. The + // host reads which names are replaceable and which files they cover from + // here, so a new format needs no change in the host. + Slots []contract.Slot // Running is true when the add-on can serve now: installed, enabled and // running. Running bool @@ -77,6 +82,13 @@ type addonSkills struct { // documentNotePrefix starts the note that explains the missing document skills. const documentNotePrefix = "the document skills " +// hiddenDocumentNote reports whether note is the document note (withNotes) +// and lists the skill name among the hidden ones. +func hiddenDocumentNote(note, name string) bool { + list, _, ok := strings.Cut(strings.TrimPrefix(note, documentNotePrefix), " need isolated") + return strings.HasPrefix(note, documentNotePrefix) && ok && slices.Contains(strings.Split(list, ", "), name) +} + func (a addonSkills) withNotes(notes []string) []string { if len(a.hiddenDocuments) == 0 { return notes @@ -154,10 +166,12 @@ func (a addonSkills) roots() []skills.Root { // AddonProvider): the pinned add-on when it ships the name, or none when that // add-on cannot serve now; with no pin, the default one (ChooseProvider: the oldest install). func (a addonSkills) provider(name string) (string, bool) { - if !contract.ReplaceableSkill(name) { + slot, ok := a.slot(name) + if !ok { return "", false } - if pin := a.pins[name]; pin != "" && (contract.DocumentSkill(name) || shipsReplacement(a.known, pin, name)) { + documents := slot.Kind == contract.SlotKindDocuments + if pin := a.pins[name]; pin != "" && (documents || shipsReplacement(a.known, pin, name)) { if shipsReplacement(a.usable, pin, name) { return pin, true } @@ -166,7 +180,45 @@ func (a addonSkills) provider(name string) (string, bool) { if id := ChooseProvider(a.shippers(a.usable, name), a.installTimes(), ""); id != "" { return id, true } - return "", contract.DocumentSkill(name) + return "", documents +} + +// slot returns the slot called name: one that a known add-on serves, else a +// default one. The default entries come from data (contract.DefaultSlots). +func (a addonSkills) slot(name string) (contract.Slot, bool) { + for _, set := range a.known { + for _, sl := range set.Slots { + if sl.Name == name { + return sl, true + } + } + } + return contract.DefaultSlot(name) +} + +// slotNames lists the names of all the slots: the default ones in data order, +// then those that known add-ons serve, sorted. +func (a addonSkills) slotNames() []string { + return slotNamesOf(a.known) +} + +func slotNamesOf(sets []AddonSkillSet) []string { + seen := map[string]bool{} + var out, extra []string + for _, sl := range contract.DefaultSlots() { + seen[sl.Name] = true + out = append(out, sl.Name) + } + for _, set := range sets { + for _, sl := range set.Slots { + if !seen[sl.Name] { + seen[sl.Name] = true + extra = append(extra, sl.Name) + } + } + } + sort.Strings(extra) + return append(out, extra...) } // shippers lists the add-ons of the sets that ship a replacement for name. @@ -192,7 +244,7 @@ func (a addonSkills) installTimes() map[string]string { // options are the add-on parts of skills.Options. func (a addonSkills) options(o skills.Options) skills.Options { o.AddonProvider = a.provider - o.Replaceable = contract.ReplaceableSkill + o.Replaceable = func(name string) bool { _, ok := a.slot(name); return ok } return o } @@ -208,6 +260,8 @@ func shipsReplacement(sets []AddonSkillSet, addon, name string) bool { return false } +func (a addonSkills) replaceable(name string) bool { _, ok := a.slot(name); return ok } + // title returns the title of a known add-on, or its ID. func (a addonSkills) title(id string) string { for _, set := range a.known { @@ -226,7 +280,7 @@ func (a addonSkills) markFallbacks(list []skills.Skill) string { var parts []string for i := range list { sk := &list[i] - if sk.Layer != skills.LayerBuiltin || sk.Invalid != "" || sk.Shadowed != "" || !contract.ReplaceableSkill(sk.Name) { + if sk.Layer != skills.LayerBuiltin || sk.Invalid != "" || sk.Shadowed != "" || !a.replaceable(sk.Name) { continue } var by string diff --git a/core/services/studio/addonskills_internal_test.go b/core/services/studio/addonskills_internal_test.go index 314c78496b..e75e32bb8c 100644 --- a/core/services/studio/addonskills_internal_test.go +++ b/core/services/studio/addonskills_internal_test.go @@ -18,6 +18,7 @@ import ( "github.com/mudler/LocalAI/core/services/studio/store" "github.com/mudler/LocalAI/core/services/studio/workspace/blob" "github.com/mudler/LocalAI/core/services/studio/workspace/gormstore" + "github.com/mudler/LocalAI/pkg/addons/contract" "github.com/mudler/LocalAI/pkg/sqlitedb" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -254,6 +255,21 @@ var _ = Describe("add-on skills that replace built-in skills", func() { Expect(fallback).To(ContainSubstring("xlsx (Office)"), "the turn says why the built-in skill serves") }) + It("serves a slot that an add-on declares, with no change in the host", func() { + Expect(f.SyncAddonSkills(ctx, "odfkit", fstest.MapFS{"odt/SKILL.md": {Data: md("odt", "ODF documents.")}})).To(Succeed()) + slot := contract.Slot{Name: "odt", SlotDefinition: contract.SlotDefinition{Kind: contract.SlotKindDocuments, Extensions: []string{".odt"}}} + b.set(AddonSkillSet{AddonID: "odfkit", Title: "ODF", Skills: []string{"odt"}, Shadows: []string{"odt"}, Slots: []contract.Slot{slot}, Running: true}) + prompt, _, _ := turn(nil, nil) + Expect(prompt).To(ContainSubstring("- odt: ODF documents.")) + list, err := svc.AdminBuiltinSkills(ctx, "alice") + Expect(err).NotTo(HaveOccurred()) + var names []string + for _, i := range list { + names = append(names, i.ID) + } + Expect(names).To(ContainElement("odt")) + }) + It("gives the built-in skill back, with the notice, when the chat switches the add-on or its skill off", func() { b.set(office(true)) prompt, _, fallback := turn(nil, []string{"office"}) diff --git a/core/services/studio/binaryhint.go b/core/services/studio/binaryhint.go index bb1a60b26f..30a837bcef 100644 --- a/core/services/studio/binaryhint.go +++ b/core/services/studio/binaryhint.go @@ -6,26 +6,15 @@ import ( "errors" "fmt" "path" + "slices" "strings" "github.com/mudler/LocalAI/core/services/studio/files" "github.com/mudler/LocalAI/core/services/studio/session" "github.com/mudler/LocalAI/core/services/studio/workspace" + "github.com/mudler/LocalAI/pkg/addons/contract" ) -// officeKinds are the binary document types a chat meets most: the words a -// skill of the type has in its name, and a library that reads the type in a -// script. -var officeKinds = map[string]struct { - skills []string - library string -}{ - ".docx": {[]string{"docx", "word"}, "python-docx"}, - ".xlsx": {[]string{"xlsx", "excel"}, "openpyxl"}, - ".pptx": {[]string{"pptx", "powerpoint"}, "python-pptx"}, - ".pdf": {[]string{"pdf"}, "pypdf"}, -} - // binaryHintTools wraps the tools that read a file as text: when one meets a // binary file, the error says what the chat can do with it. It names the // skill that matches the file type when the chat has one, and a script when @@ -55,12 +44,12 @@ func (s *Service) binaryHintTools(a workspace.Actor, canRun bool, tools []sessio func (s *Service) binaryAdvice(ctx context.Context, a workspace.Actor, canRun bool, nt *files.NotTextError) string { ext := strings.ToLower(path.Ext(nt.Name)) msg := fmt.Sprintf("%s is a binary file (%d bytes), not text.", nt.Name, nt.Size) - kind, office := officeKinds[ext] - if office { + kind, known := s.slotForExtension(ctx, a.Principal, ext) + if known { if list, _, err := s.Skills(ctx, a.Principal, a.ChatID); err == nil { for _, sk := range list { name := strings.ToLower(sk.Name) - for _, w := range kind.skills { + for _, w := range kind.Words { if strings.Contains(name, w) { return msg + fmt.Sprintf(" Use the %s skill for %s files (load_skill).", sk.Name, ext) } @@ -69,10 +58,33 @@ func (s *Service) binaryAdvice(ctx context.Context, a workspace.Actor, canRun bo } } switch { - case canRun && office: - return msg + fmt.Sprintf(" Run a script with the commands tool instead, for example with %s.", kind.library) + case canRun && known && kind.Library != "": + return msg + fmt.Sprintf(" Run a script with the commands tool instead, for example with %s.", kind.Library) case canRun: return msg + " Run a script with the commands tool instead." } return msg + " This chat has no tool that opens it." } + +// slotForExtension finds the slot that covers files with the extension ext. +// The slots come from the skills the add-ons declare, then from the default +// slots (data in the contract); the host holds no list of formats of its own. +func (s *Service) slotForExtension(ctx context.Context, p, ext string) (contract.Slot, bool) { + var sets []AddonSkillSet + if lister, ok := s.d.AddonTools.(AddonSkillLister); ok && s.d.AddonTools != nil { + sets, _ = lister.AddonSkills(ctx, p) + } + for _, set := range sets { + for _, sl := range set.Slots { + if sl.Kind == contract.SlotKindDocuments && slices.Contains(sl.Extensions, ext) { + return sl, true + } + } + } + for _, sl := range contract.DefaultSlots() { + if sl.Kind == contract.SlotKindDocuments && slices.Contains(sl.Extensions, ext) { + return sl, true + } + } + return contract.Slot{}, false +} diff --git a/core/services/studio/binaryhint_internal_test.go b/core/services/studio/binaryhint_internal_test.go index a21885bcff..59be029af6 100644 --- a/core/services/studio/binaryhint_internal_test.go +++ b/core/services/studio/binaryhint_internal_test.go @@ -8,6 +8,7 @@ import ( "github.com/mudler/LocalAI/core/services/studio/files" "github.com/mudler/LocalAI/core/services/studio/session" + "github.com/mudler/LocalAI/pkg/addons/contract" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -47,6 +48,20 @@ var _ = Describe("hints on binary files", func() { Expect(advice("data.bin", true)).To(HaveSuffix("Run a script with the commands tool instead.")) }) + It("learns the files of a format from the add-on that declares its slot", func() { + ctx := context.Background() + b := &skillBroker{} + b.set(AddonSkillSet{AddonID: "odfkit", Title: "ODF", Skills: []string{"odt"}, Shadows: []string{"odt"}, Running: true, + Slots: []contract.Slot{{Name: "odt", SlotDefinition: contract.SlotDefinition{Kind: contract.SlotKindDocuments, Extensions: []string{".odt"}, Words: []string{"odt"}, Library: "odfpy"}}}}) + svc, f := newSkillService(b) + binary := func(context.Context, json.RawMessage) (string, error) { + return "", &files.NotTextError{Name: "memo.odt", Size: 10} + } + tools := svc.binaryHintTools(f.Actor("alice"), true, []session.Tool{{Name: "read_file", Run: binary}}) + _, err := tools[0].Run(ctx, json.RawMessage(`{"path":"/x"}`)) + Expect(err).To(MatchError(HaveSuffix("for example with odfpy."))) + }) + It("says so when the chat has neither", func() { Expect(advice("report.pdf", false)).To(HaveSuffix("This chat has no tool that opens it.")) }) diff --git a/core/services/studio/loadskill.go b/core/services/studio/loadskill.go index 9a2bdfb605..12cd9fe442 100644 --- a/core/services/studio/loadskill.go +++ b/core/services/studio/loadskill.go @@ -14,7 +14,6 @@ import ( "github.com/mudler/LocalAI/core/services/studio/session" "github.com/mudler/LocalAI/core/services/studio/skills" "github.com/mudler/LocalAI/core/services/studio/workspace" - "github.com/mudler/LocalAI/pkg/addons/contract" ) // loadSkillName is the name of the tool that loads a skill by its name. @@ -175,8 +174,9 @@ func (s *Service) loadSkillRun(a workspace.Actor, o loadOpts) func(context.Conte // The tool was offered because the chat had this skill; a document // skill is missing now only because its add-on is not known to be // isolated, and the model must say so rather than "no skills". - if _, ok := skills.Find(list, in.Name); !ok && contract.DocumentSkill(skills.Clean(in.Name)) { - if i := slices.IndexFunc(notes, func(n string) bool { return strings.HasPrefix(n, documentNotePrefix) }); i >= 0 { + if _, ok := skills.Find(list, in.Name); !ok { + name := skills.Clean(in.Name) + if i := slices.IndexFunc(notes, func(n string) bool { return hiddenDocumentNote(n, name) }); i >= 0 { return "", fmt.Errorf("unknown skill %q: %s", skills.Clean(in.Name), notes[i]) } } diff --git a/core/services/studio/noaddonid_test.go b/core/services/studio/noaddonid_test.go index 9128fdd80d..5f8bd653dd 100644 --- a/core/services/studio/noaddonid_test.go +++ b/core/services/studio/noaddonid_test.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" + "github.com/mudler/LocalAI/pkg/addons/contract" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -50,7 +51,7 @@ var _ = Describe("the command execution code of the host", func() { It("compares no string with the ID of the add-on that runs commands", func() { // Other add-on IDs join this list as their special cases are removed. - ids := []string{"sandbox"} + ids := []string{"sandbox", "office"} for _, dir := range []string{".", "../addons/tools"} { for path, values := range literals(dir) { for _, v := range values { @@ -67,4 +68,18 @@ var _ = Describe("the command execution code of the host", func() { } } }) + + It("holds no list of document formats: slots come from the add-ons and from data", func() { + var formats []string + for _, sl := range contract.DefaultSlots() { + formats = append(formats, sl.Name, sl.Library) + formats = append(formats, sl.Extensions...) + formats = append(formats, sl.Words...) + } + for path, values := range literals(".", "binaryhint.go", "addonskills.go", "loadskill.go", "toolsadmin.go") { + for _, v := range values { + Expect(formats).NotTo(ContainElement(v), "%s names a document format; read it from the slot", path) + } + } + }) }) diff --git a/core/services/studio/toolsadmin.go b/core/services/studio/toolsadmin.go index 57293733ad..245d135bed 100644 --- a/core/services/studio/toolsadmin.go +++ b/core/services/studio/toolsadmin.go @@ -7,7 +7,6 @@ import ( "time" "github.com/mudler/LocalAI/core/services/studio/hostexec" - "github.com/mudler/LocalAI/pkg/addons/contract" ) // BuiltinToolInfo is one built-in chat tool as Settings > Tools shows it. @@ -203,7 +202,7 @@ func (s *Service) AdminBuiltinSkills(ctx context.Context, user string) ([]Builti return nil, err } var out []BuiltinSkillInfo - for _, name := range contract.ReplaceableSkills { + for _, name := range ad.slotNames() { info := BuiltinSkillInfo{ID: name, Pinned: ad.pins[name], Titles: map[string]string{}} installed := ad.installTimes() info.Candidates = OrderProviders(ad.shippers(ad.known, name), installed) diff --git a/docs/content/features/addons.md b/docs/content/features/addons.md index 7da719650d..cfbcf98398 100644 --- a/docs/content/features/addons.md +++ b/docs/content/features/addons.md @@ -140,7 +140,7 @@ and reconciling the operation before further use. These declarations extend the strict manifest contract. They keep `schemaVersion: 1`, host/protocol/data-format level 1 and UI SDK 2. The manifest -`api` range is 1 to 2: the execution descriptor, the resource listing and the network capability below need 2. +`api` range is 1 to 2: the execution descriptor, the resource listing, the network capability, the slot of a skill and the legacy arguments of a tool below need 2. The Go contract parser is authoritative; there is no separate JSON Schema. The fragments below merge into an otherwise valid manifest, not a complete installable package. Existing executable, identity, compatibility and UI @@ -288,6 +288,20 @@ list and rejects its pending responses, even before effect cleanup. A new list request can populate the refreshed session. These in-memory scopes contain no credentials. +### Legacy arguments of a tool + +Needs manifest `api` 2. A tool can name arguments that an older version took +and that it refuses now. When a call fails the schema check and carries one of +them, the host answers with the hint instead of a bare refusal. The argument is +never forwarded, and the schema must not declare it. + +```json +{ "name": "create-docx", "legacyArgs": [ { "name": "blocks", "hint": "use the markdown string" } ] } +``` + +The answer is `blocks is no longer supported; use the markdown string`. At most +8 entries; the hint is one plain sentence of at most 200 characters. + ### Execution descriptor of a commands tool Needs manifest `api` 2. On the definition of a tool that shadows `bash`: @@ -1412,7 +1426,8 @@ refreshed: | Name | `[a-z][a-z0-9-]{0,63}`, unique in the manifest | | A plain name | the add-on ID, or the ID and `-` and more (`office-quickstart`), so two add-ons never ship one name | | A reserved name | refused for a plain skill: core names `writing-skills`, `localai-workspace`, `localai-apps`, and document names `docx`, `pptx`, `pdf`, `xlsx` | -| `shadows` | optional; must repeat `name` and identify a supported document-provider slot: `xlsx`, `docx`, `pptx`, or `pdf` | +| `shadows` | optional; must repeat `name` and name a slot: one the skill declares with `slot` (needs `api` 2), or a default slot (`xlsx`, `docx`, `pptx`, `pdf`) | +| `slot` | optional, needs `shadows` and `api` 2; see Document skill providers | Staging checks the files against the manifest. The folders under `skills/` must be the declared names, and a package that declares no skills must have @@ -1439,7 +1454,11 @@ with a saved agent only when the agent grants the add-on. #### Document skill providers -The names `docx`, `pptx`, `pdf`, and `xlsx` are reserved document-provider slots. +A slot is a skill name that one add-on serves in place of another. The host +holds no list of formats: the add-on that provides a skill declares its slot. +For manifests that declare nothing, the host reads four default slots (`docx`, +`pptx`, `pdf`, `xlsx`) from a data file in the contract, so existing packages +keep working. Core does not embed these document procedures. Its three remaining built-in skills cannot be replaced. The existing `shadows` declaration identifies a document provider: @@ -1450,6 +1469,20 @@ The existing `shadows` declaration identifies a document provider: ] ``` +A manifest of `api` 2 can declare the slot, which also adds a format the host +has never heard of. `extensions` and `words` tell the host which files the +skill covers and how to find the skill by name when a chat meets such a file, +and `library` is the hint it gives for a script: + +```json +{ "name": "odt", "shadows": "odt", + "slot": { "kind": "documents", "extensions": [".odt"], "words": ["odt"], "library": "odfpy" } } +``` + +`kind` is `documents`: the skill is offered only while the add-on's commands +provider reports isolated execution. At most 8 extensions (lower case, with the +dot) and 8 words. A slot may not be the name of a protected core skill. + Existing Office packages with these declarations remain compatible; you do not need to reinstall them to change naming metadata. Unknown unprefixed names remain invalid. Ordinary add-on skills keep the add-on namespace rule. diff --git a/pkg/addons/contract/execution.go b/pkg/addons/contract/execution.go index b219ee6555..2b2903b916 100644 --- a/pkg/addons/contract/execution.go +++ b/pkg/addons/contract/execution.go @@ -98,5 +98,11 @@ func IsolatedSkills(defs []ToolDefinition, skills []SkillDefinition) []string { if e := tool.Execution; e != nil && (e.Skills == nil || !slices.Contains(e.Skills.ProvidesRuntimeFor, SkillKindDocuments)) { return nil } - return ShadowedSkills(skills) + var out []string + for _, s := range SlotsOf(skills) { + if s.Kind == SlotKindDocuments { + out = append(out, s.Name) + } + } + return out } diff --git a/pkg/addons/contract/legacyargs.go b/pkg/addons/contract/legacyargs.go new file mode 100644 index 0000000000..8be4359511 --- /dev/null +++ b/pkg/addons/contract/legacyargs.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: MIT +package contract + +import ( + "encoding/json" + "fmt" +) + +// MinLegacyArgsAPI is the first manifest API version that knows legacyArgs. A +// manifest that declares one must accept it. +const MinLegacyArgsAPI uint64 = 2 + +// Bounds of the legacy argument list of one tool. +const ( + MaxLegacyArgs = 8 + MaxLegacyArgChars = 64 + MaxLegacyHintChars = 200 +) + +// LegacyArgument names an argument that an older version of a tool took and +// that the tool refuses now. When a call fails the schema check and carries +// it, the host answers with the hint instead of a bare refusal, so a model +// that learned the old schema can correct itself. The argument is never +// forwarded. +type LegacyArgument struct { + // Name is the old argument name. The tool's schema must not declare it. + Name string `json:"name"` + // Hint is one plain sentence on what to send instead, such as "use the + // markdown string". + Hint string `json:"hint"` +} + +// LegacyMessage returns the message for a call whose arguments carry a legacy +// argument of the tool d, or "". Arguments that would be refused for another +// reason (a forbidden name, bad JSON) get no message. +func LegacyMessage(d ToolDefinition, raw json.RawMessage) string { + if len(d.LegacyArgs) == 0 || len(raw) == 0 { + return "" + } + var fields map[string]json.RawMessage + if json.Unmarshal(raw, &fields) != nil { + return "" + } + for key := range fields { + if ForbiddenToolProperty(key) { + return "" + } + } + for _, l := range d.LegacyArgs { + if _, ok := fields[l.Name]; ok { + return fmt.Sprintf("%s is no longer supported; %s", l.Name, l.Hint) + } + } + return "" +} + +func validateLegacyArgs(d ToolDefinition) error { + if len(d.LegacyArgs) == 0 { + return nil + } + if len(d.LegacyArgs) > MaxLegacyArgs { + return fmt.Errorf("tool %s: too many legacy arguments", d.Name) + } + var schema struct { + Properties map[string]json.RawMessage `json:"properties"` + } + _ = json.Unmarshal(d.InputSchema, &schema) + seen := map[string]bool{} + for _, l := range d.LegacyArgs { + if !plainText(l.Name, MaxLegacyArgChars) || !plainText(l.Hint, MaxLegacyHintChars) || ForbiddenToolProperty(l.Name) || seen[l.Name] { + return fmt.Errorf("tool %s: invalid legacy argument", d.Name) + } + if _, ok := schema.Properties[l.Name]; ok { + return fmt.Errorf("tool %s: legacy argument %s is in the schema", d.Name, l.Name) + } + seen[l.Name] = true + } + return nil +} diff --git a/pkg/addons/contract/manifest.go b/pkg/addons/contract/manifest.go index c3356345b0..04fda433ca 100644 --- a/pkg/addons/contract/manifest.go +++ b/pkg/addons/contract/manifest.go @@ -393,6 +393,9 @@ func ValidateManifest(m Manifest, h HostCompatibility) error { if d.Execution != nil && m.API.Max < MinExecutionAPI { return fmt.Errorf("tool %s declares an execution descriptor, which needs api %d", d.Name, MinExecutionAPI) } + if len(d.LegacyArgs) > 0 && m.API.Max < MinLegacyArgsAPI { + return fmt.Errorf("tool %s declares legacy arguments, which need api %d", d.Name, MinLegacyArgsAPI) + } if d.Kind == ToolKindAction && !c.Actions { return fmt.Errorf("tool %s is an action tool but the add-on does not declare actions", d.Name) } @@ -421,6 +424,11 @@ func ValidateManifest(m Manifest, h HostCompatibility) error { if e := validateConnector(c.Connector, c.UI, m.API); e != nil { return e } + for _, d := range c.SkillDefinitions { + if d.Slot != nil && m.API.Max < MinSlotAPI { + return fmt.Errorf("skill %s declares a slot, which needs api %d", d.Name, MinSlotAPI) + } + } if e := validateSkillDefinitions(m.ID, c.SkillDefinitions); e != nil { return e } diff --git a/pkg/addons/contract/skills.go b/pkg/addons/contract/skills.go index df5fe1de1b..b0d8312498 100644 --- a/pkg/addons/contract/skills.go +++ b/pkg/addons/contract/skills.go @@ -25,11 +25,16 @@ type SkillDefinition struct { // for a reserved document slot has the slot's name (see Shadows). Name string `json:"name"` // Shadows names the document-provider slot this skill supplies. It must equal Name - // and be one of ReplaceableSkills. It repeats the name on purpose: the + // and name a slot: one the skill declares (Slot) or a default one + // (DefaultSlots). It repeats the name on purpose: the // replacement is an explicit act, which the install warning and the check // read from the manifest without opening the package. Empty: a plain // add-on skill. Shadows string `json:"shadows,omitempty"` + // Slot describes the slot this skill serves: the files it covers and the + // words to say about them (see SlotDefinition). It needs Shadows and + // manifest API 2. A skill that shadows a default slot may leave it out. + Slot *SlotDefinition `json:"slot,omitempty"` } // protectedSkills are the built-in skills that no add-on may replace or @@ -44,14 +49,22 @@ func ProtectedSkill(name string) bool { return slices.Contains(protectedSkills, name) } -// reservedDocumentSkills are provider slots, independent of embedded core skills. -var reservedDocumentSkills = []string{"xlsx", "docx", "pptx", "pdf"} +// DocumentSkill reports whether name is a default document slot (slots.json). +// An add-on can declare more slots of its own; the host learns them from the +// skills it lists, not from here. +func DocumentSkill(name string) bool { + s, ok := DefaultSlot(name) + return ok && s.Kind == SlotKindDocuments +} -// DocumentSkill reports whether name is a supported document-provider slot. -func DocumentSkill(name string) bool { return slices.Contains(reservedDocumentSkills, name) } - -// ReplaceableSkills retains the existing slot inventory for compatibility. -var ReplaceableSkills = slices.Clone(reservedDocumentSkills) +// ReplaceableSkills lists the default slots. +var ReplaceableSkills = func() []string { + var out []string + for _, s := range defaultSlots() { + out = append(out, s.Name) + } + return out +}() // ReplaceableSkill is the compatibility name for DocumentSkill. func ReplaceableSkill(name string) bool { return DocumentSkill(name) } @@ -87,11 +100,21 @@ func validateSkillDefinitions(id string, defs []SkillDefinition) error { switch { case d.Shadows != d.Name: return fmt.Errorf("skill %s: shadows must repeat the skill's name", d.Name) - case !ReplaceableSkill(d.Shadows): - return fmt.Errorf("skill %s: %q is not a document-provider slot an add-on may replace", d.Name, d.Shadows) + case ProtectedSkill(d.Shadows): + return fmt.Errorf("skill %s: %q is a built-in skill that an add-on may replace in no way", d.Name, d.Shadows) + case d.Slot == nil && !ReplaceableSkill(d.Shadows): + return fmt.Errorf("skill %s: %q is not a slot an add-on may replace; declare its slot", d.Name, d.Shadows) + } + if d.Slot != nil { + if err := d.Slot.validate(d.Name); err != nil { + return err + } } continue } + if d.Slot != nil { + return fmt.Errorf("skill %s: a slot needs shadows", d.Name) + } if DocumentSkill(d.Name) { return fmt.Errorf("skill %s: reserved document name requires same-name shadows", d.Name) } @@ -110,7 +133,7 @@ func validateSkillDefinitions(id string, defs []SkillDefinition) error { func ShadowedSkills(defs []SkillDefinition) []string { var out []string for _, d := range defs { - if d.Shadows != "" && d.Shadows == d.Name && ReplaceableSkill(d.Shadows) { + if _, ok := d.slot(); ok { out = append(out, d.Shadows) } } diff --git a/pkg/addons/contract/slots.go b/pkg/addons/contract/slots.go new file mode 100644 index 0000000000..a561eb7154 --- /dev/null +++ b/pkg/addons/contract/slots.go @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: MIT +package contract + +import ( + _ "embed" + "encoding/json" + "fmt" + "slices" + "strings" + "sync" +) + +// MinSlotAPI is the first manifest API version that knows the slot +// declaration of a skill. A manifest that declares one must accept it. +const MinSlotAPI uint64 = 2 + +// SlotKindDocuments is the kind of a slot for a document format: the skill +// that serves it needs a commands provider that reports isolated execution. +const SlotKindDocuments = SkillKindDocuments + +// Bounds of a slot declaration. +const ( + MaxSlotExtensions = 8 + MaxSlotWords = 8 + MaxSlotWordChars = 32 + MaxSlotLibChars = 64 +) + +// SlotDefinition describes the slot a skill serves. The add-on that provides +// the skill declares it, so the host needs no list of formats. It carries no +// authority: it only tells the host which files the skill covers and what to +// say when the host meets one. +type SlotDefinition struct { + // Kind is SlotKindDocuments. + Kind string `json:"kind"` + // Extensions are the file extensions the skill covers, lower case with the + // dot, such as ".docx". + Extensions []string `json:"extensions,omitempty"` + // Words are lower-case words that a skill of this slot has in its name. + Words []string `json:"words,omitempty"` + // Library names what a script can use to read these files, shown as a hint. + Library string `json:"library,omitempty"` +} + +// Slot is a named slot: the name of the skill that serves it and its +// definition. +type Slot struct { + Name string `json:"name"` + SlotDefinition +} + +//go:embed slots.json +var defaultSlotsJSON []byte + +var defaultSlots = sync.OnceValue(func() []Slot { + var out []Slot + if err := json.Unmarshal(defaultSlotsJSON, &out); err != nil { + panic("contract: slots.json: " + err.Error()) + } + return out +}) + +// DefaultSlots are the slots the host knows without any declaration, read +// from data. A skill that shadows one of them and declares no slot of its own +// (a manifest of API 1) is served by these. +func DefaultSlots() []Slot { return slices.Clone(defaultSlots()) } + +// DefaultSlot returns the default slot called name. +func DefaultSlot(name string) (Slot, bool) { + i := slices.IndexFunc(defaultSlots(), func(s Slot) bool { return s.Name == name }) + if i < 0 { + return Slot{}, false + } + return defaultSlots()[i], true +} + +// SlotsOf lists the slots the skill definitions serve, in order: the slot a +// skill declares, or else the default slot of the name it shadows. +func SlotsOf(defs []SkillDefinition) []Slot { + var out []Slot + for _, d := range defs { + if s, ok := d.slot(); ok { + out = append(out, s) + } + } + return out +} + +// slot returns the slot this skill serves. +func (d SkillDefinition) slot() (Slot, bool) { + if d.Shadows == "" || d.Shadows != d.Name { + return Slot{}, false + } + if d.Slot != nil { + return Slot{Name: d.Name, SlotDefinition: *d.Slot}, true + } + return DefaultSlot(d.Name) +} + +// validate checks a slot declaration of the skill name. +func (s *SlotDefinition) validate(name string) error { + if s.Kind != SlotKindDocuments { + return fmt.Errorf("skill %s: unknown slot kind %q", name, s.Kind) + } + if len(s.Extensions) > MaxSlotExtensions || len(s.Words) > MaxSlotWords { + return fmt.Errorf("skill %s: too many slot extensions or words", name) + } + for _, e := range s.Extensions { + if len(e) < 2 || len(e) > 16 || e[0] != '.' || strings.ToLower(e) != e || !plainText(e, 16) || strings.ContainsAny(e[1:], "./\\ ") { + return fmt.Errorf("skill %s: invalid slot extension %q", name, e) + } + } + for _, w := range s.Words { + if !plainText(w, MaxSlotWordChars) || strings.ToLower(w) != w { + return fmt.Errorf("skill %s: invalid slot word", name) + } + } + if s.Library != "" && !plainText(s.Library, MaxSlotLibChars) { + return fmt.Errorf("skill %s: invalid slot library", name) + } + return nil +} diff --git a/pkg/addons/contract/slots.json b/pkg/addons/contract/slots.json new file mode 100644 index 0000000000..b7c48667db --- /dev/null +++ b/pkg/addons/contract/slots.json @@ -0,0 +1,6 @@ +[ + {"name": "xlsx", "kind": "documents", "extensions": [".xlsx"], "words": ["xlsx", "excel"], "library": "openpyxl"}, + {"name": "docx", "kind": "documents", "extensions": [".docx"], "words": ["docx", "word"], "library": "python-docx"}, + {"name": "pptx", "kind": "documents", "extensions": [".pptx"], "words": ["pptx", "powerpoint"], "library": "python-pptx"}, + {"name": "pdf", "kind": "documents", "extensions": [".pdf"], "words": ["pdf"], "library": "pypdf"} +] diff --git a/pkg/addons/contract/slots_test.go b/pkg/addons/contract/slots_test.go new file mode 100644 index 0000000000..9dd5877d2f --- /dev/null +++ b/pkg/addons/contract/slots_test.go @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: MIT +package contract + +import ( + "encoding/json" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Slots served by skills", func() { + host := func() HostCompatibility { + h := compatible() + h.SDK = 0 + h.API, h.APIMin = HostAPIVersion, HostAPIMin + return h + } + manifestWith := func(api VersionRange, defs ...SkillDefinition) Manifest { + m, err := ParseManifest(fixture()) + Expect(err).NotTo(HaveOccurred()) + m.API = api + m.Capabilities.SkillDefinitions = defs + return m + } + odt := SkillDefinition{Name: "odt", Shadows: "odt", Slot: &SlotDefinition{Kind: SlotKindDocuments, Extensions: []string{".odt"}, Words: []string{"odt", "opendocument"}, Library: "odfpy"}} + + It("reads the default slots from data", func() { + Expect(ReplaceableSkills).To(ConsistOf("xlsx", "docx", "pptx", "pdf")) + s, ok := DefaultSlot("docx") + Expect(ok).To(BeTrue()) + Expect(s.Extensions).To(Equal([]string{".docx"})) + Expect(s.Library).To(Equal("python-docx")) + }) + + It("lets an add-on declare a slot the contract has never heard of", func() { + Expect(ValidateManifest(manifestWith(VersionRange{Min: 1, Max: 2}, odt), host())).To(Succeed()) + Expect(ShadowedSkills([]SkillDefinition{odt})).To(Equal([]string{"odt"})) + Expect(SlotsOf([]SkillDefinition{odt})[0].Library).To(Equal("odfpy")) + Expect(IsolatedSkills([]ToolDefinition{{Name: "run", Kind: ToolKindAction, Shadows: "bash", InputSchema: json.RawMessage(`{"type":"object","properties":{"command":{"type":"string"}},"required":["command"],"additionalProperties":false}`)}}, []SkillDefinition{odt})).To(Equal([]string{"odt"})) + }) + + It("needs the manifest to accept API 2 to declare a slot", func() { + Expect(ValidateManifest(manifestWith(VersionRange{Min: 1, Max: 1}, odt), host())).NotTo(Succeed()) + }) + + It("keeps a skill that shadows a default slot and declares nothing working", func() { + d := SkillDefinition{Name: "docx", Shadows: "docx"} + Expect(ValidateManifest(manifestWith(VersionRange{Min: 1, Max: 1}, d), host())).To(Succeed()) + Expect(SlotsOf([]SkillDefinition{d})[0].Library).To(Equal("python-docx")) + }) + + It("refuses a shadow of an unknown slot without a declaration, and bad declarations", func() { + Expect(validateSkillDefinitions("sample", []SkillDefinition{{Name: "odt", Shadows: "odt"}})).NotTo(Succeed()) + Expect(validateSkillDefinitions("sample", []SkillDefinition{{Name: "sample", Slot: odt.Slot}})).NotTo(Succeed()) + Expect(validateSkillDefinitions("sample", []SkillDefinition{{Name: "writing-skills", Shadows: "writing-skills", Slot: odt.Slot}})).NotTo(Succeed()) + for _, bad := range []*SlotDefinition{ + {Kind: "games"}, + {Kind: SlotKindDocuments, Extensions: []string{"odt"}}, + {Kind: SlotKindDocuments, Extensions: []string{".ODT"}}, + {Kind: SlotKindDocuments, Words: []string{""}}, + {Kind: SlotKindDocuments, Library: ""}, + } { + Expect(validateSkillDefinitions("sample", []SkillDefinition{{Name: "odt", Shadows: "odt", Slot: bad}})).NotTo(Succeed()) + } + }) +}) + +var _ = Describe("Legacy arguments of a tool", func() { + const schema = `{"type":"object","properties":{"path":{"type":"string"},"markdown":{"type":"string"}},"required":["path"],"additionalProperties":false}` + def := func(l ...LegacyArgument) ToolDefinition { + return ToolDefinition{Name: "create", Description: "Create.", Kind: ToolKindCreate, InputSchema: json.RawMessage(schema), LegacyArgs: l} + } + blocks := LegacyArgument{Name: "blocks", Hint: "use the markdown string"} + + It("answers with the hint when a call carries the old argument", func() { + Expect(LegacyMessage(def(blocks), json.RawMessage(`{"path":"/a","blocks":[1]}`))).To(Equal("blocks is no longer supported; use the markdown string")) + }) + It("says nothing without the argument, without a declaration or for a forbidden property", func() { + Expect(LegacyMessage(def(blocks), json.RawMessage(`{"path":"/a"}`))).To(BeEmpty()) + Expect(LegacyMessage(def(), json.RawMessage(`{"blocks":1}`))).To(BeEmpty()) + Expect(LegacyMessage(def(blocks), json.RawMessage(`{"blocks":1,"approved":true}`))).To(BeEmpty()) + Expect(LegacyMessage(def(blocks), json.RawMessage(`nope`))).To(BeEmpty()) + }) + It("is validated: bounded, plain, not in the schema, and API 2", func() { + Expect(validateLegacyArgs(def(blocks))).To(Succeed()) + Expect(validateLegacyArgs(def(LegacyArgument{Name: "markdown", Hint: "x"}))).NotTo(Succeed()) + Expect(validateLegacyArgs(def(LegacyArgument{Name: "blocks", Hint: ""}))).NotTo(Succeed()) + Expect(validateLegacyArgs(def(blocks, blocks))).NotTo(Succeed()) + m, err := ParseManifest(fixture()) + Expect(err).NotTo(HaveOccurred()) + h := compatible() + h.SDK = 0 + h.API, h.APIMin = HostAPIVersion, HostAPIMin + m.Capabilities.Tools = []string{"create"} + m.Capabilities.ToolDefinitions = []ToolDefinition{def(blocks)} + m.API = VersionRange{Min: 1, Max: 1} + Expect(ValidateManifest(m, h)).NotTo(Succeed()) + m.API = VersionRange{Min: 1, Max: 2} + Expect(ValidateManifest(m, h)).To(Succeed()) + }) +}) diff --git a/pkg/addons/contract/tooldefs.go b/pkg/addons/contract/tooldefs.go index c7641c46a0..e9a7ee6d87 100644 --- a/pkg/addons/contract/tooldefs.go +++ b/pkg/addons/contract/tooldefs.go @@ -77,6 +77,10 @@ type ToolDefinition struct { // say about the provider. Only a tool that shadows bash may declare it, and // the manifest must then accept API 2 (see MinExecutionAPI). Execution *ExecutionDescriptor `json:"execution,omitempty"` + // LegacyArgs name arguments that an older version of the tool took and + // that it refuses now, with a hint for the model (see LegacyArgument). The + // manifest must accept API 2 (see MinLegacyArgsAPI). + LegacyArgs []LegacyArgument `json:"legacyArgs,omitempty"` } // ReplaceableTool says what a tool that shadows a built-in chat tool must look @@ -229,6 +233,9 @@ func validateToolDefinitions(defs []ToolDefinition, tools []string) error { if err := d.Execution.validate(d); err != nil { return err } + if err := validateLegacyArgs(d); err != nil { + return err + } if d.Shadows != "" { if err := ValidateShadow(d); err != nil { return err diff --git a/scripts/addons/suites.json b/scripts/addons/suites.json index e70e01d905..2cb7d71262 100644 --- a/scripts/addons/suites.json +++ b/scripts/addons/suites.json @@ -404,6 +404,14 @@ "fails closed for a commands tool without a descriptor", "names nothing for a descriptor that does not supply the runtime of documents", "names nothing for an add-on without a commands tool", + "reads the default slots from data", + "lets an add-on declare a slot the contract has never heard of", + "needs the manifest to accept API 2 to declare a slot", + "keeps a skill that shadows a default slot and declares nothing working", + "refuses a shadow of an unknown slot without a declaration, and bad declarations", + "answers with the hint when a call carries the old argument", + "says nothing without the argument, without a declaration or for a forbidden property", + "is validated: bounded, plain, not in the schema, and API 2", "reports network-policy enforcement only for an available isolated provider", "refuses a non-boolean enforcement claim", "accepts a listing declared with API 2", @@ -3504,7 +3512,8 @@ "go: are listed in any state, with the names they replace and whether they run", "go: are listed in any state, with their files and whether they run", "go: keeps its namespaced name and carries the built-in it shadows, only for a replaceable name", - "go: names the replacement string for legacy arguments at the chat boundary without sending them", + "go: names the replacement string for the legacy arguments the shipped Office manifest declares, without sending them", + "go: works for any add-on that declares legacyArgs, and for none that does not", "go: synthesizes exactly one strict host read, without enumerating resources at discovery", "go: projects only opaque IDs, optional names and caller roles through a normal data envelope", "go: returns an array even for empty success and truthfully bounds sorted 100/101 results",