diff --git a/docs/guides/dev/behaviour-testing.md b/docs/guides/dev/behaviour-testing.md index 8660b0684c..f8b82205bb 100644 --- a/docs/guides/dev/behaviour-testing.md +++ b/docs/guides/dev/behaviour-testing.md @@ -226,6 +226,30 @@ Background: The `Given a fork` step remaps the logical name as above and is idempotent for that actual fork repo. Each scenario then creates its own branch and PR within the fork. +### Fork PR behaviour contract + +The `fork-dispatch.feature` file defines the canonical fork-PR behaviour contract for `harness-dispatch`. Each CEL port PR ([#2896](https://github.com/fullsend-ai/fullsend/issues/2896)–[#2901](https://github.com/fullsend-ai/fullsend/issues/2901)) should follow this contract when adding fork-PR rows to the agent's harness behaviour feature file. + +| Scenario | Expected | How tested | +|----------|----------|------------| +| Fork PR matches CEL trigger; authorized actor | Agent runs via `harness-dispatch`; workflow completes | Positive dispatch + artifact assertion | +| Kill switch active (`kill_switch: true`) on fork event | Empty matrix, exit 0 | Separate scenario; `the kill switch is active` step + assert agent did not run | +| Disabled harness (`enabled: false`) on fork event | Empty matrix, exit 0 | Disabled harness in positive scenario; assert agent did not run | +| Fork PR `synchronize` + label dispatches harness | Harness dispatched exactly 1 time; workflow completes | Separate scenario with sync commit + label | +| CEL `is_fork` exclusion (`!event.state.change_proposal.is_fork`) | Empty matrix, exit 0 | Harness with `is_fork` guard in positive scenario; assert agent did not run | + +**Kill switch vs disabled harness:** These are distinct mechanisms. The **kill switch** (`kill_switch: true` in `config.yaml`) is a global emergency stop that blocks *all* harness dispatch for the repo — tested in its own scenario because no positive harness can run alongside it. A **disabled harness** (`enabled: false` per agent entry) only prevents that single agent from running — tested as a piggyback negative assertion in the positive-path scenario. + +**Consolidation pattern:** To conserve parallel execution slots, add negative-path harnesses (disabled agent, CEL exclusion) alongside the positive-path harness in a single scenario rather than creating separate scenarios. The positive harness wait acts as the settle window for negative assertions (piggyback pattern — see `negativeSettleDuration` in `dispatch.go`). The kill switch scenario cannot be consolidated because it blocks all harnesses. + +**Unauthorized-actor denial** ([ADR 0054](../../ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md)) for fork PRs is tracked separately in [#5613](https://github.com/fullsend-ai/fullsend/issues/5613) and is not part of this contract. + +### Dispatch step reference + +**`a disabled custom harness "" with:`** — Registers the harness YAML under `.fullsend/harness/.yaml` and adds an agent entry with `enabled: false` to the repo's `config.yaml`. Use this step for negative dispatch assertions where a single agent should be excluded while other agents in the same scenario continue to run. This is *not* the kill switch; for the global emergency stop that blocks all harnesses, use `the kill switch is active`. + +**`the kill switch is active`** — Sets `kill_switch: true` in the repo's `config.yaml`, causing `Dispatch` to return an empty matrix for *all* agents. Use this step in a dedicated scenario where no harness should run. Because the kill switch blocks everything, it cannot share a scenario with a positive-path harness. + ## Forge operational constraints When modifying behaviour test repo provisioning, fork handling, or workflow dispatch, be aware of these constraints. They are not enforced by the compiler or linter — violations surface as cryptic API errors or silently dropped events in CI. diff --git a/e2e/behaviour/features/dispatch/fork-dispatch.feature b/e2e/behaviour/features/dispatch/fork-dispatch.feature index 45b8072b48..e5db9923df 100644 --- a/e2e/behaviour/features/dispatch/fork-dispatch.feature +++ b/e2e/behaviour/features/dispatch/fork-dispatch.feature @@ -29,6 +29,31 @@ Feature: Fork PR dispatch && event.transition.kind == "label_changed" && event.transition.label.name == "ready-for-fork-ping" """ + And a disabled custom harness "fork-pr-killed" with: + """ + agent: agents/triage.md + role: triage + slug: fullsend-ai-fork-pr-killed + model: opus + image: ghcr.io/fullsend-ai/fullsend-sandbox:latest + trigger: | + event.entity.kind == "change_proposal" + && event.transition.kind == "label_changed" + && event.transition.label.name == "ready-for-fork-ping" + """ + And a custom harness "fork-pr-nofork" with: + """ + agent: agents/triage.md + role: triage + slug: fullsend-ai-fork-pr-nofork + model: opus + image: ghcr.io/fullsend-ai/fullsend-sandbox:latest + trigger: | + event.entity.kind == "change_proposal" + && event.transition.kind == "label_changed" + && event.transition.label.name == "ready-for-fork-ping" + && !event.state.change_proposal.is_fork + """ And a dummy agent that would: | description | op | args | | Fork PR payload | assert_json | .fullsend/dispatch/event-payload.json,pull_request.head.repo.fork | @@ -39,6 +64,26 @@ Feature: Fork PR dispatch And the agent will succeed to Prove fork execution And the harness "fork-pr-ping" was dispatched exactly 1 time And the harness "fork-issue-ping" agent did not run + And the harness "fork-pr-killed" agent did not run + And the harness "fork-pr-nofork" agent did not run + + Scenario: Fork PR kill switch blocks all harnesses + Given a custom harness "fork-pr-killswitch" with: + """ + agent: agents/triage.md + role: triage + slug: fullsend-ai-fork-pr-killswitch + model: opus + image: ghcr.io/fullsend-ai/fullsend-sandbox:latest + trigger: | + event.entity.kind == "change_proposal" + && event.transition.kind == "label_changed" + && event.transition.label.name == "ready-for-fork-killswitch" + """ + And the kill switch is active + When a fork pull request is opened + And the fork pull request is labeled "ready-for-fork-killswitch" + Then the harness "fork-pr-killswitch" agent did not run Scenario: Fork PR sync + label dispatches harness Given a custom harness "fork-pr-sync" with: diff --git a/pkg/behaviourtest/steps/cleanup.go b/pkg/behaviourtest/steps/cleanup.go index 05c07e9b2d..4fa7c8eea1 100644 --- a/pkg/behaviourtest/steps/cleanup.go +++ b/pkg/behaviourtest/steps/cleanup.go @@ -76,6 +76,17 @@ func CleanupScenario(w *world.World) { } } + // --- Kill switch cleanup --- + // Deactivate the kill switch so the next scenario on this slot is + // not blocked by sticky state. Runs before dummy-script cleanup + // because the kill switch is a repo-level config that affects all + // harnesses. + if w.KillSwitchActivated { + if err := DeactivateKillSwitch(w); err != nil { + worldLogf(w, "behaviour cleanup: deactivate kill switch: %v", err) + } + } + // --- Dummy script cleanup --- if len(w.DummyOps) > 0 { empty := []byte("ops: []\n") diff --git a/pkg/behaviourtest/steps/cleanup_test.go b/pkg/behaviourtest/steps/cleanup_test.go index 822b1c5e4e..78cf0ef9cd 100644 --- a/pkg/behaviourtest/steps/cleanup_test.go +++ b/pkg/behaviourtest/steps/cleanup_test.go @@ -429,6 +429,8 @@ type fakeCleanupSCM struct { deleteRepoErr error commitFileCalled bool commitFileErr error + fileContent []byte + getFileErr error } type closedIssueRecord struct { @@ -491,7 +493,7 @@ func (f *fakeCleanupSCM) GetIssue(context.Context, string, string, int) (*forge. } func (f *fakeCleanupSCM) GetFileContent(context.Context, string, string, string) ([]byte, error) { - return nil, nil + return f.fileContent, f.getFileErr } func (f *fakeCleanupSCM) CommitFile(_ context.Context, _, _, _, _ string, _ []byte) error { @@ -634,6 +636,62 @@ func TestCleanupScenario_ClearsDummyOps_Error(t *testing.T) { assert.Contains(t, logged[0], "clear dummy script") } +// --- Kill switch cleanup tests --- + +func TestCleanupScenario_DeactivatesKillSwitch(t *testing.T) { + t.Parallel() + + scmDriver := &fakeCleanupSCM{ + fileContent: []byte("version: \"1\"\nkill_switch: true\nroles:\n - triage\n"), + } + installDriver := &fakeCleanupInstall{owner: "org", repo: "repo"} + w := &world.World{ + RepoOwner: "org", + RepoName: "repo", + KillSwitchActivated: true, + Install: installDriver, + SCM: scmDriver, + } + CleanupScenario(w) + assert.True(t, scmDriver.commitFileCalled, "should commit config to deactivate kill switch") +} + +func TestCleanupScenario_SkipsKillSwitchWhenNotActivated(t *testing.T) { + t.Parallel() + + scmDriver := &fakeCleanupSCM{} + w := &world.World{ + RepoOwner: "org", + RepoName: "repo", + KillSwitchActivated: false, + SCM: scmDriver, + } + CleanupScenario(w) + assert.False(t, scmDriver.commitFileCalled, "should not commit when kill switch was not activated") +} + +func TestCleanupScenario_DeactivateKillSwitch_Error(t *testing.T) { + t.Parallel() + + var logged []string + scmDriver := &fakeCleanupSCM{ + fileContent: []byte("version: \"1\"\nkill_switch: true\nroles:\n - triage\n"), + commitFileErr: fmt.Errorf("commit failed"), + } + installDriver := &fakeCleanupInstall{owner: "org", repo: "repo"} + w := &world.World{ + RepoOwner: "org", + RepoName: "repo", + KillSwitchActivated: true, + Install: installDriver, + 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], "deactivate kill switch") +} + // fakeCleanupInstall satisfies the Install interface for cleanup tests. type fakeCleanupInstall struct { owner string diff --git a/pkg/behaviourtest/steps/dispatch.go b/pkg/behaviourtest/steps/dispatch.go index 63b269b95f..a7008858e4 100644 --- a/pkg/behaviourtest/steps/dispatch.go +++ b/pkg/behaviourtest/steps/dispatch.go @@ -37,6 +37,59 @@ func registerDispatchSteps(sc *godog.ScenarioContext) { sc.Step(`^a review comment is submitted on the pull request$`, func(ctx context.Context) (context.Context, error) { return ctx, whenPullRequestReviewComment(world.FromContext(ctx)) }) + sc.Step(`^the kill switch is active$`, func(ctx context.Context) (context.Context, error) { + return ctx, givenKillSwitchActive(world.FromContext(ctx)) + }) +} + +// givenKillSwitchActive sets kill_switch: true in the enrolled repo's +// config.yaml, causing Dispatch to return an empty matrix for all agents. +// It also marks w.KillSwitchActivated so CleanupScenario deactivates the +// switch before the slot is reused by another scenario. +func givenKillSwitchActive(w *world.World) error { + cfgPath := filepath.Join(".fullsend", "config.yaml") + cfgData, err := w.SCM.GetFileContent(context.Background(), w.Install.ConfigOwner(), w.Install.ConfigRepo(), 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.SetKillSwitch(true) + merged, err := cfg.Marshal() + if err != nil { + return err + } + if err := w.SCM.CommitFile(context.Background(), w.Install.ConfigOwner(), w.Install.ConfigRepo(), cfgPath, "behaviour: activate kill switch", merged); err != nil { + return fmt.Errorf("updating config: %w", err) + } + w.KillSwitchActivated = true + return nil +} + +// DeactivateKillSwitch sets kill_switch: false in the enrolled repo's +// config.yaml. Exported so CleanupScenario (in package steps) can call +// it during scenario teardown. +func DeactivateKillSwitch(w *world.World) error { + cfgPath := filepath.Join(".fullsend", "config.yaml") + cfgData, err := w.SCM.GetFileContent(context.Background(), w.Install.ConfigOwner(), w.Install.ConfigRepo(), 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.SetKillSwitch(false) + merged, err := cfg.Marshal() + if err != nil { + return err + } + if err := w.SCM.CommitFile(context.Background(), w.Install.ConfigOwner(), w.Install.ConfigRepo(), cfgPath, "behaviour: deactivate kill switch", merged); err != nil { + return fmt.Errorf("updating config: %w", err) + } + return nil } func givenDisabledCustomHarness(w *world.World, name, doc string) error { diff --git a/pkg/behaviourtest/steps/dispatch_test.go b/pkg/behaviourtest/steps/dispatch_test.go index a4b6cb8c77..001c39ff92 100644 --- a/pkg/behaviourtest/steps/dispatch_test.go +++ b/pkg/behaviourtest/steps/dispatch_test.go @@ -2,6 +2,7 @@ package steps import ( "context" + "fmt" "testing" "time" @@ -37,6 +38,167 @@ func TestEnsureHarnessArtifacts_NoWorkflowRun(t *testing.T) { assert.Contains(t, err.Error(), "workflow run") } +// --- givenKillSwitchActive tests --- + +func TestGivenKillSwitchActive_SetsKillSwitch(t *testing.T) { + scm := &fakeDispatchSCM{ + fileContent: []byte("version: \"1\"\nroles:\n - triage\n"), + } + w := &world.World{ + SCM: scm, + Install: &fakeDispatchInstall{owner: "org", repo: "repo"}, + } + err := givenKillSwitchActive(w) + require.NoError(t, err) + assert.True(t, scm.commitCalled, "CommitFile should have been called") + assert.Contains(t, string(scm.committedContent), "kill_switch: true") + assert.True(t, w.KillSwitchActivated, "KillSwitchActivated should be set for cleanup") +} + +func TestDeactivateKillSwitch_ClearsKillSwitch(t *testing.T) { + scm := &fakeDispatchSCM{ + fileContent: []byte("version: \"1\"\nkill_switch: true\nroles:\n - triage\n"), + } + w := &world.World{ + SCM: scm, + Install: &fakeDispatchInstall{owner: "org", repo: "repo"}, + } + err := DeactivateKillSwitch(w) + require.NoError(t, err) + assert.True(t, scm.commitCalled, "CommitFile should have been called") + assert.Contains(t, string(scm.committedContent), "kill_switch: false") +} + +func TestDeactivateKillSwitch_GetFileContentError(t *testing.T) { + scm := &fakeDispatchSCM{ + getFileErr: fmt.Errorf("not found"), + } + w := &world.World{ + SCM: scm, + Install: &fakeDispatchInstall{owner: "org", repo: "repo"}, + } + err := DeactivateKillSwitch(w) + require.Error(t, err) + assert.Contains(t, err.Error(), "reading config") +} + +func TestDeactivateKillSwitch_CommitFileError(t *testing.T) { + scm := &fakeDispatchSCM{ + fileContent: []byte("version: \"1\"\nkill_switch: true\nroles:\n - triage\n"), + commitErr: fmt.Errorf("commit failed"), + } + w := &world.World{ + SCM: scm, + Install: &fakeDispatchInstall{owner: "org", repo: "repo"}, + } + err := DeactivateKillSwitch(w) + require.Error(t, err) + assert.Contains(t, err.Error(), "updating config") +} + +func TestGivenKillSwitchActive_GetFileContentError(t *testing.T) { + scm := &fakeDispatchSCM{ + getFileErr: fmt.Errorf("not found"), + } + w := &world.World{ + SCM: scm, + Install: &fakeDispatchInstall{owner: "org", repo: "repo"}, + } + err := givenKillSwitchActive(w) + require.Error(t, err) + assert.Contains(t, err.Error(), "reading config") +} + +func TestGivenKillSwitchActive_CommitFileError(t *testing.T) { + scm := &fakeDispatchSCM{ + fileContent: []byte("version: \"1\"\nroles:\n - triage\n"), + commitErr: fmt.Errorf("commit failed"), + } + w := &world.World{ + SCM: scm, + Install: &fakeDispatchInstall{owner: "org", repo: "repo"}, + } + err := givenKillSwitchActive(w) + require.Error(t, err) + assert.Contains(t, err.Error(), "updating config") +} + +// fakeDispatchInstall implements install.State for dispatch step tests. +type fakeDispatchInstall struct { + owner string + repo string +} + +func (f *fakeDispatchInstall) Mode() string { return "per-repo" } +func (f *fakeDispatchInstall) TestRepo() string { return f.repo } +func (f *fakeDispatchInstall) ConfigOwner() string { return f.owner } +func (f *fakeDispatchInstall) ConfigRepo() string { return f.repo } +func (f *fakeDispatchInstall) ConfigPathPrefix() string { return ".fullsend" } +func (f *fakeDispatchInstall) TriageWorkflowRepo() string { return f.repo } +func (f *fakeDispatchInstall) TriageWorkflowFile() string { return "" } +func (f *fakeDispatchInstall) AgentWorkflowFile() string { return "" } +func (f *fakeDispatchInstall) AgentArtifactName() string { return "" } + +// fakeDispatchSCM implements scm.Driver for dispatch step tests. +type fakeDispatchSCM struct { + fileContent []byte + getFileErr error + commitCalled bool + committedContent []byte + commitErr error +} + +func (f *fakeDispatchSCM) GetFileContent(_ context.Context, _, _, _ string) ([]byte, error) { + return f.fileContent, f.getFileErr +} +func (f *fakeDispatchSCM) CommitFile(_ context.Context, _, _, _, _ string, content []byte) error { + f.commitCalled = true + f.committedContent = content + return f.commitErr +} +func (f *fakeDispatchSCM) CreateIssue(context.Context, string, string, string, string, ...string) (*forge.Issue, error) { + return nil, nil +} +func (f *fakeDispatchSCM) AddIssueLabels(context.Context, string, string, int, ...string) error { + return nil +} +func (f *fakeDispatchSCM) AddComment(context.Context, string, string, int, string) (*forge.IssueComment, error) { + return nil, nil +} +func (f *fakeDispatchSCM) GetIssue(context.Context, string, string, int) (*forge.Issue, error) { + return nil, nil +} +func (f *fakeDispatchSCM) CreateBranch(context.Context, string, string, string) error { return nil } +func (f *fakeDispatchSCM) DeleteBranch(context.Context, string, string, string) error { return nil } +func (f *fakeDispatchSCM) CommitFileToBranch(context.Context, string, string, string, string, string, []byte) error { + return nil +} +func (f *fakeDispatchSCM) CreateChangeProposal(context.Context, string, string, string, string, string, string) (*forge.ChangeProposal, error) { + return nil, nil +} +func (f *fakeDispatchSCM) SubmitPullRequestReview(context.Context, string, string, int, string) error { + return nil +} +func (f *fakeDispatchSCM) CloseIssue(context.Context, string, string, int) error { return nil } +func (f *fakeDispatchSCM) DeleteRepo(context.Context, string, string) error { return nil } +func (f *fakeDispatchSCM) CreateFork(context.Context, string, string, string) (string, error) { + return "", nil +} +func (f *fakeDispatchSCM) CommitFileToFork(context.Context, string, string, string, string, string, []byte) error { + return nil +} +func (f *fakeDispatchSCM) CreateForkChangeProposal(context.Context, string, string, string, string, string, string, string, string) (*forge.ChangeProposal, error) { + return nil, nil +} +func (f *fakeDispatchSCM) CreateRepo(context.Context, string, string, string) error { return nil } +func (f *fakeDispatchSCM) EnsureRepoPublic(context.Context, string, string) error { return nil } +func (f *fakeDispatchSCM) GetDefaultBranch(context.Context, string, string) (string, error) { + return "main", nil +} +func (f *fakeDispatchSCM) GetBranchRef(context.Context, string, string, string) (string, error) { + return "abc123", nil +} + func TestNegativeSettleDuration(t *testing.T) { now := time.Date(2026, 7, 23, 12, 0, 0, 0, time.UTC) diff --git a/pkg/behaviourtest/suite/init.go b/pkg/behaviourtest/suite/init.go index eb544ab5e9..2e0e7d8e8c 100644 --- a/pkg/behaviourtest/suite/init.go +++ b/pkg/behaviourtest/suite/init.go @@ -96,6 +96,7 @@ func resetScenarioWorld(w *world.World) { w.URLHarnessRepoOwner = "" w.URLHarnessRepoName = "" w.LeasedRepoName = "" + w.KillSwitchActivated = false w.JiraMockServer = nil w.JiraMockState = nil w.JiraConfigDir = "" diff --git a/pkg/behaviourtest/world/world.go b/pkg/behaviourtest/world/world.go index 8857b34905..22654d3f73 100644 --- a/pkg/behaviourtest/world/world.go +++ b/pkg/behaviourtest/world/world.go @@ -64,6 +64,11 @@ type World struct { // Nil when lazy ensure is not configured. Ensurer install.RepoEnsurer + // KillSwitchActivated records whether this scenario activated the + // repo-level kill switch. CleanupScenario uses this to deactivate + // the switch so the next scenario on this slot is not affected. + KillSwitchActivated bool + // Jira mock state — set by the "Given a mock Jira server" step. JiraMockServer *httptest.Server JiraMockState *jiramock.State