diff --git a/core/http/endpoints/studio/draft.go b/core/http/endpoints/studio/draft.go index be63180c52..b1aaaeedb9 100644 --- a/core/http/endpoints/studio/draft.go +++ b/core/http/endpoints/studio/draft.go @@ -3,9 +3,11 @@ package studio import ( "errors" "net/http" + "strings" "github.com/labstack/echo/v4" "github.com/mudler/LocalAI/core/http/auth" + studiosvc "github.com/mudler/LocalAI/core/services/studio" ) // draftQuery reads the model and the agent of a chat that does not exist yet, @@ -58,11 +60,14 @@ func (h *Handlers) DraftTools(c echo.Context) error { // DraftAddons lists the add-ons a new chat could use. // @Summary List the add-ons a new chat could use. -// @Description What GET /api/chats/{id}/addons lists, for a chat that does not exist yet, as GET /api/chats/draft/tools takes it: the add-ons with their default switch and whether the chat could run commands at its first turn. +// @Description What GET /api/chats/{id}/addons lists, for a chat that does not exist yet, as GET /api/chats/draft/tools takes it: the add-ons with their switch and whether the chat could run commands at its first turn. The draft's current switches may be passed (addons_off, builtins_off, builtins_on: comma-separated lists, as the create request takes them) so the answer is the one the chat would get: with the shell group off, commands is switched_off. // @Tags chats // @Param model query string false "the model of the new chat" // @Param agent_id query string false "a saved agent of yours" -// @Success 200 {object} schema.ChatsAddonList "The add-ons and their default switches" +// @Param addons_off query string false "comma-separated add-on IDs the draft switched off" +// @Param builtins_off query string false "comma-separated built-in groups the draft switched off" +// @Param builtins_on query string false "comma-separated opt-in built-in groups the draft switched on" +// @Success 200 {object} schema.ChatsAddonList "The add-ons and the commands state, with the draft switches applied" // @Failure 400 {object} schema.ErrorResponse "The request is not valid" // @Failure 403 {object} schema.ErrorResponse "Refused" // @Failure 404 {object} schema.ErrorResponse "No such agent" @@ -72,9 +77,26 @@ func (h *Handlers) DraftAddons(c echo.Context) error { if !ok { return nil } - list, cmds, err := h.svc.DraftAddons(c.Request().Context(), userID(c), model, agentID) + list, cmds, err := h.svc.DraftAddons(c.Request().Context(), userID(c), model, agentID, draftSwitches(c)) if err != nil { return failFor(c, err) } return writeAddonList(c, list, cmds) } + +// draftSwitches reads the switches of a draft from the query: comma-separated +// lists, also when the parameter is repeated. The service checks the names. +func draftSwitches(c echo.Context) studiosvc.Switches { + list := func(name string) []string { + var out []string + for _, v := range c.QueryParams()[name] { + for _, item := range strings.Split(v, ",") { + if item = strings.TrimSpace(item); item != "" { + out = append(out, item) + } + } + } + return out + } + return studiosvc.Switches{AddonsOff: list("addons_off"), BuiltinsOff: list("builtins_off"), BuiltinsOn: list("builtins_on")} +} diff --git a/core/http/react-ui/e2e/home-composer.spec.js b/core/http/react-ui/e2e/home-composer.spec.js index 7a82752011..5ce42edb9a 100644 --- a/core/http/react-ui/e2e/home-composer.spec.js +++ b/core/http/react-ui/e2e/home-composer.spec.js @@ -134,6 +134,30 @@ test.describe('Home composer', () => { expect(seen.created).toHaveLength(0) }) + test('a draft with the shell group switched off asks the server and shows the switched off notice', async ({ page }) => { + await server(page) + const asked = [] + await page.route(/\/api\/chats\/draft\/tools(\?.*)?$/, route => route.fulfill({ json: { + builtins: [{ group: 'shell', tools: ['bash'], off: false, usable: true, provider: 'sandbox', providers: { bash: 'sandbox' } }], + addons: [{ id: 'sandbox', tools: 7, off: false }], servers: [], connectors: [], off: [], + } })) + await page.route(/\/api\/chats\/draft\/addons(\?.*)?$/, route => { + const url = new URL(route.request().url()) + asked.push(url.searchParams.get('builtins_off') || '') + const off = url.searchParams.get('builtins_off') === 'shell' + return route.fulfill({ json: { addons: [{ id: 'sandbox', tools: 7, off: false }], commands: off ? 'switched_off' : 'available', commands_provider: 'sandbox', + execution_status: { readiness: 'unknown', isolation: 'unknown', unknown_reason: 'draft' } } }) + }) + await ready(page) + await page.getByTestId('chat-tools-button').click() + await expect(page.getByTestId('chat-commands-notice')).toHaveCount(0) + await page.getByTestId('chat-tools-menu').getByRole('switch', { name: 'Run commands', exact: true }).click() + await expect(page.getByTestId('chat-commands-notice')).toContainText('Running commands is switched off for this chat.') + expect(asked).toContain('shell') + await page.getByTestId('chat-tools-menu').getByRole('switch', { name: 'Run commands', exact: true }).click() + await expect(page.getByTestId('chat-commands-notice')).toHaveCount(0) + }) + test('the draft lists the groups of built-in tools, and one switched off goes with the new chat', async ({ page }) => { const seen = await server(page) await page.route(/\/api\/chats\/draft\/tools(\?.*)?$/, route => route.fulfill({ json: { diff --git a/core/http/react-ui/src/hooks/useChatSwitches.js b/core/http/react-ui/src/hooks/useChatSwitches.js index f61763008c..06c8da3e25 100644 --- a/core/http/react-ui/src/hooks/useChatSwitches.js +++ b/core/http/react-ui/src/hooks/useChatSwitches.js @@ -4,7 +4,7 @@ import { connectorRef, toolsCounts } from '../utils/chatMCP' import { offWith, withOn } from '../utils/chatSwitchRows' import { changedIds, offList, pickerSkills, revertSkills, skillKey, withSkillsSet } from '../utils/chatSkills' import { commandsState, noticeIdentity } from '../utils/chatCommands' -import { applyChoices, draftSwitches as switchesOf, emptyChoices, withChoice } from '../utils/chatDraftSwitches' +import { applyChoices, draftListSwitches, draftSwitches as switchesOf, emptyChoices, withChoice } from '../utils/chatDraftSwitches' const noMCP = { servers: [], connectors: [], off: [] } const mcpOf = (res) => ({ servers: res.servers || [], connectors: res.connectors || [], off: res.off || [] }) @@ -40,6 +40,8 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' const setCommands = (value) => storeCommands({ ...value, context: commandContext.current }) const [mcp, setMcp] = useState(noMCP) const [builtins, setBuiltins] = useState([]) + // The built-in groups as the screen shows them now (also read by the effects below). + const builtinsRef = useRef(builtins) // The built-in tools an installed add-on replaces but cannot serve here now. const [fallbacks, setFallbacks] = useState([]) const [skills, setSkills] = useState([]) @@ -67,7 +69,7 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' setCommands(noCommands) // A draft list keeps the choices made so far for the rows it still has. const chosen = (lists) => (draftOn ? applyChoices(lists, choicesRef.current, connectorRef) : lists) - ;(draftOn ? chatAddonsApi.draft(draftModel, draftAgent) : chatAddonsApi.list(chatId)) + ;(draftOn ? chatAddonsApi.draft(draftModel, draftAgent, draftListSwitches(choicesRef.current, builtinsRef.current)) : chatAddonsApi.list(chatId)) .then((res) => { if (live && epoch === commandEpoch.current) { setAddons(chosen({ addons: res.addons || [] }).addons); setCommands(commandsOf(res)) } }) .catch(() => { if (live && epoch === commandEpoch.current) { setAddons([]); setCommands(noCommands) } }) ;(draftOn ? chatToolsApi.draft(draftModel, draftAgent) : chatToolsApi.list(chatId)) @@ -104,7 +106,6 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' // render: "Select all" in the Tools menu sends one request per kind in a row. const addonsRef = useRef(addons) const mcpRef = useRef(mcp) - const builtinsRef = useRef(builtins) addonsRef.current = addons mcpRef.current = mcp builtinsRef.current = builtins @@ -112,6 +113,18 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' const putMcp = (v) => { mcpRef.current = v; setMcp(v) } const putBuiltins = (v) => { builtinsRef.current = v; setBuiltins(v) } + // A draft has no status of its own: the commands state is what the server + // answers for the draft's choices so far. Asked again after a switch of the + // add-ons or of the built-in groups, so a sandbox switched off shows its + // notice before the first message. The old state stays until the answer + // comes; a later switch drops an answer that is late. + const refreshDraftCommands = useCallback(() => { + const epoch = ++commandEpoch.current + chatAddonsApi.draft(draftModel, draftAgent, draftListSwitches(choicesRef.current, builtinsRef.current)) + .then((res) => { if (epoch === commandEpoch.current) setCommands(commandsOf(res)) }) + .catch(() => { if (epoch === commandEpoch.current) setCommands(noCommands) }) + }, [draftModel, draftAgent]) + // Switches the add-ons with these ids (an array) on or off with one request. const setAddonsOn = useCallback(async (ids, on) => { const before = addonsRef.current @@ -119,11 +132,12 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' if (ids.length === 0) return putAddons(next) setError('') - // A draft has no status to read and keeps the state the list gave it: a - // reset here would never be refilled, and the notice would stay unknown. + // A draft keeps the state it had until the server answers for the new + // choices (refreshDraftCommands): a reset here would leave the notice unknown. if (draftOn) { ids.forEach((id) => choose('addons', id, !on)) setSkillsRev((n) => n + 1) + refreshDraftCommands() return } const epoch = ++commandEpoch.current @@ -148,7 +162,7 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' putAddons(before) setError(e.message || 'error') } - }, [chatId, draftOn, choose]) + }, [chatId, draftOn, choose, refreshDraftCommands]) // keys: server names of the model or connector:, switched with one request. const setMCPOn = useCallback(async (keys, on) => { @@ -182,9 +196,9 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' const next = withOn(before, (b) => b.group, new Set(groups), on) putBuiltins(next) setError('') - // A draft has no status to read and keeps the state the list gave it: a - // reset here would never be refilled, and the notice would stay unknown. - if (draftOn) { groups.forEach((group) => choose('builtins', group, !on)); return } + // A draft keeps the state it had until the server answers for the new + // choices (refreshDraftCommands). + if (draftOn) { groups.forEach((group) => choose('builtins', group, !on)); refreshDraftCommands(); return } const epoch = ++commandEpoch.current setCommands(noCommands) try { @@ -205,7 +219,7 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = '' putBuiltins(before) setError(e.message || 'error') } - }, [chatId, draftOn, choose]) + }, [chatId, draftOn, choose, refreshDraftCommands]) const toggleAddon = useCallback((id, on) => setAddonsOn([id], on), [setAddonsOn]) const toggleMCP = useCallback((key, on) => setMCPOn([key], on), [setMCPOn]) diff --git a/core/http/react-ui/src/utils/api.js b/core/http/react-ui/src/utils/api.js index 1dfd8c90a9..5489f68a44 100644 --- a/core/http/react-ui/src/utils/api.js +++ b/core/http/react-ui/src/utils/api.js @@ -371,10 +371,15 @@ export const sharedFoldersApi = { // Chat skills: folders with a SKILL.md in four layers, switched per chat. const enc2 = encodeURIComponent // The query of the draft lists: the model and the agent of the new chat. -function draftQuery(model, agentId) { +// switches (draft add-ons only) are the draft's choices so far, as the create +// request takes them: addons_off, builtins_off and builtins_on, arrays of names. +function draftQuery(model, agentId, switches) { const q = new URLSearchParams() if (model) q.set('model', model) if (agentId) q.set('agent_id', agentId) + for (const k of ['addons_off', 'builtins_off', 'builtins_on']) { + if (switches?.[k]?.length) q.set(k, switches[k].join(',')) + } const s = q.toString() return s ? `?${s}` : '' } @@ -397,7 +402,9 @@ export const chatSkillsApi = { export const chatAddonsApi = { list: (chat) => fetchJSON(`/api/chats/${enc2(chat)}/addons`), // The add-ons of a chat that does not exist yet, on model with the agent. - draft: (model, agentId) => fetchJSON(`/api/chats/draft/addons${draftQuery(model, agentId)}`), + // switches: the draft's choices ({addons_off, builtins_off, builtins_on}), so the + // commands state answers for them (see draftListSwitches). + draft: (model, agentId, switches) => fetchJSON(`/api/chats/draft/addons${draftQuery(model, agentId, switches)}`), setOff: (chat, off) => fetchJSON(`/api/chats/${enc2(chat)}/addons`, { method: 'PUT', body: JSON.stringify({ off }) }), } diff --git a/core/http/react-ui/src/utils/chatDraftSwitches.js b/core/http/react-ui/src/utils/chatDraftSwitches.js index 099ef2f2a0..a9979d4efb 100644 --- a/core/http/react-ui/src/utils/chatDraftSwitches.js +++ b/core/http/react-ui/src/utils/chatDraftSwitches.js @@ -50,3 +50,18 @@ export function draftSwitches({ addons = [], mcpOff = [], builtins = [], skills for (const k of Object.keys(out)) if (out[k].length === 0) delete out[k] return Object.keys(out).length ? out : null } + +// The switches of the draft add-ons request, from the choices so far. Only a +// group that the list knows is sent, since the server refuses a name it does +// not: builtins is the list as the server gave it (it says which groups are +// opt-in). null when no choice changes anything, so the request is the plain one. +export function draftListSwitches(choices, builtins = []) { + const groups = new Map(builtins.map((b) => [b.group, !!b.opt_in])) + const out = { + addons_off: Object.entries(choices.addons).filter(([, off]) => off).map(([id]) => id).sort(), + builtins_off: Object.entries(choices.builtins).filter(([g, off]) => off && groups.get(g) === false).map(([g]) => g).sort(), + builtins_on: Object.entries(choices.builtins).filter(([g, off]) => !off && groups.get(g) === true).map(([g]) => g).sort(), + } + for (const k of Object.keys(out)) if (out[k].length === 0) delete out[k] + return Object.keys(out).length ? out : null +} diff --git a/core/http/react-ui/tests/unit/chatdraftswitches.test.mjs b/core/http/react-ui/tests/unit/chatdraftswitches.test.mjs index c0a96a2ec3..5f4bfba9dc 100644 --- a/core/http/react-ui/tests/unit/chatdraftswitches.test.mjs +++ b/core/http/react-ui/tests/unit/chatdraftswitches.test.mjs @@ -1,6 +1,6 @@ import test from 'node:test' import assert from 'node:assert/strict' -import { applyChoices, draftSwitches, emptyChoices, withChoice } from '../../src/utils/chatDraftSwitches.js' +import { applyChoices, draftListSwitches, draftSwitches, emptyChoices, withChoice } from '../../src/utils/chatDraftSwitches.js' const ref = (id) => `connector:${id}` @@ -54,3 +54,23 @@ test('draft switches: a choice for a row the list no longer has is ignored', () const c = withChoice(emptyChoices(), 'addons', 'gone', true) assert.deepEqual(applyChoices({ addons: [{ id: 'office', off: false }] }, c).addons, [{ id: 'office', off: false }]) }) + +test('draft list switches: a plain draft asks the plain question', () => { + assert.equal(draftListSwitches(emptyChoices(), [{ group: 'shell', off: false }]), null) +}) + +test('draft list switches: the choices go as the names the server checks', () => { + let choices = withChoice(emptyChoices(), 'builtins', 'shell', true) + choices = withChoice(choices, 'builtins', 'saved_agents', false) + choices = withChoice(choices, 'builtins', 'web', false) + choices = withChoice(choices, 'addons', 'sandbox', true) + choices = withChoice(choices, 'addons', 'office', false) + const builtins = [{ group: 'shell' }, { group: 'web' }, { group: 'saved_agents', opt_in: true }] + assert.deepEqual(draftListSwitches(choices, builtins), { addons_off: ['sandbox'], builtins_off: ['shell'], builtins_on: ['saved_agents'] }) +}) + +test('draft list switches: a group the list does not know is not sent', () => { + const choices = withChoice(emptyChoices(), 'builtins', 'gone', true) + assert.equal(draftListSwitches(choices, [{ group: 'shell' }]), null) + assert.equal(draftListSwitches(choices), null) +}) diff --git a/core/http/routes/studio_draft_test.go b/core/http/routes/studio_draft_test.go index 23dd61b080..b587bcd1c3 100644 --- a/core/http/routes/studio_draft_test.go +++ b/core/http/routes/studio_draft_test.go @@ -44,6 +44,20 @@ var _ = Describe("Chats draft switches", func() { Expect(decodeJSON(addons)).To(HaveKey("commands")) }) + It("reports the commands as switched off when the draft switched the shell group off", func() { + off := s.do("alice", http.MethodGet, "/api/chats/draft/addons?model=test-model&builtins_off=shell", nil) + Expect(off.Code).To(Equal(http.StatusOK), off.Body.String()) + Expect(decodeJSON(off)).To(HaveKeyWithValue("commands", "switched_off")) + + on := s.do("alice", http.MethodGet, "/api/chats/draft/addons?model=test-model", nil) + Expect(decodeJSON(on)).NotTo(HaveKeyWithValue("commands", "switched_off")) + }) + + It("refuses a draft switch that names no group", func() { + bad := s.do("alice", http.MethodGet, "/api/chats/draft/addons?model=test-model&builtins_off=nope", nil) + Expect(bad.Code).To(Equal(http.StatusBadRequest), bad.Body.String()) + }) + It("refuses a model the server does not have and another user's agent", func() { Expect(s.do("alice", http.MethodGet, "/api/chats/draft/tools?model=nope", nil).Code).To(Equal(http.StatusBadRequest)) rec := s.do("alice", http.MethodPost, "/api/chats/agents", map[string]any{"name": "Mine"}) diff --git a/core/services/studio/capabilities_test.go b/core/services/studio/capabilities_test.go index 04344c0205..d05cbaa1a4 100644 --- a/core/services/studio/capabilities_test.go +++ b/core/services/studio/capabilities_test.go @@ -287,7 +287,7 @@ var _ = Describe("the execution status of a chat that does not exist yet", func( It("says the chat is a draft, not that the add-on is not selected", func() { broker := presentationAddons{fakeAddons: &fakeAddons{tools: map[string][]studio.AddonTool{alice: runAddon()}}, bound: true} svc, _, _ := newCapsChat(broker, nil) - _, c, err := svc.DraftAddons(context.Background(), alice, "m", "") + _, c, err := svc.DraftAddons(context.Background(), alice, "m", "", studio.Switches{}) Expect(err).NotTo(HaveOccurred()) Expect(c.ExecutionStatus).NotTo(BeNil()) Expect(c.ExecutionStatus.UnknownReason).To(Equal("draft")) @@ -303,6 +303,39 @@ func (b observingAddons) Unavailable(ctx context.Context, user, addon string, _ return "" } +var _ = Describe("the commands state of a draft with switches", func() { + broker := func() presentationAddons { + return presentationAddons{fakeAddons: &fakeAddons{tools: map[string][]studio.AddonTool{alice: runAddon()}}, bound: true} + } + It("is not switched off without switches", func() { + svc, _, _ := newCapsChat(broker(), nil) + _, c, err := svc.DraftAddons(context.Background(), alice, "m", "", studio.Switches{}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.State).NotTo(Equal(studio.CommandsSwitchedOff)) + }) + It("is switched off when the draft switched the shell group off", func() { + svc, _, _ := newCapsChat(broker(), nil) + _, c, err := svc.DraftAddons(context.Background(), alice, "m", "", studio.Switches{BuiltinsOff: []string{"shell"}}) + Expect(err).NotTo(HaveOccurred()) + Expect(c.State).To(Equal(studio.CommandsSwitchedOff)) + }) + It("lists an add-on the draft switched off as off, and says the commands are off", func() { + svc, _, _ := newCapsChat(broker(), nil) + list, c, err := svc.DraftAddons(context.Background(), alice, "m", "", studio.Switches{AddonsOff: []string{"sandbox"}}) + Expect(err).NotTo(HaveOccurred()) + Expect(list).NotTo(BeEmpty()) + for _, a := range list { + Expect(a.Off).To(BeTrue(), a.ID) + } + Expect(c.State).To(Equal(studio.CommandsOff)) + }) + It("refuses a group that does not exist", func() { + svc, _, _ := newCapsChat(broker(), nil) + _, _, err := svc.DraftAddons(context.Background(), alice, "m", "", studio.Switches{BuiltinsOff: []string{"nope"}}) + Expect(err).To(MatchError(studio.ErrBadBuiltinGroup)) + }) +}) + var _ = Describe("the draft lists of a chat that does not exist yet", func() { // The scopeless guard of this suite fails the spec when a draft path reads // the execution status outside an execution scope. @@ -321,7 +354,7 @@ var _ = Describe("the draft lists of a chat that does not exist yet", func() { It("reads the add-on status inside a scope for the add-on list", func() { broker := observingAddons{presentationAddons{fakeAddons: &fakeAddons{tools: map[string][]studio.AddonTool{alice: runAddon()}}, bound: true}} svc, _, _ := newCapsChat(broker, nil) - _, _, err := svc.DraftAddons(context.Background(), alice, "m", "") + _, _, err := svc.DraftAddons(context.Background(), alice, "m", "", studio.Switches{}) Expect(err).NotTo(HaveOccurred()) }) }) diff --git a/core/services/studio/draftswitches.go b/core/services/studio/draftswitches.go index 8767761019..d31097fcf1 100644 --- a/core/services/studio/draftswitches.go +++ b/core/services/studio/draftswitches.go @@ -27,12 +27,23 @@ func (s *Service) DraftTools(ctx context.Context, user, model, agentID string) ( } // DraftAddons lists the add-ons of a new chat as DraftTools does, and whether -// it could run commands at its first turn. -func (s *Service) DraftAddons(ctx context.Context, user, model, agentID string) ([]AddonInfo, Commands, error) { +// it could run commands at its first turn. sw are the switches the person made +// in the composer so far (only the add-ons, the built-in groups and the opt-in +// groups count): the chat would start with them, so the commands state and the +// add-on switches answer for them, for example "switched off" when the +// sandbox group is off. They are checked as Create checks them. +func (s *Service) DraftAddons(ctx context.Context, user, model, agentID string, sw Switches) ([]AddonInfo, Commands, error) { rec, err := s.draftRecord(ctx, user, model, agentID) if err != nil { return nil, Commands{}, err } + clean, err := s.cleanSwitches(Switches{AddonsOff: sw.AddonsOff, BuiltinsOff: sw.BuiltinsOff, BuiltinsOn: sw.BuiltinsOn}) + if err != nil { + return nil, Commands{}, err + } + rec.AddonsOff = orNil(clean.AddonsOff) + rec.BuiltinsOff = orNil(clean.BuiltinsOff) + rec.BuiltinsOn = mergeGroups(rec.BuiltinsOn, clean.BuiltinsOn) ctx, closeScope := s.executionScope(ctx, principal(user), rec.ID) defer closeScope() list, err := s.addonsFor(ctx, user, rec) diff --git a/docs/content/features/chats.md b/docs/content/features/chats.md index c6b77dca3b..19e3336975 100644 --- a/docs/content/features/chats.md +++ b/docs/content/features/chats.md @@ -175,6 +175,13 @@ The answers have the shapes of `GET /api/chats/{id}/tools` and `GET /api/chats/{id}/addons`. A model you may not use is `403`, a model the server does not have is `400`, and another user's agent is `404`. +The draft add-ons list also takes the switches the composer has so far, as +comma-separated query parameters `addons_off`, `builtins_off` and `builtins_on` +(the lists that `POST /api/chats` takes in `switches`). The `commands` state of +the answer is then the one the new chat would have: with `builtins_off=shell` +it is `switched_off`. A name that is not an add-on switch or a built-in group +is `400`. The Home composer sends them each time you change a switch. + ### List the chats of one origin `GET /api/chats` returns every chat unless you send `origin`. With