From a728722c29fd933d4e69879340f4fa3bd5028e7e Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Tue, 3 Nov 2026 11:21:40 +0000 Subject: [PATCH] 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 --- core/services/studio/capabilities.go | 4 +++ .../documenteligibility_internal_test.go | 4 +++ core/services/studio/executionobservation.go | 20 +++++++++++++- core/services/studio/loadskill.go | 4 +++ .../studio/scopeguard_internal_test.go | 27 +++++++++++++++++++ 5 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 core/services/studio/scopeguard_internal_test.go diff --git a/core/services/studio/capabilities.go b/core/services/studio/capabilities.go index 51d1164629..8890db9309 100644 --- a/core/services/studio/capabilities.go +++ b/core/services/studio/capabilities.go @@ -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. diff --git a/core/services/studio/documenteligibility_internal_test.go b/core/services/studio/documenteligibility_internal_test.go index 4f61cdaac9..b90ea48b68 100644 --- a/core/services/studio/documenteligibility_internal_test.go +++ b/core/services/studio/documenteligibility_internal_test.go @@ -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 } diff --git a/core/services/studio/executionobservation.go b/core/services/studio/executionobservation.go index a022d48738..38670f9f4d 100644 --- a/core/services/studio/executionobservation.go +++ b/core/services/studio/executionobservation.go @@ -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() diff --git a/core/services/studio/loadskill.go b/core/services/studio/loadskill.go index 44830a489d..9a2bdfb605 100644 --- a/core/services/studio/loadskill.go +++ b/core/services/studio/loadskill.go @@ -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 diff --git a/core/services/studio/scopeguard_internal_test.go b/core/services/studio/scopeguard_internal_test.go new file mode 100644 index 0000000000..f713e46d88 --- /dev/null +++ b/core/services/studio/scopeguard_internal_test.go @@ -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) + } +})