fix(chats): keep the draft commands state and let Hide work

The draft tools and add-on lists, and the tools list of a stored chat,
read the execution status without an execution scope. The host logged
an error and reported the status as not selected. They now run in a
scope; a draft has no conversation, so its status says "draft".

In the Home composer, switching a built-in tool group on or off reset
the commands state to the loading value and never read it again. The
composer then showed the unknown warning for good. The Hide button also
did nothing for that state, because it needed a binding identity that
the loading value does not have. A draft now keeps the state the list
gave it, and Hide works for any notice that is shown.

Assisted-by: Claude:claude-sonnet-5-5 go ginkgo node
This commit is contained in:
Ettore Di Giacinto committed 2026-11-03 17:23:29 +00:00
1 parent c007ccf994
commit f97a96cc83
6 files changed
+90 -4

No files matched your search

+3 -1
View File
@@ -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)
+6 -3
View File
@@ -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 }
}
@@ -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`)
}
})
+32
View File
@@ -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")
+6
View File
@@ -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
+2
View File
@@ -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