Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/behaviourtest/steps/base_dispatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ func givenCustomHarnessWithURLBase(w *world.World, name, baseName, doc string) e
// update boilerplate shared by givenCustomHarnessWithLocalBase and
// givenCustomHarnessWithURLBase.
func registerLocalAgentConfig(ctx context.Context, w *world.World, name, commitMsg string) error {
// Snapshot agents before modification so CleanupScenario can restore.
if err := snapshotAgents(w); err != nil {
return fmt.Errorf("snapshotting agents: %w", err)
}

cfgPath := path.Join(".fullsend", "config.yaml")
cfgData, err := w.SCM.GetFileContent(ctx, w.Org, w.RepoName, cfgPath)
if err != nil {
Expand Down
13 changes: 13 additions & 0 deletions pkg/behaviourtest/steps/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,19 @@ func CleanupScenario(w *world.World) {
}
}

// --- Agents cleanup ---
// Restore the pre-scenario agents list so a later scenario on this
// slot does not inherit custom agent entries (local or URL-sourced)
// from the previous lessee. Without this, harness registrations
// accumulate on the config overlay for the rest of the run.
if w.AgentsOverridden {
if err := cleanupRetry(w.Logf, "restore agents", func() error {
return RestoreAgents(w)
}); err != nil {
worldLogf(w, "behaviour cleanup: restore agents: %v", err)
}
}

// --- Dummy script cleanup ---
if len(w.DummyOps) > 0 {
if w.Org == "" || w.RepoName == "" {
Expand Down
56 changes: 56 additions & 0 deletions pkg/behaviourtest/steps/cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,62 @@ func TestCleanupScenario_RestoreAllowedResources_Error(t *testing.T) {
assert.Contains(t, logged[0], "restore allowed_remote_resources")
}

// --- Agents cleanup tests ---

func TestCleanupScenario_RestoresAgents(t *testing.T) {
t.Parallel()

scmDriver := &fakeCleanupSCM{
fileContent: []byte("version: \"1\"\nagents:\n - name: custom\n source: harness/custom.yaml\nroles:\n - triage\n"),
}
w := &world.World{
Org: "org",
RepoOwner: "org",
RepoName: "repo",
AgentsOverridden: true,
AgentsOriginal: nil, // restore to empty (install-time default)
SCM: scmDriver,
}
CleanupScenario(w)
assert.True(t, scmDriver.commitFileCalled, "should commit config to restore agents")
}

func TestCleanupScenario_SkipsAgentsWhenNotOverridden(t *testing.T) {
t.Parallel()

scmDriver := &fakeCleanupSCM{}
w := &world.World{
RepoOwner: "org",
RepoName: "repo",
AgentsOverridden: false,
SCM: scmDriver,
}
CleanupScenario(w)
assert.False(t, scmDriver.commitFileCalled, "should not commit when agents were not overridden")
}

func TestCleanupScenario_RestoreAgents_Error(t *testing.T) {
t.Parallel()

var logged []string
scmDriver := &fakeCleanupSCM{
fileContent: []byte("version: \"1\"\nagents:\n - name: custom\n source: harness/custom.yaml\nroles:\n - triage\n"),
commitFileErr: fmt.Errorf("commit failed"),
}
w := &world.World{
Org: "org",
RepoOwner: "org",
RepoName: "repo",
AgentsOverridden: true,
AgentsOriginal: nil,
SCM: scmDriver,
Logf: func(format string, args ...any) { logged = append(logged, fmt.Sprintf(format, args...)) },
}
CleanupScenario(w)
require.Len(t, logged, 1)
assert.Contains(t, logged[0], "restore agents")
}

func TestCleanupScenario_BranchScenarioSweep(t *testing.T) {
t.Parallel()

Expand Down
10 changes: 10 additions & 0 deletions pkg/behaviourtest/steps/dispatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ func givenDisabledCustomHarness(w *world.World, name, doc string) error {
return err
}

// Snapshot agents before modification so CleanupScenario can restore.
if err := snapshotAgents(w); err != nil {
return fmt.Errorf("snapshotting agents: %w", err)
}

cfgPath := filepath.Join(".fullsend", "config.yaml")
cfgData, err := w.SCM.GetFileContent(context.Background(), w.Org, w.RepoName, cfgPath)
if err != nil {
Expand Down Expand Up @@ -171,6 +176,11 @@ func givenCustomHarness(w *world.World, name, doc string) error {
return err
}

// Snapshot agents before modification so CleanupScenario can restore.
if err := snapshotAgents(w); err != nil {
return fmt.Errorf("snapshotting agents: %w", err)
}

cfgPath := filepath.Join(".fullsend", "config.yaml")
cfgData, err := w.SCM.GetFileContent(context.Background(), w.Org, w.RepoName, cfgPath)
if err != nil {
Expand Down
64 changes: 62 additions & 2 deletions pkg/behaviourtest/steps/url_dispatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,35 @@ func snapshotAllowedResources(w *world.World) error {
return nil
}

// snapshotAgents captures the current agents list from config.yaml into
// w.AgentsOriginal — but only on the first call per scenario, so that
// multiple harness steps in the same scenario do not overwrite the
// original value with an already-modified copy.
//
// Called before any cfg.SetAgents() call (dispatch.go, url_dispatch.go,
// base_dispatch.go). CleanupScenario uses the snapshot to restore the
// original agents when the scenario is done.
func snapshotAgents(w *world.World) error {
if w.AgentsOverridden {
return nil // already snapshotted this scenario
}
cfgPath := path.Join(".fullsend", "config.yaml")
cfgData, err := w.SCM.GetFileContent(context.Background(), w.Org, w.RepoName, cfgPath)
if err != nil {
return fmt.Errorf("reading config for agents snapshot: %w", err)
}
cfg, err := config.ParsePerRepoConfigWriter(cfgData)
if err != nil {
return fmt.Errorf("parsing config for agents snapshot: %w", err)
}
orig := cfg.AgentEntries()
// Store a copy so later mutations do not alias the snapshot.
w.AgentsOriginal = make([]config.AgentEntry, len(orig))
copy(w.AgentsOriginal, orig)
w.AgentsOverridden = true
return nil
}

// RestoreAllowedResources sets allowed_remote_resources back to the
// pre-scenario value captured by snapshotAllowedResources. Exported so
// CleanupScenario can call it during scenario teardown.
Expand Down Expand Up @@ -95,6 +124,33 @@ func RestoreAllowedResources(w *world.World) error {
return nil
}

// RestoreAgents sets the agents list back to the pre-scenario value
// captured by snapshotAgents. Exported so CleanupScenario can call it
// during scenario teardown.
func RestoreAgents(w *world.World) error {
if w.Org == "" || w.RepoName == "" {
return fmt.Errorf("no repo configured; call 'Given the enrolled test repository' before agent operations")
}
cfgPath := path.Join(".fullsend", "config.yaml")
cfgData, err := w.SCM.GetFileContent(context.Background(), w.Org, w.RepoName, cfgPath)
if err != nil {
return fmt.Errorf("reading config: %w", err)
}
cfg, err := config.ParsePerRepoConfigWriter(cfgData)
if err != nil {
return fmt.Errorf("parsing config: %w", err)
}
cfg.SetAgents(w.AgentsOriginal)
merged, err := cfg.Marshal()
if err != nil {
return err
}
if err := w.SCM.CommitFile(context.Background(), w.Org, w.RepoName, cfgPath, "behaviour: restore agents", merged); err != nil {
return fmt.Errorf("updating config: %w", err)
}
return nil
}

// givenHarnessHostingRepo creates a public repository to host URL-sourced
// harness YAML files. The repo is created in the same org as the test
// repository. It is idempotent — if the repo already exists, it returns
Expand Down Expand Up @@ -248,11 +304,15 @@ func givenURLSourcedCustomHarness(w *world.World, name, doc string, opts urlHarn
// Build the URL prefix for the allowlist.
urlPrefix := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/", hostOwner, hostRepo)

// Snapshot the current allowed_remote_resources before any modification
// so CleanupScenario can restore it when the slot is reused.
// Snapshot the current allowed_remote_resources and agents before any
// modification so CleanupScenario can restore them when the slot is
// reused.
if err := snapshotAllowedResources(w); err != nil {
return fmt.Errorf("snapshotting allowed_remote_resources: %w", err)
}
if err := snapshotAgents(w); err != nil {
return fmt.Errorf("snapshotting agents: %w", err)
}

// Update config.yaml on the enrolled test repo: register agent with URL
// source and update allowlist.
Expand Down
116 changes: 116 additions & 0 deletions pkg/behaviourtest/steps/url_dispatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,122 @@ func TestGivenURLSourcedCustomHarness_SnapshotsAllowlist(t *testing.T) {
"snapshot should not contain the newly added prefix")
}

// --- snapshotAgents / RestoreAgents tests ---

func TestSnapshotAgents_CapturesOriginal(t *testing.T) {
t.Parallel()
scm := &fakeURLSCM{files: map[string][]byte{
"org/repo/.fullsend/config.yaml": []byte("version: \"1\"\nagents:\n - name: triage\n source: harness/triage.yaml\n"),
}}
w := &world.World{
Org: "org",
RepoName: "repo",
SCM: scm,
}
err := snapshotAgents(w)
require.NoError(t, err)
assert.True(t, w.AgentsOverridden)
require.Len(t, w.AgentsOriginal, 1)
assert.Equal(t, "triage", w.AgentsOriginal[0].Name)
assert.Equal(t, "harness/triage.yaml", w.AgentsOriginal[0].Source)
}

func TestSnapshotAgents_OnlySnapshotsOnce(t *testing.T) {
t.Parallel()
scm := &fakeURLSCM{files: map[string][]byte{
"org/repo/.fullsend/config.yaml": []byte("version: \"1\"\nagents:\n - name: triage\n source: harness/triage.yaml\n"),
}}
w := &world.World{
Org: "org",
RepoName: "repo",
SCM: scm,
}
err := snapshotAgents(w)
require.NoError(t, err)
require.Len(t, w.AgentsOriginal, 1)

// Mutate the stored snapshot to verify it isn't overwritten on second call.
w.AgentsOriginal = nil
err = snapshotAgents(w)
require.NoError(t, err)
assert.Nil(t, w.AgentsOriginal,
"second call should not re-snapshot")
}

func TestSnapshotAgents_EmptyAgentsList(t *testing.T) {
t.Parallel()
scm := &fakeURLSCM{files: map[string][]byte{
"org/repo/.fullsend/config.yaml": []byte("version: \"1\"\nagents: []\n"),
}}
w := &world.World{
Org: "org",
RepoName: "repo",
SCM: scm,
}
err := snapshotAgents(w)
require.NoError(t, err)
assert.True(t, w.AgentsOverridden)
assert.Empty(t, w.AgentsOriginal)
}

func TestRestoreAgents_RestoresOriginal(t *testing.T) {
t.Parallel()
scm := &fakeURLSCM{files: map[string][]byte{
"org/repo/.fullsend/config.yaml": []byte("version: \"1\"\nagents:\n - name: custom\n source: harness/custom.yaml\n"),
}}
original := []config.AgentEntry{{Name: "triage", Source: "harness/triage.yaml"}}
w := &world.World{
Org: "org",
RepoName: "repo",
SCM: scm,
AgentsOriginal: original,
}
err := RestoreAgents(w)
require.NoError(t, err)

// Parse the committed config and verify agents were restored.
cfgData := scm.files["org/repo/.fullsend/config.yaml"]
require.NotNil(t, cfgData)
assert.Contains(t, string(cfgData), "triage")
assert.NotContains(t, string(cfgData), "custom")
}

func TestRestoreAgents_EmptyOrg(t *testing.T) {
t.Parallel()
w := &world.World{
RepoName: "repo",
SCM: &fakeURLSCM{files: map[string][]byte{}},
}
err := RestoreAgents(w)
require.Error(t, err)
assert.Contains(t, err.Error(), "no repo configured")
}

func TestGivenURLSourcedCustomHarness_SnapshotsAgents(t *testing.T) {
stubRawHTTPClient(t)
scm := &fakeURLSCM{files: map[string][]byte{
"my-org/my-repo/.fullsend/config.yaml": []byte(
"version: \"1\"\nagents:\n - name: existing\n source: harness/existing.yaml\nallowed_remote_resources:\n - \"https://example.com/\"\n"),
}}
w := &world.World{
Org: "my-org",
RepoName: "my-repo",
SCM: scm,
URLHarnessRepoOwner: "my-org",
URLHarnessRepoName: "harness-host",
}

err := givenURLSourcedCustomHarness(w, "url-test",
"agent: agents/triage.md\nrole: triage\nslug: url-test", urlHarnessOpts{})
require.NoError(t, err)

assert.True(t, w.AgentsOverridden,
"AgentsOverridden should be set after URL harness step")
require.Len(t, w.AgentsOriginal, 1)
assert.Equal(t, "existing", w.AgentsOriginal[0].Name,
"original agents should be preserved in snapshot")
}

// --- fakes ---

// fakeURLSCM keys files by "owner/repo/path" so multi-repo tests
Expand Down
2 changes: 2 additions & 0 deletions pkg/behaviourtest/suite/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ func resetScenarioWorld(w *world.World) {
w.RuntimeOriginal = ""
w.AllowedResourcesOverridden = false
w.AllowedResourcesOriginal = nil
w.AgentsOverridden = false
w.AgentsOriginal = nil
w.JiraMockServer = nil
w.JiraMockState = nil
w.JiraConfigDir = ""
Expand Down
5 changes: 5 additions & 0 deletions pkg/behaviourtest/suite/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/fullsend-ai/fullsend/internal/config"
"github.com/fullsend-ai/fullsend/internal/forge"
"github.com/fullsend-ai/fullsend/pkg/behaviourtest/drivers/env"
"github.com/fullsend-ai/fullsend/pkg/behaviourtest/drivers/install"
Expand Down Expand Up @@ -149,6 +150,8 @@ func TestResetScenarioWorld_ClearsSharedState(t *testing.T) {
URLHarnessRepoName: "harness-host",
AllowedResourcesOverridden: true,
AllowedResourcesOriginal: []string{"https://example.com/"},
AgentsOverridden: true,
AgentsOriginal: []config.AgentEntry{{Name: "test", Source: "harness/test.yaml"}},
}
resetScenarioWorld(w)
assert.Equal(t, 0, w.PRNumber)
Expand All @@ -164,6 +167,8 @@ func TestResetScenarioWorld_ClearsSharedState(t *testing.T) {
assert.Equal(t, "", w.URLHarnessRepoName)
assert.False(t, w.AllowedResourcesOverridden)
assert.Nil(t, w.AllowedResourcesOriginal)
assert.False(t, w.AgentsOverridden)
assert.Nil(t, w.AgentsOriginal)
}

func TestSkipErrorForTagNames(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/behaviourtest/world/world.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"path/filepath"
"time"

"github.com/fullsend-ai/fullsend/internal/config"
"github.com/fullsend-ai/fullsend/internal/forge"
"github.com/fullsend-ai/fullsend/internal/runtime"
"github.com/fullsend-ai/fullsend/pkg/behaviourtest/drivers/ci"
Expand Down Expand Up @@ -105,6 +106,13 @@ type World struct {
AllowedResourcesOverridden bool
AllowedResourcesOriginal []string

// AgentsOverridden records that this scenario modified the agents
// list in config.yaml; AgentsOriginal holds the pre-scenario value.
// CleanupScenario restores it so the next scenario on this slot
// does not inherit custom agent entries from the previous lessee.
AgentsOverridden bool
AgentsOriginal []config.AgentEntry

// Jira mock state — set by the "Given a mock Jira server" step.
JiraMockServer *httptest.Server
JiraMockState *jiramock.State
Expand Down
Loading