fix(studio): keep the commands state and skills of a draft chat

A chat that does not exist yet has no execution status to read, so its
isolation is unknown by construction. Three places treated that as a
failure:

- Switching an add-on in a draft reset the commands state and never
  refilled it, so the "isolation is unknown" notice stayed. The draft
  now keeps the state its list gave it.
- The skills list of a draft hid the document skills of the selected
  commands provider, because they need isolated commands. A draft list
  now offers them; the turn of the real chat still gates them on the
  status it reads. The draft add-on switch also reads the skills again.
- The shell row of the Tools menu printed the generic unknown text for
  the draft state. It now says nothing about isolation there.

In a real chat, switching an add-on or a tool group now reads the
chat's network view again, so the network controls appear once the
provider serves commands.

Assisted-by: Claude:claude-sonnet-5-5 go gofmt playwright
This commit is contained in:
Ettore Di Giacinto committed 2026-11-03 14:00:45 +00:00
1 parent ab78cd0301
commit 76efd6b26b
8 files changed
+92 -3

No files matched your search

@@ -113,6 +113,25 @@ test.describe('Chat that cannot run commands', () => {
await expect(page.getByTestId('chat-commands-chip')).toHaveCount(0)
})
test('switching the Sandbox on reads the network controls of the chat again', async ({ page }) => {
let egressReads = 0
await stubChat(page, (off) => {
const isOff = off === null ? true : off.includes('sandbox')
return { addons: [{ id: 'sandbox', tools: 7, off: isOff }], commands: isOff ? 'off' : 'available', commands_provider: 'sandbox', commands_isolation: isOff ? '' : 'isolated',
execution_status: isOff ? undefined : { readiness: 'available', isolation: 'isolated' } }
})
await page.route('**/api/chats/c1/egress', route => {
egressReads++
const on = egressReads > 1
return route.fulfill({ json: { preset: 'off', ceiling: 'all', effective: 'off', applies: on, applies_reason: on ? '' : 'none', addon: on ? 'sandbox' : '', registries: [], rules: [], cards: [] } })
})
await page.goto('/app/chats/c1')
await expect.poll(() => egressReads).toBe(1)
await page.getByTestId('chat-tools-button').click()
await page.getByRole('switch', { name: 'sandbox' }).check()
await expect.poll(() => egressReads).toBeGreaterThan(1)
})
test('an unavailable selected provider gives its typed reason without an install link', async ({ page }) => {
await stubChat(page, () => ({ addons: [{ id: 'sandbox', tools: 1, off: false }], commands: 'unavailable', commands_provider: 'sandbox', execution_status: { readiness: 'unavailable', explanation: 'no sandbox backend works on this host, so commands cannot run' }, commands_reason: 'no sandbox backend works on this host, so commands cannot run' }))
await page.goto('/app/chats/c1')
@@ -112,6 +112,28 @@ test.describe('Home composer', () => {
expect(seen.created[0].switches).toEqual({ addons_off: ['office'], skills_off: ['builtin:pdf'] })
})
test('toggling the Sandbox add-on in a draft keeps the commands notice away and reads the skills again', async ({ page }) => {
const seen = await server(page)
const skillReads = []
await page.route(/\/api\/chats\/draft\/addons(\?.*)?$/, route => route.fulfill({ json: {
addons: [{ id: 'sandbox', tools: 7, off: false }], commands: 'available', commands_provider: 'sandbox',
execution_status: { readiness: 'unknown', isolation: 'unknown', unknown_reason: 'draft' },
} }))
await page.route(/\/api\/chats\/skills(\?.*)?$/, route => { skillReads.push(route.request().url()); return route.fulfill({ json: { skills: [
{ id: 'addon:docx', name: 'docx', description: 'Word files', layer: 'addon', enabled: true },
] } }) })
await ready(page)
await page.getByTestId('chat-tools-button').click()
await expect(page.getByTestId('chat-commands-notice')).toHaveCount(0)
const reads = skillReads.length
await page.getByRole('switch', { name: 'sandbox' }).uncheck()
await page.getByRole('switch', { name: 'sandbox' }).check()
await expect(page.getByTestId('chat-commands-notice')).toHaveCount(0)
await expect(page.getByTestId('chat-commands-chip')).toHaveCount(0)
await expect.poll(() => skillReads.length).toBeGreaterThan(reads)
expect(seen.created).toHaveLength(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: {
@@ -120,6 +120,8 @@ export default function ChatComposer({
const selected = switches.commands
const chip = by === 'builtin' ? tc('tools.shell.machine') : by ? tc('tools.servedBy', { provider: by }) : undefined
// Group discovery is not evidence of the selected provider's isolation.
// A chat that is not created yet has no status to read: say nothing of isolation.
if (selected.state === 'draft' && by) return { chip }
if (!by || selected.provider !== by) return { chip, note: tc('tools.commands.unknown') }
if (['host', 'host-off', 'host-unavailable', 'provider-host'].includes(selected.state)) return { chip, note: tc('tools.commands.providerHost'), warn: true }
if (selected.state === 'available') return { chip, note: tc('tools.shell.isolated') }
+13 -2
View File
@@ -48,6 +48,9 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = ''
// A change of the add-ons or of the skills can bring a built-in skill back,
// so the list is read again after one.
const [skillsRev, setSkillsRev] = useState(0)
// Counts the changes after which what the chat can run changed: the network
// controls of the chat (its egress view) are read again.
const [commandsRev, setCommandsRev] = useState(0)
const [error, setError] = useState('')
const [skillsError, setSkillsError] = useState('')
const [skillsBusy, setSkillsBusy] = useState(0)
@@ -116,9 +119,15 @@ 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.
if (draftOn) {
ids.forEach((id) => choose('addons', id, !on))
setSkillsRev((n) => n + 1)
return
}
const epoch = ++commandEpoch.current
setCommands(noCommands)
if (draftOn) { ids.forEach((id) => choose('addons', id, !on)); return }
try {
const res = await chatAddonsApi.setOff(chatId, next.filter((a) => a.off).map((a) => a.id).sort())
if (epoch !== commandEpoch.current) return
@@ -127,6 +136,7 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = ''
setCommands(commandsOf(res))
}
setSkillsRev((n) => n + 1)
setCommandsRev((n) => n + 1)
// An add-on switched off or on changes who serves the tools it
// replaces, and the notes about them.
chatToolsApi.list(chatId)
@@ -183,6 +193,7 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = ''
const status = await chatAddonsApi.list(chatId)
if (epoch !== commandEpoch.current) return
setCommands(commandsOf(status))
setCommandsRev((n) => n + 1)
putBuiltins(res.builtins || [])
putMcp(mcpOf(res))
setFallbacks(res.fallbacks || [])
@@ -242,7 +253,7 @@ export function useChatSwitches(chatId, filesOn, draft = null, actorSession = ''
const resetDraft = useCallback(() => { choicesRef.current = emptyChoices() }, [])
return {
draftSwitches, resetDraft, draft: draftOn,
draftSwitches, resetDraft, draft: draftOn, commandsRev,
addons, builtins, fallbacks, commands: commands.context === context ? commands : noCommands, servers: mcp.servers, connectors: mcp.connectors, skills, replacedSkills: replaced, error, skillsError, skillsBusy: skillsBusy > 0,
counts: toolsCounts({ builtins, addons, servers: mcp.servers, connectors: mcp.connectors }),
toggleAddon, toggleBuiltin, toggleMCP, toggleSkill, setSkillsOn, setAddonsOn, setBuiltinsOn, setMCPOn,
+5
View File
@@ -214,6 +214,11 @@ function ChatPage() {
const model = activeChat ? activeChat.model : draftModel
// The network of the chat's isolated commands, read again on its events.
const egress = useChatEgress(activeId, chat.ticks.egress)
// Switching an add-on or a tool group on or off changes who runs the commands
// and whether the network controls apply, so the network view is read again.
const { refresh: refreshEgress } = egress
const commandsRev = switches.commandsRev
useEffect(() => { if (commandsRev) refreshEgress() }, [commandsRev, refreshEgress])
// The cards of the chat by their tool line; the ones that wait are drawn
// under the live fold until the call ends.
const egressCards = useMemo(() => ({
+5 -1
View File
@@ -122,7 +122,11 @@ func (s *Service) addonSkillsFor(ctx context.Context, p string, addonsOff []stri
if gate != nil && gate.Unavailable(ctx, p, set.AddonID, s.d.Files.Available()) != "" {
continue
}
if len(set.Isolated) > 0 && !isolatedProvider(command, set.AddonID) {
// A chat that does not exist yet has no status to read, so its list
// offers what the selected provider ships; the turn of the real chat
// still gates these skills on the status it reads then.
draftProvider := command.Draft && command.State == CommandsAvailable && command.Provider == set.AddonID
if len(set.Isolated) > 0 && !isolatedProvider(command, set.AddonID) && !draftProvider {
needs := func(n string) bool { return slices.Contains(set.Isolated, n) }
if command.State == CommandsAvailable && command.Provider == set.AddonID {
out.documentsWhy = s.documentsWhy(ctx, p, command)
+5
View File
@@ -108,6 +108,9 @@ type Commands struct {
// FallbackReason "off" (switched off for the chat or not named by its
// agent) or why it cannot run.
Fallback, FallbackReason string
// Draft is set when the chat does not exist yet. No execution status can be
// read for it, so Isolation is unknown by construction, not by a failure.
Draft bool
}
// withHostBash is the state of a chat whose tools may hold the built-in bash:
@@ -300,6 +303,7 @@ func (s *Service) commandsFor(ctx context.Context, p string, rec session.ChatRec
}
}
c := s.commandsOf(ctx, p, tools, rec.AddonsOff, sc)
c.Draft = rec.ID == ""
if _, ok := s.d.AddonTools.(ExecutionObserver); ok {
c.ExecutionStatus = executionStatusView(ctx, p, c.Addon)
}
@@ -557,6 +561,7 @@ func (s *Service) addonCommandDescriptorForActor(ctx context.Context, a workspac
}
}
c := s.commandsOf(ctx, p, tools, rec.AddonsOff, sc)
c.Draft = rec.ID == ""
if c.State == CommandsAvailable {
c.Provider = c.Addon
}
@@ -76,6 +76,27 @@ var _ = Describe("document command eligibility", func() {
Expect(err).NotTo(HaveOccurred())
Expect(skills.Names(skills.Usable(list))).To(ContainElement("docx"))
})
It("offers the selected provider's document skills in a draft, and still gates a real chat on its status", func() {
ctx := context.Background()
b := &documentBroker{skillBroker: &skillBroker{}, isolation: "", defs: []AddonTool{{AddonID: "runbox", Name: "addon__runbox__exec", Shadows: "bash",
Execution: &contract.ExecutionDescriptor{Class: contract.ExecutionClassCommands, Title: "Runbox"}}}}
svc, f := newSkillService(b.skillBroker)
svc.d.AddonTools = b
Expect(f.SyncAddonSkills(ctx, "runbox", fstest.MapFS{"docx/SKILL.md": {Data: md("docx", "Runbox document.")}})).To(Succeed())
b.set(AddonSkillSet{AddonID: "runbox", Skills: []string{"docx"}, Shadows: []string{"docx"}, Isolated: []string{"docx"}, Running: true})
draft := documentCommands(svc, ctx, "alice", session.ChatRecord{}, nil)
Expect(draft.Draft).To(BeTrue())
list, _, err := svc.Skills(ctx, "alice", "")
Expect(err).NotTo(HaveOccurred())
Expect(skills.Names(skills.Usable(list))).To(ContainElement("docx"), "a draft has no status to read, so it must not hide the skill")
rec, err := svc.Create(ctx, "alice", CreateInput{Model: "m"})
Expect(err).NotTo(HaveOccurred())
command := documentCommands(svc, ctx, "alice", rec, nil)
Expect(command.Draft).To(BeFalse())
list, _, err = chatSkillNotes(svc, ctx, nil, command)
Expect(err).NotTo(HaveOccurred())
Expect(skills.Names(skills.Usable(list))).NotTo(ContainElement("docx"), "a real chat with unknown isolation stays closed")
})
It("does not switch a missing pinned document provider to Office", func() {
a := addonSkills{pins: map[string]string{"docx": "sandbox"}, usable: []AddonSkillSet{{AddonID: "office", Shadows: []string{"docx"}}}}
id, chosen := a.provider("docx")