From bfe3c9bb45edf189416198ec9f26ebbb6173ef94 Mon Sep 17 00:00:00 2001 From: Chenghao Mou Date: Tue, 13 Oct 2026 15:04:01 +0100 Subject: [PATCH 1/3] agent simulate: include nodes from other files A scenarios file had to hold every node its scenarios use, so shared nodes such as a menu or a person were copied into each file. A top-level `include` list now takes the nodes of other files. Paths are relative to the file that names them, includes nest, and each file loads once by its real path. An included file holds only `nodes` and `include`. Includes resolve here, on the client: the server receives one flat `nodes` map, so the protocol and the server do not change. Addresses AGT-3707 --- cmd/lk/simulate.go | 16 ++-- cmd/lk/simulate_include.go | 145 +++++++++++++++++++++++++++++++ cmd/lk/simulate_include_test.go | 146 ++++++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 5 deletions(-) create mode 100644 cmd/lk/simulate_include.go create mode 100644 cmd/lk/simulate_include_test.go diff --git a/cmd/lk/simulate.go b/cmd/lk/simulate.go index 4f131e70..f96f1760 100644 --- a/cmd/lk/simulate.go +++ b/cmd/lk/simulate.go @@ -389,10 +389,12 @@ func scenarioGroupToYAML(group *livekit.ScenarioGroup) ([]byte, error) { // scenariosFile mirrors a scenarios.yaml; `userdata` is a nested mapping here // and JSON-encoded into the proto's string field. Each `nodes` entry is a -// SimulatorNode in protojson form. +// SimulatorNode in protojson form. `include` names files whose nodes join this +// file's (see simulate_include.go). type scenariosFile struct { ID string `yaml:"id"` Name string `yaml:"name"` + Include []string `yaml:"include,omitempty"` Nodes map[string]any `yaml:"nodes,omitempty"` Scenarios []yamlScenario `yaml:"scenarios"` } @@ -469,15 +471,19 @@ func loadScenarioGroup(path string) (*livekit.ScenarioGroup, error) { return nil, fmt.Errorf("scenarios file has no id") } group := &livekit.ScenarioGroup{Id: f.ID, Name: f.Name} - names := make([]string, 0, len(f.Nodes)) - for name := range f.Nodes { + nodes, err := resolveNodes(path, f.Nodes, f.Include) + if err != nil { + return nil, err + } + names := make([]string, 0, len(nodes)) + for name := range nodes { names = append(names, name) } sort.Strings(names) for _, name := range names { node := &livekit.SimulatorNode{} - if err := yamlValueToProto(f.Nodes[name], node); err != nil { - return nil, fmt.Errorf("node %q: %w", name, err) + if err := yamlValueToProto(nodes[name].value, node); err != nil { + return nil, fmt.Errorf("node %q (%s): %w", name, filepath.Base(nodes[name].file), err) } if group.Nodes == nil { group.Nodes = map[string]*livekit.SimulatorNode{} diff --git a/cmd/lk/simulate_include.go b/cmd/lk/simulate_include.go new file mode 100644 index 00000000..536c5f35 --- /dev/null +++ b/cmd/lk/simulate_include.go @@ -0,0 +1,145 @@ +// Copyright 2026 LiveKit, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + "gopkg.in/yaml.v3" +) + +// includedFile is a scenarios.yaml `include` target: it only contributes +// nodes, so running one file never runs scenarios it did not list itself. +type includedFile struct { + Include []string `yaml:"include"` + Nodes map[string]any `yaml:"nodes"` +} + +// sourcedNode is a node in YAML form and the file that defines it. +type sourcedNode struct { + value any + file string +} + +// nodeResolver merges the nodes of a scenarios file and everything it +// includes. Paths resolve relative to the file that names them, and each file +// loads once by its real path, so a file reached through two includes is not a +// duplicate. +type nodeResolver struct { + root string // directory errors name files relative to + loaded map[string]bool + chain []string // real paths from the root file to the one loading now + nodes map[string]sourcedNode +} + +func resolveNodes(path string, nodes map[string]any, include []string) (map[string]sourcedNode, error) { + real, err := filepath.EvalSymlinks(path) + if err != nil { + return nil, err + } + r := &nodeResolver{ + root: filepath.Dir(real), + loaded: map[string]bool{real: true}, + chain: []string{real}, + nodes: map[string]sourcedNode{}, + } + if err := r.add(real, nodes); err != nil { + return nil, err + } + if err := r.includeAll(real, include); err != nil { + return nil, err + } + return r.nodes, nil +} + +func (r *nodeResolver) includeAll(from string, include []string) error { + for _, rel := range include { + path := rel + if !filepath.IsAbs(path) { + path = filepath.Join(filepath.Dir(from), rel) + } + real, err := filepath.EvalSymlinks(path) + if err != nil { + return fmt.Errorf("%s: include %q: %w", r.name(from), rel, err) + } + if slices.Contains(r.chain, real) { + return fmt.Errorf("include cycle: %s", r.chainNames(append(r.chain, real))) + } + if r.loaded[real] { + continue + } + r.loaded[real] = true + if err := r.load(real); err != nil { + return err + } + } + return nil +} + +func (r *nodeResolver) load(real string) error { + data, err := os.ReadFile(real) + if err != nil { + return fmt.Errorf("%s: %w", r.name(real), err) + } + var keys map[string]any + if err := yaml.Unmarshal(data, &keys); err != nil { + return fmt.Errorf("%s: %w", r.name(real), err) + } + for key := range keys { + if key != "nodes" && key != "include" { + return fmt.Errorf("%s: an included file holds only `nodes` and `include`, not %q", r.name(real), key) + } + } + var f includedFile + if err := yaml.Unmarshal(data, &f); err != nil { + return fmt.Errorf("%s: %w", r.name(real), err) + } + if err := r.add(real, f.Nodes); err != nil { + return err + } + r.chain = append(r.chain, real) + defer func() { r.chain = r.chain[:len(r.chain)-1] }() + return r.includeAll(real, f.Include) +} + +func (r *nodeResolver) add(file string, nodes map[string]any) error { + for name, value := range nodes { + if prev, ok := r.nodes[name]; ok { + return fmt.Errorf("node %q is defined in both %s and %s", name, r.name(prev.file), r.name(file)) + } + r.nodes[name] = sourcedNode{value: value, file: file} + } + return nil +} + +// name is how errors refer to a file: relative to the root file's directory. +func (r *nodeResolver) name(real string) string { + if rel, err := filepath.Rel(r.root, real); err == nil { + return rel + } + return real +} + +func (r *nodeResolver) chainNames(chain []string) string { + names := make([]string, len(chain)) + for i, p := range chain { + names[i] = r.name(p) + } + return strings.Join(names, " -> ") +} diff --git a/cmd/lk/simulate_include_test.go b/cmd/lk/simulate_include_test.go new file mode 100644 index 00000000..99779b12 --- /dev/null +++ b/cmd/lk/simulate_include_test.go @@ -0,0 +1,146 @@ +// Copyright 2026 LiveKit, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "os" + "path/filepath" + "slices" + "testing" + + "github.com/stretchr/testify/require" +) + +// writeFiles writes each name -> content under one temp dir and returns it. +func writeFiles(t *testing.T, files map[string]string) string { + t.Helper() + dir := t.TempDir() + for name, content := range files { + path := filepath.Join(dir, name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) + } + return dir +} + +const includingScenarios = ` +id: SCNG_dental +include: [nodes/menus.yaml, nodes/people.yaml] +nodes: + greeting: { say: "Hi, you've reached Sam.", then: livekit/voicemail } +scenarios: + - id: SCN_a + label: menu-to-sam + start: menu + agent_expectations: Presses 1, then confirms with Sam. +` + +func TestIncludedNodesJoinTheGroup(t *testing.T) { + dir := writeFiles(t, map[string]string{ + "dental.yaml": includingScenarios, + // nested, with a path relative to the including file + "nodes/menus.yaml": ` +include: [common.yaml] +nodes: + menu: + say: "For Sam, press 1." + routes: [{ match: "1", to: sam }] + timeout: { after: 8s, to: goodbye } +`, + // reached twice: from dental.yaml through people.yaml and menus.yaml + "nodes/people.yaml": ` +include: [common.yaml] +nodes: + sam: { instructions: You are Sam. } +`, + "nodes/common.yaml": ` +nodes: + goodbye: { say: Goodbye., then: livekit/hangup } +`, + }) + + group, err := loadScenarioGroup(filepath.Join(dir, "dental.yaml")) + require.NoError(t, err) + + names := make([]string, 0, len(group.Nodes)) + for name := range group.Nodes { + names = append(names, name) + } + slices.Sort(names) + require.Equal(t, []string{"goodbye", "greeting", "menu", "sam"}, names) + require.Equal(t, "sam", group.Nodes["menu"].Routes[0].To) + require.Equal(t, "menu", group.Scenarios[0].Start) +} + +func TestIncludeErrors(t *testing.T) { + cases := []struct { + name string + files map[string]string + want string + }{ + { + name: "a cycle prints the include chain", + files: map[string]string{ + "dental.yaml": includingScenarios, + "nodes/menus.yaml": "include: [common.yaml]\nnodes: { menu: { say: hi, then: livekit/hangup } }\n", + "nodes/common.yaml": "include: [menus.yaml]\n", + "nodes/people.yaml": "nodes: { sam: { instructions: x } }\n", + }, + want: "include cycle: dental.yaml -> nodes/menus.yaml -> nodes/common.yaml -> nodes/menus.yaml", + }, + { + name: "a node defined in two files names both", + files: map[string]string{ + "dental.yaml": includingScenarios, + "nodes/menus.yaml": "nodes: { menu: { say: hi, then: livekit/hangup } }\n", + "nodes/people.yaml": "nodes: { menu: { say: hello, then: livekit/hangup } }\n", + }, + want: `node "menu" is defined in both nodes/menus.yaml and nodes/people.yaml`, + }, + { + name: "an included file cannot set scenarios", + files: map[string]string{ + "dental.yaml": includingScenarios, + "nodes/menus.yaml": "nodes: { menu: { say: hi, then: livekit/hangup } }\nscenarios: []\n", + "nodes/people.yaml": "nodes: { sam: { instructions: x } }\n", + }, + want: "nodes/menus.yaml: an included file holds only `nodes` and `include`, not \"scenarios\"", + }, + { + name: "a missing include path", + files: map[string]string{ + "dental.yaml": includingScenarios, + "nodes/menus.yaml": "nodes: { menu: { say: hi, then: livekit/hangup } }\n", + }, + want: `dental.yaml: include "nodes/people.yaml"`, + }, + { + name: "a bad included node names its file", + files: map[string]string{ + "dental.yaml": includingScenarios, + "nodes/menus.yaml": "nodes: { menu: { sya: hi } }\n", + "nodes/people.yaml": "nodes: { sam: { instructions: x } }\n", + }, + want: `node "menu" (menus.yaml)`, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + dir := writeFiles(t, c.files) + _, err := loadScenarioGroup(filepath.Join(dir, "dental.yaml")) + require.ErrorContains(t, err, c.want) + }) + } +} From 565a3abecc202fc01c6f8d70707dc88a61b752cd Mon Sep 17 00:00:00 2001 From: Chenghao Mou Date: Wed, 21 Oct 2026 11:54:12 +0100 Subject: [PATCH 2/3] agent simulate: print include paths with forward slashes on every OS --- cmd/lk/simulate_include.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/lk/simulate_include.go b/cmd/lk/simulate_include.go index 536c5f35..9f6f6d68 100644 --- a/cmd/lk/simulate_include.go +++ b/cmd/lk/simulate_include.go @@ -128,10 +128,11 @@ func (r *nodeResolver) add(file string, nodes map[string]any) error { return nil } -// name is how errors refer to a file: relative to the root file's directory. +// name is how errors refer to a file: relative to the root file's directory, +// with forward slashes as in `include`. func (r *nodeResolver) name(real string) string { if rel, err := filepath.Rel(r.root, real); err == nil { - return rel + return filepath.ToSlash(rel) } return real } From 6dc2ac4a3993ce54247436ff29dbedc896cb9dcc Mon Sep 17 00:00:00 2001 From: Chenghao Mou Date: Wed, 21 Oct 2026 12:54:41 +0100 Subject: [PATCH 3/3] agent simulate: say which include a test reaches twice --- cmd/lk/simulate_include_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/lk/simulate_include_test.go b/cmd/lk/simulate_include_test.go index 99779b12..426c3b23 100644 --- a/cmd/lk/simulate_include_test.go +++ b/cmd/lk/simulate_include_test.go @@ -59,7 +59,7 @@ nodes: routes: [{ match: "1", to: sam }] timeout: { after: 8s, to: goodbye } `, - // reached twice: from dental.yaml through people.yaml and menus.yaml + // includes common.yaml too, so dental.yaml reaches it twice "nodes/people.yaml": ` include: [common.yaml] nodes: