diff --git a/core/http/react-ui/src/hooks/useChatSwitches.js b/core/http/react-ui/src/hooks/useChatSwitches.js index 35dd2fb5d7..f61763008c 100644 --- a/core/http/react-ui/src/hooks/useChatSwitches.js +++ b/core/http/react-ui/src/hooks/useChatSwitches.js @@ -182,9 +182,11 @@ 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 } const epoch = ++commandEpoch.current setCommands(noCommands) - if (draftOn) { groups.forEach((group) => choose('builtins', group, !on)); return } try { const off = next.filter((b) => b.off && !b.opt_in).map((b) => b.group).sort() const optIn = next.filter((b) => b.opt_in) diff --git a/core/http/react-ui/src/hooks/useCommandsNotice.js b/core/http/react-ui/src/hooks/useCommandsNotice.js index fba2f5075f..956ae0722a 100644 --- a/core/http/react-ui/src/hooks/useCommandsNotice.js +++ b/core/http/react-ui/src/hooks/useCommandsNotice.js @@ -9,7 +9,10 @@ export function useCommandsNotice(commands, isAdmin, context) { const [hiddenIdentity, setHiddenIdentity] = useState('') useEffect(() => { setHiddenIdentity('') }, [identity]) const hide = useCallback(() => { - if (info && commands.identity) setHiddenIdentity(identity) - }, [info, commands.identity, identity]) - return { info, hidden: !!commands.identity && !!info && hiddenIdentity === identity, hide } + if (info) setHiddenIdentity(identity) + }, [info, identity]) + // A state with no binding yet (loading, a failed read) can be hidden too: the + // identity then holds the state and the reason, and a new reading brings the + // notice back. + return { info, hidden: !!info && hiddenIdentity === identity, hide } } diff --git a/core/http/react-ui/tests/unit/chatcommands.test.mjs b/core/http/react-ui/tests/unit/chatcommands.test.mjs index a8a412adca..a1637445a6 100644 --- a/core/http/react-ui/tests/unit/chatcommands.test.mjs +++ b/core/http/react-ui/tests/unit/chatcommands.test.mjs @@ -327,3 +327,44 @@ test('a draft chat has no execution status to warn about', async () => { assert.equal(commandsNotice(available.state), null) assert.equal(commandsState({ commands: 'none', execution_status: draft }).state, 'none') }) + +test('the hide button hides a notice whose state has no binding identity yet', async () => { + const { readFileSync } = await import('node:fs') + const source = readFileSync(new URL('../../src/hooks/useCommandsNotice.js', import.meta.url), 'utf8').replace(/^import .*\n/gm, '').replace('export function', 'function') + let hidden = '' + const hook = new Function('useState', 'useEffect', 'useCallback', 'commandsNotice', `${source}; return useCommandsNotice`)( + () => [hidden, v => { hidden = v }], () => {}, fn => fn, commandsNotice) + const read = () => hook({ state: 'unknown', reason: '', identity: '' }, false, ['user', null, 'm', '']) + assert.ok(read().info) + assert.equal(read().hidden, false) + read().hide() + assert.equal(read().hidden, true, 'a click on Hide must hide the notice even with no identity') +}) + +test('a draft chat keeps its commands state when a tool is switched on and off', async () => { + const { hookHarness } = await import('./chatcommands-lifecycle.mjs') + const names = ['chatMCP', 'chatSwitchRows', 'chatSkills', 'chatCommands', 'chatDraftSwitches'] + const imports = Object.assign({}, ...await Promise.all(names.map(name => import(`../../src/utils/${name}.js`)))) + const draftReply = { commands: 'available', commands_provider: 'sandbox', addons: [{ id: 'sandbox', tools: 1 }], execution_status: { readiness: 'unknown', isolation: 'unknown', unknown_reason: 'draft' } } + let draftCalls = 0 + const h = hookHarness('../../src/hooks/useChatSwitches.js', 'useChatSwitches', { + ...imports, + chatAddonsApi: { draft: async () => { draftCalls++; return draftReply }, list: async () => ({}), setOff: async () => ({}) }, + chatToolsApi: { draft: async () => ({ builtins: [{ group: 'shell', off: false }] }), list: async () => ({}), setBuiltinsOff: async () => ({}) }, + chatSkillsApi: { list: async () => ({ skills: [] }) }, + }) + const render = () => h.render(null, true, { model: 'm', agentId: '' }, 'A:1') + const settle = () => new Promise(resolve => setImmediate(resolve)) + render(); h.flush(); await settle() + assert.equal(render().commands.state, 'draft') + assert.equal(commandsNotice(render().commands.state), null, 'no banner for a draft that is ready at its first message') + for (const step of ['builtin off', 'builtin on', 'addon off', 'addon on']) { + const s = render() + if (step.startsWith('builtin')) await s.toggleBuiltin('shell', step.endsWith('on')) + else await s.toggleAddon('sandbox', step.endsWith('on')) + h.flush(); await settle() + const now = render() + assert.equal(commandsNotice(now.commands.state), null, `${step}: no banner`) + assert.ok(now.commands.identity, `${step}: the state is the list's, not a reset`) + } +}) diff --git a/core/services/studio/capabilities_test.go b/core/services/studio/capabilities_test.go index d8a277d177..04344c0205 100644 --- a/core/services/studio/capabilities_test.go +++ b/core/services/studio/capabilities_test.go @@ -294,6 +294,38 @@ var _ = Describe("the execution status of a chat that does not exist yet", func( }) }) +// observingAddons reads the execution status in Unavailable, as the real +// gateway does, so a call outside an execution scope trips the guard. +type observingAddons struct{ presentationAddons } + +func (b observingAddons) Unavailable(ctx context.Context, user, addon string, _ bool) string { + studio.ExecutionObservation(ctx, user, addon) + return "" +} + +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. + It("reads the add-on status inside a scope for the tools list", func() { + broker := observingAddons{presentationAddons{fakeAddons: &fakeAddons{tools: map[string][]studio.AddonTool{alice: runAddon()}}, bound: true}} + svc, _, _ := newCapsChat(broker, nil) + _, err := svc.DraftTools(context.Background(), alice, "m", "") + Expect(err).NotTo(HaveOccurred()) + }) + It("reads the add-on status inside a scope for the tools of a stored chat", func() { + broker := observingAddons{presentationAddons{fakeAddons: &fakeAddons{tools: map[string][]studio.AddonTool{alice: runAddon()}}, bound: true}} + svc, _, _ := newCapsChat(broker, nil) + _, err := svc.Tools(context.Background(), alice, create(svc, alice)) + Expect(err).NotTo(HaveOccurred()) + }) + 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", "") + Expect(err).NotTo(HaveOccurred()) + }) +}) + var _ = Describe("a second provider of commands", func() { runbox := func() []studio.AddonTool { bash := addonTool("runbox", "bash") diff --git a/core/services/studio/draftswitches.go b/core/services/studio/draftswitches.go index 6e85024fb6..8767761019 100644 --- a/core/services/studio/draftswitches.go +++ b/core/services/studio/draftswitches.go @@ -19,6 +19,10 @@ func (s *Service) DraftTools(ctx context.Context, user, model, agentID string) ( if err != nil { return ToolsPanel{}, err } + // The add-on gate reads the execution status. A draft has no chat, so the + // scope has no conversation and the status says "draft". + ctx, closeScope := s.executionScope(ctx, principal(user), rec.ID) + defer closeScope() return s.toolsFor(ctx, user, rec) } @@ -29,6 +33,8 @@ func (s *Service) DraftAddons(ctx context.Context, user, model, agentID string) if err != nil { return nil, Commands{}, err } + ctx, closeScope := s.executionScope(ctx, principal(user), rec.ID) + defer closeScope() list, err := s.addonsFor(ctx, user, rec) if err != nil { return nil, Commands{}, err diff --git a/core/services/studio/mcpconnectors.go b/core/services/studio/mcpconnectors.go index 14f899d578..b2b31800a0 100644 --- a/core/services/studio/mcpconnectors.go +++ b/core/services/studio/mcpconnectors.go @@ -124,6 +124,8 @@ func (s *Service) Tools(ctx context.Context, user, conv string) (ToolsPanel, err // toolsFor lists what the chat of rec can use, with its switches. func (s *Service) toolsFor(ctx context.Context, user string, rec session.ChatRecord) (ToolsPanel, error) { conv := rec.ID + ctx, closeScope := s.executionScope(ctx, principal(user), conv) + defer closeScope() sc := s.scopeOf(ctx, rec) var err error var shadowing []session.Tool