diff --git a/core/services/studio/attachfile.go b/core/services/studio/attachfile.go index 9033755d57..e57aa3767d 100644 --- a/core/services/studio/attachfile.go +++ b/core/services/studio/attachfile.go @@ -18,7 +18,9 @@ const attachFileName = "attach_file" const attachFileDescription = "Give the person a file that already exists in your files: it is sent to them with your answer. " + "Files you make or change in a turn are attached without this call; use it for a file from an earlier turn or from a folder you were given. " + - "path is the file's path as your file tools show it. A file that cannot be attached comes back with the reason: tell the person plainly." + "path is the exact path that your file tools or bash showed, such as /workspace/home/Docs/plan.pdf. " + + "Only this call sends a file: never write a file name or a paperclip line as if it were sent. " + + "A file that cannot be attached comes back with the reason: tell the person plainly." const attachFileSchema = `{"type":"object","properties":{"path":{"type":"string","description":"The path of the file, such as /workspace/chat/report.docx."}},"required":["path"]}` diff --git a/core/services/studio/connectorfiles.go b/core/services/studio/connectorfiles.go index 06ae4dbd8f..0dd8b6cec1 100644 --- a/core/services/studio/connectorfiles.go +++ b/core/services/studio/connectorfiles.go @@ -98,8 +98,9 @@ func (c *Connectors) outgoing(ctx context.Context, b connectors.Binding, x sessi // sendFiles hands the files of a turn to the sender after the answer: those // the turn made and those the agent attached. A file that cannot go is named -// in one plain message, with the reason. -func (c *Connectors) sendFiles(b connectors.Binding, token string, m contract.ConnectorMessage, chatID string, turn int, since time.Time) { +// in one plain message, with the reason. When the answer presents a file and +// none went, the message says so. +func (c *Connectors) sendFiles(b connectors.Binding, token string, m contract.ConnectorMessage, chatID string, turn int, since time.Time, answer string) { if !b.SendsFiles() { return } @@ -137,6 +138,11 @@ func (c *Connectors) sendFiles(b connectors.Binding, token string, m contract.Co notes = append(notes, note) } } + if sent == 0 && len(notes) == 0 && promisesFile(answer) { + // The answer showed a file and none went: the sender must not wait + // for it. The answer itself is not changed. + notes = append(notes, noteNoFile) + } if len(notes) > 0 { c.say(b, token, m.Chat, m.Thread, strings.Join(notes, "\n")) } diff --git a/core/services/studio/connectorpromise.go b/core/services/studio/connectorpromise.go new file mode 100644 index 0000000000..a58b7c0b47 --- /dev/null +++ b/core/services/studio/connectorpromise.go @@ -0,0 +1,53 @@ +package studio + +import ( + "strings" + "unicode" +) + +// noteNoFile is what the sender reads when the answer showed a file and none +// went with it. +const noteNoFile = "I could not attach the file. Ask me again and I will send it." + +// paperclip is the marker a model writes in place of a file it did not send. +const paperclip = "šŸ“Ž" + +// promisesFile says whether an answer presents a file as if it went with it: +// a paperclip followed at once by a file name of a type that is sent, such as +// "šŸ“Ž `report.pdf`". The host attaches files itself, so a model that writes +// such a line has made the file up or forgot to call attach_file. The test is +// narrow on purpose: a file name in prose, a name in a code block and a +// paperclip alone do not match. +func promisesFile(answer string) bool { + rest := answer + for { + i := strings.Index(rest, paperclip) + if i < 0 { + return false + } + rest = rest[i+len(paperclip):] + if _, ok := outboundType(nameAfterClip(rest)); ok { + return true + } + } +} + +// nameAfterClip reads the file name that starts a text after the paperclip: +// optional spaces, the variation selector of the emoji and light markup, then +// a name in code quotes (it may hold spaces) or up to the next space. +func nameAfterClip(s string) string { + s = strings.TrimLeftFunc(strings.TrimPrefix(s, "ļø"), func(r rune) bool { + return r == ' ' || r == '\t' || r == '*' || r == '_' || r == '"' || r == '\'' + }) + if rest, ok := strings.CutPrefix(s, "`"); ok { + name, _, _ := strings.Cut(rest, "`") + if strings.ContainsAny(name, "\r\n") { + return "" + } + return strings.TrimSpace(name) + } + if i := strings.IndexFunc(s, unicode.IsSpace); i >= 0 { + s = s[:i] + } + return strings.TrimRight(s, ".,;:!?*_\"')]") +} diff --git a/core/services/studio/connectorpromise_internal_test.go b/core/services/studio/connectorpromise_internal_test.go new file mode 100644 index 0000000000..326e67a643 --- /dev/null +++ b/core/services/studio/connectorpromise_internal_test.go @@ -0,0 +1,27 @@ +package studio + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("promisesFile", func() { + DescribeTable("finds the paperclip line of an answer that presents a file", + func(answer string, want bool) { Expect(promisesFile(answer)).To(Equal(want)) }, + Entry("paperclip after a sentence", "Here it is, I'll send you one. šŸ“Ž `design-notes.pdf`", true), + Entry("paperclip line", "Done.\nšŸ“Ž `report.xlsx`", true), + Entry("paperclip with a variation selector", "šŸ“Žļø summary.docx", true), + Entry("plain name", "šŸ“Ž data.csv", true), + Entry("bold name", "šŸ“Ž **notes.md**", true), + Entry("name with spaces in code quotes", "šŸ“Ž `Q3 plan.pdf`", true), + Entry("upper case extension", "šŸ“Ž `SCAN.PDF`", true), + Entry("a name with a trailing full stop", "šŸ“Ž report.pdf.", true), + Entry("paperclip with no file", "I attached it šŸ“Ž", false), + Entry("paperclip with prose after it", "šŸ“Ž attached below, see the summary", false), + Entry("an extension that is not sent", "šŸ“Ž `setup.exe`", false), + Entry("a file name in prose", "The file is `report.pdf` in your folder.", false), + Entry("a file name in a code block", "```\nreport.pdf\n```", false), + Entry("ordinary prose", "I will send you a summary of the file.", false), + Entry("empty", "", false), + ) +}) diff --git a/core/services/studio/connectors.go b/core/services/studio/connectors.go index 823450448c..24437d409b 100644 --- a/core/services/studio/connectors.go +++ b/core/services/studio/connectors.go @@ -1110,9 +1110,10 @@ func (c *Connectors) reply(b connectors.Binding, token string, m contract.Connec if !ended { continue } - ids := c.finish(b, token, m, l, c.answerOf(b, chatID, errText)) + answer := c.answerOf(b, chatID, errText) + ids := c.finish(b, token, m, l, answer) c.remember(b, m, chatID, ids) - c.sendFiles(b, token, m, chatID, turn, since) + c.sendFiles(b, token, m, chatID, turn, since, answer) errText = "" c.smu.Lock() tctx, tcancel := context.WithTimeout(context.Background(), 5*time.Second) diff --git a/core/services/studio/connectors_attach_test.go b/core/services/studio/connectors_attach_test.go index 6fc24121df..555d96558c 100644 --- a/core/services/studio/connectors_attach_test.go +++ b/core/services/studio/connectors_attach_test.go @@ -81,6 +81,72 @@ var _ = Describe("attach_file in a connector chat", func() { Expect(bot.answers()[1]).To(Equal("Here it is."), "the answer comes before the file, and no note follows") }) + It("attaches a file of a folder whose name has a space, by the path the shell shows", func() { + ctx := context.Background() + Expect(sh.files.EnsureHome(ctx, alice)).To(Succeed()) + home := session.HomePath(alice) + _, err := sh.files.Store().Mkdir(ctx, sh.files.Actor(alice), home+"/Design Docs") + Expect(err).NotTo(HaveOccurred()) + _, err = sh.files.Store().Put(ctx, sh.files.Actor(alice), home+"/Design Docs/overview.pdf", strings.NewReader("%PDF-1.4 x"), workspace.PutOptions{Overwrite: true}) + Expect(err).NotTo(HaveOccurred()) + r := telegram(map[string][]studiotest.ToolStep{ + "send": {attach("/workspace/home/Design Docs/overview.pdf"), attach("/Design Docs/overview.pdf"), {Say: "Here it is."}}, + }, studio.ConnectorInput{}) + ask(1, "send") + waitFor("the file", func() bool { return len(bot.sentFiles()) == 1 }) + answered(1) + res := r.h.ResultsFor("attach_file") + Expect(res).To(HaveLen(2)) + Expect(res[0].Err).NotTo(HaveOccurred()) + Expect(res[1].Out).To(ContainSubstring("attached already")) + Expect(bot.sentFiles()[0].Name).To(Equal("overview.pdf")) + }) + + Describe("an answer that shows a file nobody attached", func() { + It("ends with one plain note", func() { + telegram(map[string][]studiotest.ToolStep{ + "fake": {{Say: "Here it is, I'll send you one. šŸ“Ž `design-notes.pdf`"}}, + }, studio.ConnectorInput{}) + ask(1, "fake") + waitFor("the note", func() bool { return len(bot.answers()) == 2 }) + Expect(bot.answers()[0]).To(Equal("Here it is, I'll send you one. šŸ“Ž `design-notes.pdf`"), "the answer is not changed") + Expect(bot.answers()[1]).To(Equal("I could not attach the file. Ask me again and I will send it.")) + Expect(bot.sentFiles()).To(BeEmpty()) + }) + + It("adds no note when the file went with the answer", func() { + telegram(map[string][]studiotest.ToolStep{ + "make": {write("/out/report.csv", "a,b"), {Say: "šŸ“Ž `report.csv`"}}, + }, studio.ConnectorInput{}) + ask(1, "make") + waitFor("the file", func() bool { return len(bot.sentFiles()) == 1 }) + answered(1) + time.Sleep(300 * time.Millisecond) + Expect(bot.answers()).To(HaveLen(1)) + }) + + It("adds no note when the binding sends no files", func() { + r := newConnRig(sh, connStore(), bot, attachScript(map[string][]studiotest.ToolStep{ + "fake": {{Say: "šŸ“Ž `design-notes.pdf`"}}, + }), nil) + r.bind(studio.ConnectorInput{Allow: []string{"11"}, Reply: connectors.ReplyOff, SendFiles: &filesOff}) + ask(1, "fake") + answered(1) + time.Sleep(300 * time.Millisecond) + Expect(bot.answers()).To(HaveLen(1)) + }) + + It("adds no note to an answer that only talks about files", func() { + telegram(map[string][]studiotest.ToolStep{ + "talk": {{Say: "The file is `report.pdf`. I can send it if you want."}}, + }, studio.ConnectorInput{}) + ask(1, "talk") + answered(1) + time.Sleep(300 * time.Millisecond) + Expect(bot.answers()).To(HaveLen(1)) + }) + }) + It("sends a file once when it was made and attached in the same turn", func() { telegram(map[string][]studiotest.ToolStep{ "make": {write("/out/report.csv", "a,b"), attach("/out/report.csv"), attach("/out/report.csv"), {Say: "Made."}}, diff --git a/core/services/studio/connectorturn.go b/core/services/studio/connectorturn.go index 26c65a5734..164c14f6ae 100644 --- a/core/services/studio/connectorturn.go +++ b/core/services/studio/connectorturn.go @@ -46,7 +46,8 @@ func (s *Service) connectorBlock(ctx context.Context, rec session.ChatRecord, ca sb.WriteString(" Files are NOT sent to them: paste short content in the answer, or say you cannot attach a file. Never tell them a file path.") default: fmt.Fprintf(&sb, " Files you make or change in a turn are attached to your answer automatically. To give them a file that already exists, call %s with its path."+ - " Say the file is attached; never say you cannot send files, and never tell them a path."+ + " Use the exact path your file tools or bash showed, for a file in a folder you were given too."+ + " Say the file is attached only after the call worked; never write a file name or a paperclip line yourself, never say you cannot send files, and never tell them a path."+ " Up to %d files per turn, %s each; documents, data, images, audio, video and zip only, no scripts or HTML. If one cannot be attached, say so.", attachFileName, maxFilesPerTurn, limits.Send.sentence(limits.KindTitle)) } diff --git a/docs/content/features/chats-connectors.md b/docs/content/features/chats-connectors.md index 138beb5d39..de44354320 100644 --- a/docs/content/features/chats-connectors.md +++ b/docs/content/features/chats-connectors.md @@ -743,6 +743,12 @@ host chooses the files in two ways: path against the chat's access when the tool is called and again when it sends, drops repeated paths, and counts both kinds in the limit of 5 files a turn. +When an answer shows a file with a paperclip line (for example a paperclip and a file name +such as `report.pdf`) and no file went with it, the host adds one note after the answer: +"I could not attach the file. Ask me again and I will send it." The answer is not changed. +No note is added when a file went, when the binding sends no files, or for a paperclip with +no file name. + An add-on that implements `send_file` gets both, with no code of its own. It must not read chat files or add tools for this. When the platform refuses a file, return a `ConnectorError` with a short, plain `message` (for example the name of a missing