fix(studio): read the execution status inside a scope on every turn path

load_skill runs with the context the harness gives it, which has no
execution scope, so the status of the commands read as not selected and
the document skills were dropped. Make the descriptor and skill
resolution open a scope when the context has none, and report a scope-less
status read as a programming error: logged once in production, a failure
of the spec in tests.

Assisted-by: Claude Code:claude-sonnet-5-5
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
Ettore Di Giacinto committed 2026-11-03 11:31:36 +00:00
1 parent 67a503c224
commit a728722c29
5 files changed
+58 -1

No files matched your search

+4
View File
@@ -281,6 +281,8 @@ func (s *Service) CommandRunner(ctx context.Context, user, conv string) (Command
// commandsFor says whether the chat of rec can run commands at its next turn.
func (s *Service) commandsFor(ctx context.Context, p string, rec session.ChatRecord) (Commands, error) {
conv := rec.ID
ctx, closeScope := s.executionScope(ctx, p, conv)
defer closeScope()
a := s.chatActorFor(ctx, rec)
if rec.Kind == session.KindSubAgent && rec.AgentID == "" {
s.mu.Lock()
@@ -538,6 +540,8 @@ func (s *Service) addonCommandDescriptorForActor(ctx context.Context, a workspac
if sc != nil && sc.deleted {
return Commands{State: CommandsNone}, nil
}
ctx, closeScope := s.executionScope(ctx, p, rec.ID)
defer closeScope()
var tools []session.Tool
// Only ad-hoc children inherit a pruned tool set. Saved-agent children
// resolve current grants through their scope, just like their turn tools.
@@ -121,6 +121,8 @@ var _ = Describe("document provider matrix", func() {
}
b.sets = append(b.sets, set)
}
ctx, closeScope := svc.executionScope(ctx, "alice", "")
defer closeScope()
list, _, _, err := svc.chatSkillSet(ctx, "alice", nil, off, sc, command)
Expect(err).NotTo(HaveOccurred())
usable := skills.Usable(list)
@@ -401,6 +403,8 @@ func documentCommands(s *Service, ctx context.Context, p string, rec session.Cha
// chatSkillNotes resolves the skills of a chat as the turn does and returns
// the notes it gets.
func chatSkillNotes(svc *Service, ctx context.Context, off []string, command Commands) ([]skills.Skill, []string, error) {
ctx, closeScope := svc.executionScope(ctx, "alice", "")
defer closeScope()
list, notes, _, err := svc.chatSkillSet(ctx, "alice", nil, off, nil, command)
return list, notes, err
}
+19 -1
View File
@@ -4,6 +4,8 @@ package studio
import (
"cmp"
"context"
"log/slog"
"runtime/debug"
"sync"
"time"
@@ -101,12 +103,28 @@ func ExecutionObservation(ctx context.Context, user, addon string) (model.Value,
return value, detail.Explanation
}
var scopelessOnce sync.Once
// onScopeless is called when a status is read from a context that has no
// execution scope. That is a programming error: the read cannot succeed and
// the caller would report "not selected" for an add-on that is fine. It logs
// once; the tests replace it to fail.
var onScopeless = func() {
scopelessOnce.Do(func() {
slog.Error("studio: the execution status was read outside an execution scope; the status is reported as not selected", "stack", string(debug.Stack()))
})
}
// ObserveExecution is ExecutionObservation with the whole detail. Failure says
// why the value is unknown; Explanation and NetworkPolicyEnforced are set only
// when the provider answered.
func ObserveExecution(ctx context.Context, user, addon string) (model.Value, ExecutionDetail) {
scope, ok := ctx.Value(executionObservationKey{}).(*executionObservation)
if !ok || user != scope.user {
if !ok {
onScopeless()
return model.Unknown(), ExecutionDetail{Failure: UnknownNotSelected}
}
if user != scope.user {
return model.Unknown(), ExecutionDetail{Failure: UnknownNotSelected}
}
scope.mu.Lock()
+4
View File
@@ -134,6 +134,10 @@ func (s *Service) loadableSkillsForActor(ctx context.Context, a workspace.Actor,
}
func (s *Service) loadableSkillsAndNotesForActor(ctx context.Context, a workspace.Actor, final *bool) ([]skills.Skill, []string, error) {
// The harness calls a tool with a context of its own, without the turn's
// scope. The status of the commands is read here, so the scope is too.
ctx, closeScope := s.executionScope(ctx, a.Principal, a.ChatID)
defer closeScope()
rec, err := s.d.Store.GetChat(ctx, a.Principal, a.ChatID)
if err != nil {
return nil, nil, err
@@ -0,0 +1,27 @@
package studio
import (
"runtime/debug"
"sync/atomic"
. "github.com/onsi/ginkgo/v2"
)
// A status read without an execution scope is a programming error. Every
// spec fails when one happens, with the stack of the first.
var scopeless atomic.Pointer[string]
var _ = BeforeSuite(func() {
onScopeless = func() {
stack := string(debug.Stack())
scopeless.CompareAndSwap(nil, &stack)
}
})
var _ = BeforeEach(func() { scopeless.Store(nil) })
var _ = AfterEach(func() {
if stack := scopeless.Load(); stack != nil {
Fail("the execution status was read outside an execution scope:\n" + *stack)
}
})