diff --git a/core/http/react-ui/e2e/connector-fixtures.js b/core/http/react-ui/e2e/connector-fixtures.js index 7ea5393f7b..39eb95dfa3 100644 --- a/core/http/react-ui/e2e/connector-fixtures.js +++ b/core/http/react-ui/e2e/connector-fixtures.js @@ -50,32 +50,33 @@ const AGENT_GRANTS = ['memory.write', 'files.upload', 'triggers.propose', 'trigg // A stand-in for the host's access preview: the same shape and the same order // of checks (agent permission, connection permission, sender trust), written // from the form values of the request. -// The words the host uses for each capability in its sentences. +// The verb phrases the host uses for each capability in its sentences. const WORDS = { - 'proposals.confirm': 'confirming proposals', 'memory.write': 'saving to memory', 'files.upload': 'receiving files', - 'triggers.propose': 'reminders and scheduled tasks', 'triggers.run': 'running scheduled tasks', - 'talkback.channels': 'writing to other channels', 'talkback.new_channel': 'starting conversations in new channels', - 'tools.write': 'changes made by add-on tools', + 'proposals.confirm': 'confirm proposals', 'memory.write': 'save notes to memory', 'files.upload': 'receive files', + 'triggers.propose': 'set reminders and scheduled tasks', 'triggers.run': 'run scheduled tasks', + 'talkback.channels': 'write to other channels', 'talkback.new_channel': 'start conversations in new channels', + 'profile.write': "change the agent's rules", 'skills.write': 'save skills', 'pages.write': 'publish pages', + 'dashboard.write': 'change the dashboard', 'tools.write': 'change things in other services with add-on tools', } export function previewFor(body, roles = []) { const agent = AGENTS.find(a => a.id === body.agent_id) const decide = (c, who) => { - if (!body.agent_id) return { capability: c.name, allowed: false, code: 'no_agent', blocked_by: 'connection', message: 'This connection has no agent.', fix: { kind: 'open_connection', target: 'agent_id' }, then_check: ['sender'] } + if (!body.agent_id) return { capability: c.name, allowed: false, code: 'no_agent', blocked_by: 'connection', message: 'This connection has no agent, so the bot cannot answer or act yet.', fix: { kind: 'open_connection', target: 'agent_id' }, then_check: ['sender'] } if (agent && AGENT_GRANTS.includes(c.name) && !agent.builtins.includes(c.name)) { - return { capability: c.name, allowed: false, code: 'agent_permission_off', blocked_by: 'agent', message: `The agent has not been given permission for ${WORDS[c.name]}.`, fix: { kind: 'open_agent', target: agent.id, field: c.name }, then_check: ['connection', 'sender'] } + return { capability: c.name, allowed: false, code: 'agent_permission_off', blocked_by: 'agent', message: `The agent is not allowed to ${WORDS[c.name]}. Turn on the matching permission in the agent.`, fix: { kind: 'open_agent', target: agent.id, field: c.name }, then_check: ['connection', 'sender'] } } if (!(body.caps || []).includes(c.name)) { - return { capability: c.name, allowed: false, code: 'connection_permission_off', blocked_by: 'connection', message: `This connection does not allow ${WORDS[c.name]}.`, fix: { kind: 'open_connection', target: 'caps', field: c.name }, then_check: ['sender'] } + return { capability: c.name, allowed: false, code: 'connection_permission_off', blocked_by: 'connection', message: `This connection does not allow the bot to ${WORDS[c.name]}. Tick the matching permission on the connection.`, fix: { kind: 'open_connection', target: 'caps', field: c.name }, then_check: ['sender'] } } - if (who === 'anyone') return { capability: c.name, allowed: false, code: 'channel_not_trusted', blocked_by: 'sender', message: `This person is let in through a channel, or because the bot accepts anyone, so they are not trusted for ${WORDS[c.name]}. Add them by name or pair them.`, fix: { kind: 'add_person', target: 'allow' } } + if (who === 'anyone') return { capability: c.name, allowed: false, code: 'channel_not_trusted', blocked_by: 'sender', message: `People in a channel are not trusted one by one, so the bot cannot ${WORDS[c.name]} for them. Add the person to the allowed people.`, fix: { kind: 'add_person', target: 'allow' } } if (who.startsWith('role:')) { const id = who.slice(5) return (body.grants || []).some(g => g.role === id && g.caps.includes(c.name)) && !c.sensitive ? { capability: c.name, allowed: true, via: who } - : { capability: c.name, allowed: false, code: 'role_not_granted', blocked_by: 'sender', message: `The role of this person was not given ${WORDS[c.name]}.`, fix: { kind: 'open_connection', target: 'grants' } } + : { capability: c.name, allowed: false, code: 'role_not_granted', blocked_by: 'sender', message: `The role of this person does not let the bot ${WORDS[c.name]}. Allow it for that role on the connection.`, fix: { kind: 'open_connection', target: 'grants' } } } - if (c.sensitive && who !== 'paired') return { capability: c.name, allowed: false, code: 'sender_needs_pairing', blocked_by: 'sender', message: `This person has to be paired with the bot by the owner before ${WORDS[c.name]}.`, fix: { kind: 'pair_sender', target: 'pairing' } } + if (c.sensitive && who !== 'paired') return { capability: c.name, allowed: false, code: 'sender_needs_pairing', blocked_by: 'sender', message: `This person needs to be paired with this bot before the bot can ${WORDS[c.name]} for them.`, fix: { kind: 'pair_sender', target: 'pairing' } } return { capability: c.name, allowed: true, via: who === 'paired' ? 'paired' : 'allowlist' } } const subjects = [ diff --git a/core/http/react-ui/e2e/connector-permissions.spec.js b/core/http/react-ui/e2e/connector-permissions.spec.js index cdcca89bb3..1e1a30fda8 100644 --- a/core/http/react-ui/e2e/connector-permissions.spec.js +++ b/core/http/react-ui/e2e/connector-permissions.spec.js @@ -325,7 +325,7 @@ test.describe('Connector permissions', () => { // The agent lacks the permission: the first blocker is the agent, and the fix opens the agent in a new tab. const trig = can.getByTestId('connector-can-triggers.propose') await expect(trig).toHaveAttribute('data-allowed', 'false') - await expect(trig.getByTestId('connector-can-why')).toContainText('has not been given permission') + await expect(trig.getByTestId('connector-can-why')).toContainText('is not allowed to set reminders and scheduled tasks') await expect(trig.getByTestId('connector-fix')).toHaveAttribute('href', '/app/agents/a3/edit') await expect(trig.getByTestId('connector-fix')).toHaveAttribute('target', '_blank') await expect(trig.getByTestId('connector-fix')).toHaveCount(1) @@ -360,7 +360,7 @@ test.describe('Connector permissions', () => { await expect(page.getByTestId('connector-pair-start')).toBeFocused() // A member of the listed channel is not trusted one by one: add the person. await can.getByTestId('connector-can-sender').selectOption('channel_member') - await expect(can.getByTestId('connector-can-memory.write').getByTestId('connector-can-why')).toContainText('not trusted for saving to memory') + await expect(can.getByTestId('connector-can-memory.write').getByTestId('connector-can-why')).toContainText('cannot save notes to memory for them') await can.getByTestId('connector-can-memory.write').getByRole('button', { name: 'Add a person' }).click() await expect(page.getByTestId('connector-allow').getByRole('textbox')).toBeFocused() }) @@ -486,7 +486,7 @@ test.describe('Connector permissions', () => { // The agent lacks Propose triggers: the row shows the host sentence and a link to the agent. const trig = ready.getByTestId('connector-ready-triggers') await expect(trig).toHaveAttribute('data-state', 'attention') - await expect(trig).toContainText('has not been given permission') + await expect(trig).toContainText('is not allowed to set reminders and scheduled tasks') await expect(trig.getByTestId('connector-fix')).toHaveAttribute('href', '/app/agents/a3/edit') // Choosing an agent that has it changes the row without a save; the row then works after a save. await page.getByLabel('Agent', { exact: true }).selectOption('a1') diff --git a/core/services/studio/connectordenied.go b/core/services/studio/connectordenied.go index 7e6eb001a7..91e106dcb5 100644 --- a/core/services/studio/connectordenied.go +++ b/core/services/studio/connectordenied.go @@ -3,6 +3,8 @@ package studio import ( "slices" "strings" + "unicode" + "unicode/utf8" "github.com/mudler/LocalAI/core/services/studio/builtins" "github.com/mudler/LocalAI/core/services/studio/session" @@ -12,7 +14,7 @@ import ( // and not every tool, stays short, and goes first in the note of the turn. const ( deniedNoteStart = "Held back from this sender: " - deniedNoteEnd = " If asked for one of these, say why; never say it does not exist." + deniedNoteEnd = " If asked for one of these, relay the reason plainly; never say it does not exist." deniedNoteBudget = 400 - len(deniedNoteStart) - len(deniedNoteEnd) - 1 ) @@ -28,7 +30,7 @@ func deniedToolsNote(rec session.ChatRecord, built []builtins.Built) string { prefix := builtins.ReasonNotPermitted + ": " var words []string for name, w := range capabilityWords { - words = append(words, name, w) + words = append(words, name, w.Noun) } say := strings.NewReplacer(words...) var reasons []string @@ -37,7 +39,7 @@ func deniedToolsNote(rec session.ChatRecord, built []builtins.Built) string { if b.OK || !ok || why == "" { continue } - if why = say.Replace(why); !slices.Contains(reasons, why) { + if why = sentence(say.Replace(why)); !slices.Contains(reasons, why) { reasons = append(reasons, why) } } @@ -45,14 +47,31 @@ func deniedToolsNote(rec session.ChatRecord, built []builtins.Built) string { var kept []string used := 0 for _, why := range reasons { - if used+len(why)+2 > deniedNoteBudget { + if used+len(why)+1 > deniedNoteBudget { break } kept = append(kept, why) - used += len(why) + 2 + used += len(why) + 1 } if len(kept) == 0 { return "" } - return deniedNoteStart + strings.Join(kept, "; ") + "." + deniedNoteEnd + return deniedNoteStart + strings.Join(kept, " ") + deniedNoteEnd +} + +// sentence makes a reason a whole sentence: it starts with a capital and ends +// with a full stop. The reasons of the table already are; a reason that a tool +// wrote on its own may not be. +func sentence(s string) string { + s = strings.TrimSpace(s) + if s == "" { + return s + } + if r, n := utf8.DecodeRuneInString(s); unicode.IsLower(r) { + s = string(unicode.ToUpper(r)) + s[n:] + } + if !strings.HasSuffix(s, ".") { + s += "." + } + return s } diff --git a/core/services/studio/connectordenied_internal_test.go b/core/services/studio/connectordenied_internal_test.go index 382c62645f..9e7fc20ca7 100644 --- a/core/services/studio/connectordenied_internal_test.go +++ b/core/services/studio/connectordenied_internal_test.go @@ -25,9 +25,9 @@ var _ = Describe("the note about tools held back from a sender", func() { {ID: "bash", Reason: builtins.ReasonOff}, }) Expect(note).To(HavePrefix("Held back from this sender: ")) - Expect(note).To(ContainSubstring("the owner has not enabled reminders and scheduled tasks for this bot"), "said once for both tools") + Expect(note).To(ContainSubstring("The owner has not enabled reminders and scheduled tasks for this bot."), "said once for both tools") Expect(strings.Count(note, "reminders")).To(Equal(1)) - Expect(note).To(ContainSubstring("starting conversations in new channels needs a sender that the owner paired with this bot")) + Expect(note).To(ContainSubstring("Starting conversations in new channels needs a sender that the owner paired with this bot.")) Expect(note).To(ContainSubstring("never say it does not exist")) Expect(note).NotTo(ContainSubstring("triggers.propose")) Expect(note).NotTo(ContainSubstring("bash")) diff --git a/core/services/studio/connectorreasons.go b/core/services/studio/connectorreasons.go index d2b39e9745..7e7bb7c20e 100644 --- a/core/services/studio/connectorreasons.go +++ b/core/services/studio/connectorreasons.go @@ -1,8 +1,8 @@ package studio import ( - "fmt" "slices" + "strings" "github.com/mudler/LocalAI/pkg/addons/contract" ) @@ -110,10 +110,13 @@ type Block struct { func (b Block) Off() bool { return b.Code != "" || b.Reason != "" } // blockInfo is a row of the table: who holds the link, what the owner reads, -// and what the person who asked (and the model) is told. +// and what the person who asked (and the model) is told. Both texts are whole +// sentences. {action} stands for the verb phrase of the capability ("set +// reminders"), so a template reads "the bot cannot {action}" and never needs a +// noun where a verb belongs. type blockInfo struct { By string - // Message is for the owner. %s is the capability in words. + // Message is for the owner, on the connection page. Message string // Say is for the sender and the model. It names no setting, host or // error code, and goes to the person who asked, never to a channel. @@ -121,44 +124,76 @@ type blockInfo struct { } var blockTable = map[string]blockInfo{ - CodeNoAgent: {ByConnection, "This connection has no agent.", "this bot has no agent"}, - CodeUnknownCapability: {ByServer, "That is not a permission this server knows.", "that is not something this bot can be allowed to do"}, - CodeServerSettingOff: {ByServer, "An administrator turned %s off for this whole server.", "%s is turned off on this server; an administrator can change that"}, - CodeToolsSwitchOff: {ByTools, "The tools for %s are switched off for the agent.", "the owner has switched off the tools for %s"}, - CodeAgentPermissionOff: {ByAgent, "The agent has not been given permission for %s.", "the owner has not given the agent permission for %s"}, - CodeAgentUnavailable: {ByAgent, "The agent of this connection cannot be read.", "the agent of this bot cannot be read"}, - CodeConnectionPermissionOff: {ByConnection, "This connection does not allow %s.", "the owner has not allowed %s for this bot"}, - CodeSenderNotListed: {BySender, "This person is not on the list of people the bot answers.", "you are not on the list of people this bot answers"}, - CodeSenderNeedsPairing: {BySender, "This person has to be paired with the bot by the owner before %s.", "%s needs you to be paired with this bot by the owner"}, - CodeChannelNotTrusted: {BySender, "This person is let in through a channel, or because the bot accepts anyone, so they are not trusted for %s. Add them by name or pair them.", "%s is only for people the owner added by name or paired"}, - CodeRoleNotGranted: {BySender, "The role of this person was not given %s.", "the owner has not allowed %s for your role"}, - CodeKindNotRunning: {BySender, "The app for this connection is not running, so roles cannot be checked.", "the app for this bot is not running"}, - CodeContextRule: {ByConnection, "%s is not allowed in this place.", "%s is not allowed in this place"}, + CodeNoAgent: {ByConnection, + "This connection has no agent, so the bot cannot answer or act yet.", + "This bot has no agent set up yet."}, + CodeUnknownCapability: {ByServer, + "This server does not know that permission.", + "That is not something this bot can be allowed to do."}, + CodeServerSettingOff: {ByServer, + "An administrator turned this off for the whole server, so the bot cannot {action}.", + "An administrator turned this off on the server, so the bot cannot {action}."}, + CodeToolsSwitchOff: {ByTools, + "The tools the bot needs to {action} are switched off for the agent. Switch them on in the agent.", + "The owner switched off the tools the bot needs to {action}."}, + CodeAgentPermissionOff: {ByAgent, + "The agent is not allowed to {action}. Turn on the matching permission in the agent.", + "The owner has not allowed the agent to {action}."}, + CodeAgentUnavailable: {ByAgent, + "The agent of this connection cannot be read right now. Check that it still exists.", + "The agent of this bot cannot be read right now."}, + CodeConnectionPermissionOff: {ByConnection, + "This connection does not allow the bot to {action}. Tick the matching permission on the connection.", + "The owner has not allowed this bot to {action}."}, + CodeSenderNotListed: {BySender, + "This person is not on the list of people the bot answers. Add them to the allowed people.", + "You are not on the list of people this bot answers."}, + CodeSenderNeedsPairing: {BySender, + "This person needs to be paired with this bot before the bot can {action} for them.", + "You need to be paired with this bot by the owner before it can {action} for you."}, + CodeChannelNotTrusted: {BySender, + "People in a channel are not trusted one by one, so the bot cannot {action} for them. Add the person to the allowed people.", + "This bot can {action} only for people the owner added by name or paired, and you are not one of them."}, + CodeRoleNotGranted: {BySender, + "The role of this person does not let the bot {action}. Allow it for that role on the connection.", + "The owner has not allowed this bot to {action} for your role."}, + CodeKindNotRunning: {BySender, + "The app for this connection is not running, so roles cannot be checked. Start the app.", + "The app for this bot is not running, so it cannot check what you may do."}, + CodeContextRule: {ByConnection, + "The bot cannot {action} in this place.", + "This bot cannot {action} in this place."}, } -// capabilityWords says a capability in the words a person uses. The reasons of -// Authorize name the capability; the model and the owner's page should not -// repeat a key that the person never saw. -var capabilityWords = map[string]string{ - contract.CapProposalsConfirm: "confirming proposals", - contract.CapMemoryWrite: "saving to memory", - contract.CapFilesUpload: "receiving files", - contract.CapTriggersPropose: "reminders and scheduled tasks", - contract.CapTriggersRun: "running scheduled tasks", - contract.CapTalkbackChannels: "writing to other channels", - contract.CapTalkbackNewChan: "starting conversations in new channels", - contract.CapProfileWrite: "changing the agent's rules", - contract.CapSkillsWrite: "saving skills", - contract.CapPagesWrite: "publishing pages", - contract.CapDashboardWrite: "changing the dashboard", - contract.CapToolsWrite: "changes made by add-on tools", +// capabilityPhrase says a capability in the words a person uses. Action is a +// verb phrase in the infinitive and fills "the bot cannot ...". Noun names the +// same thing as a noun phrase, for the places that cannot rewrite a sentence +// (see deniedToolsNote). The reasons of Authorize name the capability; the +// model and the owner's page should not repeat a key that the person never saw. +type capabilityPhrase struct{ Action, Noun string } + +var capabilityWords = map[string]capabilityPhrase{ + contract.CapProposalsConfirm: {"confirm proposals", "confirming proposals"}, + contract.CapMemoryWrite: {"save notes to memory", "saving to memory"}, + contract.CapFilesUpload: {"receive files", "receiving files"}, + contract.CapTriggersPropose: {"set reminders and scheduled tasks", "reminders and scheduled tasks"}, + contract.CapTriggersRun: {"run scheduled tasks", "running scheduled tasks"}, + contract.CapTalkbackChannels: {"write to other channels", "writing to other channels"}, + contract.CapTalkbackNewChan: {"start conversations in new channels", "starting conversations in new channels"}, + contract.CapProfileWrite: {"change the agent's rules", "changing the agent's rules"}, + contract.CapSkillsWrite: {"save skills", "saving skills"}, + contract.CapPagesWrite: {"publish pages", "publishing pages"}, + contract.CapDashboardWrite: {"change the dashboard", "changing the dashboard"}, + contract.CapToolsWrite: {"change things in other services with add-on tools", "changes made by add-on tools"}, } -func wordsOf(capability string) string { +// actionOf is the verb phrase of a capability, "do this" for a name that is +// not on the list. +func actionOf(capability string) string { if w, ok := capabilityWords[capability]; ok { - return w + return w.Action } - return "this" + return "do this" } // blockMessage is what the owner reads for a code. @@ -167,7 +202,7 @@ func blockMessage(code, capability string) string { if !ok { return "" } - return sprintfOnce(row.Message, wordsOf(capability)) + return strings.ReplaceAll(row.Message, "{action}", actionOf(capability)) } // BlockSay is what the person who asked, and the model, is told for a code: @@ -177,16 +212,7 @@ func BlockSay(code, capability string) string { if !ok { return "" } - return sprintfOnce(row.Say, wordsOf(capability)) -} - -func sprintfOnce(format, arg string) string { - for i := 0; i+1 < len(format); i++ { - if format[i] == '%' && format[i+1] == 's' { - return fmt.Sprintf(format, arg) - } - } - return format + return strings.ReplaceAll(row.Say, "{action}", actionOf(capability)) } // Say is the sentence for the person who asked and for the model. A refusal diff --git a/core/services/studio/connectorreasons_internal_test.go b/core/services/studio/connectorreasons_internal_test.go index 279d4fcf23..fbcec5606c 100644 --- a/core/services/studio/connectorreasons_internal_test.go +++ b/core/services/studio/connectorreasons_internal_test.go @@ -131,7 +131,7 @@ var _ = Describe("the codes of a refusal", func() { It("says a refusal for the person who asked from the code, and keeps the reason of a rule", func() { d := Authorize(binding, &kind, listed, contract.CapProfileWrite, now, Block{}) - Expect(d.Say()).To(Equal("the owner has not allowed changing the agent's rules for this bot")) + Expect(d.Say()).To(Equal("The owner has not allowed this bot to change the agent's rules.")) Expect(Decision{Reason: "not in this thread"}.Say()).To(Equal("not in this thread")) }) }) diff --git a/core/services/studio/connectorreasons_texts_internal_test.go b/core/services/studio/connectorreasons_texts_internal_test.go new file mode 100644 index 0000000000..dfbf2980be --- /dev/null +++ b/core/services/studio/connectorreasons_texts_internal_test.go @@ -0,0 +1,132 @@ +package studio + +import ( + "regexp" + "strings" + "unicode" + + "github.com/mudler/LocalAI/core/services/studio/builtins" + "github.com/mudler/LocalAI/core/services/studio/session" + "github.com/mudler/LocalAI/pkg/addons/contract" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// The sentences of the table are read by people: the owner on the connection +// page, the sender in the chat, the model in its note. These specs render every +// code for every capability and hold the grammar of each result, so a new +// capability phrase cannot slip in a noun where a verb belongs. +var _ = Describe("the texts of a refusal", func() { + // verbs a capability action may start with: a new action needs a new entry. + actionVerbs := []string{"confirm", "save", "receive", "set", "run", "write", "start", "change", "publish"} + names := append(contract.ConnectorCapabilityNames(), "") + internal := regexp.MustCompile(`[a-z]+\.[a-z_]+|_|https?:|\d+\.\d+\.\d+|[{}%]|(?i:triggers?\.|localai_|bash)`) + const ( + messageMax = 200 + sayMax = 150 + ) + + It("has an action phrase, starting with a verb, for every capability of the closed list", func() { + Expect(capabilityWords).To(HaveLen(len(contract.ConnectorCapabilityNames()))) + for _, name := range contract.ConnectorCapabilityNames() { + w, ok := capabilityWords[name] + Expect(ok).To(BeTrue(), "no words for %s", name) + first, _, _ := strings.Cut(w.Action, " ") + Expect(actionVerbs).To(ContainElement(first), "%s: %q must start with a verb of the list", name, w.Action) + Expect(w.Action).To(Equal(strings.ToLower(w.Action)), name) + Expect(w.Noun).NotTo(BeEmpty(), name) + Expect(w.Noun).NotTo(HavePrefix(first+" "), "%s: the noun phrase must not be the action", name) + for _, text := range []string{w.Action, w.Noun} { + Expect(text).NotTo(ContainSubstring(" "), name) + Expect(text).NotTo(MatchRegexp(`[.,;:]`), "%s: a phrase has no punctuation: %q", name, text) + Expect(internal.MatchString(text)).To(BeFalse(), "%s: %q", name, text) + } + } + }) + + It("renders a whole, plain sentence for every code and capability", func() { + for _, code := range BlockCodes() { + for _, name := range names { + for kind, text := range map[string]string{"message": blockMessage(code, name), "say": BlockSay(code, name)} { + where := code + "/" + name + "/" + kind + ": " + text + Expect(text).NotTo(BeEmpty(), where) + Expect(text).To(HaveSuffix("."), where) + Expect([]rune(text)[0]).To(Satisfy(unicode.IsUpper), "starts with a capital: %s", where) + Expect(text).NotTo(ContainSubstring(" "), where) + Expect(text).NotTo(ContainSubstring("localai"), "write LocalAI: %s", where) + Expect(text).NotTo(ContainSubstring(" ."), where) + Expect(text).NotTo(ContainSubstring(".."), where) + Expect(strings.TrimSpace(text)).To(Equal(text), where) + Expect(internal.MatchString(text)).To(BeFalse(), "no key, setting, host or code: %s", where) + Expect(text).NotTo(MatchRegexp(`(?i)permission for|allowed for [a-z]+ing|given [a-z]+ing`), "a noun slot filled with a verb: %s", where) + Expect(strings.Count(text, ".")).To(BeNumerically("<=", 2), "one or two sentences: %s", where) + for _, key := range contract.ConnectorCapabilityNames() { + Expect(text).NotTo(ContainSubstring(key), where) + } + limit := messageMax + if kind == "say" { + limit = sayMax + } + Expect(len(text)).To(BeNumerically("<=", limit), where) + } + } + } + }) + + It("says the action inside the sentence for every capability that has one", func() { + for _, code := range BlockCodes() { + if !strings.Contains(blockTable[code].Message, "{action}") { + continue + } + for _, name := range contract.ConnectorCapabilityNames() { + Expect(blockMessage(code, name)).To(ContainSubstring(capabilityWords[name].Action), code+"/"+name) + Expect(BlockSay(code, name)).To(ContainSubstring(capabilityWords[name].Action), code+"/"+name) + } + } + }) + + It("keeps the note of held-back tools in the budget with the longest sentences", func() { + conn := session.ChatRecord{Origin: session.OriginConnector} + for _, code := range BlockCodes() { + var built []builtins.Built + for _, name := range contract.ConnectorCapabilityNames() { + built = append(built, builtins.Built{ID: name, Reason: builtins.ReasonNotPermitted + ": " + BlockSay(code, name)}) + } + note := deniedToolsNote(conn, built) + Expect(len(note)).To(BeNumerically("<=", 400), code) + Expect(note).To(HavePrefix(deniedNoteStart), code) + Expect(note).To(ContainSubstring("relay the reason plainly"), code) + Expect(note).NotTo(ContainSubstring(" "), code) + // At least one reason fits, and every one that fits is whole. + Expect(note).To(MatchRegexp(`: [A-Z][^;]*\.`), code) + } + }) + + // The six refusals people meet most, written out in full. + golden := []struct{ code, capability, message, say string }{ + {CodeConnectionPermissionOff, contract.CapTriggersPropose, + "This connection does not allow the bot to set reminders and scheduled tasks. Tick the matching permission on the connection.", + "The owner has not allowed this bot to set reminders and scheduled tasks."}, + {CodeAgentPermissionOff, contract.CapMemoryWrite, + "The agent is not allowed to save notes to memory. Turn on the matching permission in the agent.", + "The owner has not allowed the agent to save notes to memory."}, + {CodeSenderNeedsPairing, contract.CapTalkbackNewChan, + "This person needs to be paired with this bot before the bot can start conversations in new channels for them.", + "You need to be paired with this bot by the owner before it can start conversations in new channels for you."}, + {CodeChannelNotTrusted, contract.CapMemoryWrite, + "People in a channel are not trusted one by one, so the bot cannot save notes to memory for them. Add the person to the allowed people.", + "This bot can save notes to memory only for people the owner added by name or paired, and you are not one of them."}, + {CodeRoleNotGranted, contract.CapTalkbackChannels, + "The role of this person does not let the bot write to other channels. Allow it for that role on the connection.", + "The owner has not allowed this bot to write to other channels for your role."}, + {CodeToolsSwitchOff, contract.CapToolsWrite, + "The tools the bot needs to change things in other services with add-on tools are switched off for the agent. Switch them on in the agent.", + "The owner switched off the tools the bot needs to change things in other services with add-on tools."}, + } + for _, g := range golden { + It("says "+g.code+" for "+g.capability+" the same way every time", func() { + Expect(blockMessage(g.code, g.capability)).To(Equal(g.message)) + Expect(BlockSay(g.code, g.capability)).To(Equal(g.say)) + }) + } +}) diff --git a/core/services/studio/connectors_enforce_test.go b/core/services/studio/connectors_enforce_test.go index 0f0a722e2b..499f30d197 100644 --- a/core/services/studio/connectors_enforce_test.go +++ b/core/services/studio/connectors_enforce_test.go @@ -116,7 +116,7 @@ var _ = Describe("permissions in the turns of a connector chat", func() { ask("11", []string{"admin"}, "profile") res = r.h.ResultsFor("propose_change") Expect(res).To(HaveLen(2)) - Expect(res[1].Err).To(MatchError(ContainSubstring("not permitted: changing the agent's rules needs you to be paired with this bot by the owner"))) + Expect(res[1].Err).To(MatchError(ContainSubstring("not permitted: You need to be paired with this bot by the owner before it can change the agent's rules for you."))) list, _ := r.svc.Changes().List(ctx, alice) Expect(list).To(HaveLen(1), "the refused change made no proposal") }) diff --git a/core/services/studio/connectors_reasons_test.go b/core/services/studio/connectors_reasons_test.go index 738e062714..bf98152d25 100644 --- a/core/services/studio/connectors_reasons_test.go +++ b/core/services/studio/connectors_reasons_test.go @@ -69,7 +69,7 @@ var _ = Describe("why a capability is blocked on a connection", func() { Expect(d.BlockedBy).To(Equal(studio.ByConnection)) Expect(d.Fix).To(Equal(&studio.Fix{Kind: studio.FixOpenConnection, Target: "caps", Field: propose})) Expect(d.ThenCheck).To(Equal([]string{studio.BySender, studio.ByApproval})) - Expect(d.Message).To(Equal("This connection does not allow reminders and scheduled tasks.")) + Expect(d.Message).To(Equal("This connection does not allow the bot to set reminders and scheduled tasks. Tick the matching permission on the connection.")) Expect(d.Reason).NotTo(BeEmpty()) Expect(decision(saved(), "listed", contract.CapMemoryWrite).Allowed).To(BeTrue()) }) diff --git a/core/services/studio/connectortalkback_test.go b/core/services/studio/connectortalkback_test.go index fa246c7242..7a560b51df 100644 --- a/core/services/studio/connectortalkback_test.go +++ b/core/services/studio/connectortalkback_test.go @@ -141,7 +141,7 @@ var _ = Describe("talk-back from a connector chat", func() { Expect(chatTools()).NotTo(ContainElement("send_message")) l, _ := r.svc.List(ctx, alice, 10) prompt := strings.Join(r.h.SentRaw(l[0].ID), "\n") - Expect(prompt).To(ContainSubstring("writing to other channels is only for people the owner added by name or paired"), "the model is told why") + Expect(prompt).To(ContainSubstring("This bot can write to other channels only for people the owner added by name or paired"), "the model is told why") Expect(prompt).To(ContainSubstring("never say it does not exist")) }) diff --git a/docs/content/features/chats-connectors.md b/docs/content/features/chats-connectors.md index 3fabfeabef..3549140d40 100644 --- a/docs/content/features/chats-connectors.md +++ b/docs/content/features/chats-connectors.md @@ -809,7 +809,7 @@ a page can turn into a sentence and a link, in the first place the check failed: "reason": "the owner has not enabled triggers.propose for this bot", "code": "connection_permission_off", "blocked_by": "connection", - "message": "This connection does not allow reminders and scheduled tasks.", + "message": "This connection does not allow the bot to set reminders and scheduled tasks. Tick the matching permission on the connection.", "fix": {"kind": "open_connection", "target": "caps", "field": "triggers.propose"}, "then_check": ["sender", "approval"] } @@ -851,9 +851,15 @@ The server sends no URL. A page maps `fix.kind` and `fix.target` to its own link set of codes is closed, and the recent decisions carry `code` too. The same codes feed the words a sender and the model get when a tool is held back. The -note for the model and the tool error use one plain sentence per code ("the owner has -not allowed reminders and scheduled tasks for this bot"), with no setting names, host -names or error codes. The host never posts it to a channel on its own. +note for the model and the tool error use one whole sentence per code ("The owner has +not allowed this bot to set reminders and scheduled tasks."), with no setting names, host +names or error codes. The note tells the model to relay the reason plainly. The host +never posts it to a channel on its own. + +Each capability has a verb phrase ("set reminders and scheduled tasks") that fills the +sentence of every code, so `message` always reads as a whole sentence, for example +"People in a channel are not trusted one by one, so the bot cannot save notes to memory +for them. Add the person to the allowed people." ### Preview before saving