From c5d5b077a146b08f8008747055adb04c0e7c9456 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:03:31 +0000 Subject: [PATCH 1/5] test(#3732): add kill switch and CEL is_fork exclusion to fork dispatch contract Add the two remaining contract rows to the existing "Fork PR label dispatches harness" scenario in fork-dispatch.feature: - fork-pr-killed: a disabled harness with the same CEL trigger, asserting that the kill switch (enabled: false in config) prevents dispatch on fork events. - fork-pr-nofork: a harness whose CEL trigger includes !event.state.change_proposal.is_fork, asserting that harness-level fork exclusion produces an empty matrix. Both harnesses are added to the existing scenario (not as new scenarios) per maintainer guidance to conserve parallel execution slots. The positive harness wait provides the settle window for the negative assertions. Document the four-row fork PR behaviour contract in behaviour-testing.md as the canonical template for per-agent CEL port PRs. Note: pre-commit could not run (network restrictions in sandbox). The behaviour e2e suite requires live GitHub infrastructure and could not run in this sandbox. go vet passed. Closes #3732 --- docs/guides/dev/behaviour-testing.md | 15 +++++++++++ .../features/dispatch/fork-dispatch.feature | 27 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/docs/guides/dev/behaviour-testing.md b/docs/guides/dev/behaviour-testing.md index feed4e0b8d..20762ee8a9 100644 --- a/docs/guides/dev/behaviour-testing.md +++ b/docs/guides/dev/behaviour-testing.md @@ -170,6 +170,21 @@ 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 on fork event | Empty matrix, exit 0 | Disabled harness in same scenario; assert agent did not run | +| Fork PR `synchronize` re-triggers | Agent runs again per concurrency rules | Separate scenario with sync + label | +| CEL `is_fork` exclusion (`!event.state.change_proposal.is_fork`) | Empty matrix, exit 0 | Harness with `is_fork` guard in same scenario; assert agent did not run | + +**Consolidation pattern:** To conserve parallel execution slots, add negative-path harnesses (kill switch, 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`). + +**Unauthorized-actor denial** (ADR 54) for fork PRs is tracked separately in [#5613](https://github.com/fullsend-ai/fullsend/issues/5613) and is not part of this contract. + ## 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..a0bffa67aa 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,8 @@ 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 sync + label dispatches harness Given a custom harness "fork-pr-sync" with: From 9432941645e30f1d363c61799c396b25ac75b5a9 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 05:27:59 +0000 Subject: [PATCH 2/5] fix: link ADR 54 reference using established zero-padded format Change unlinked `(ADR 54)` to linked `([ADR 0054](...))` in the fork PR behaviour contract section, matching the consistent ADR reference pattern used throughout docs/guides/dev/. Addresses review feedback on #5665 --- docs/guides/dev/behaviour-testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/dev/behaviour-testing.md b/docs/guides/dev/behaviour-testing.md index 20762ee8a9..3de79e679a 100644 --- a/docs/guides/dev/behaviour-testing.md +++ b/docs/guides/dev/behaviour-testing.md @@ -183,7 +183,7 @@ The `fork-dispatch.feature` file defines the canonical fork-PR behaviour contrac **Consolidation pattern:** To conserve parallel execution slots, add negative-path harnesses (kill switch, 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`). -**Unauthorized-actor denial** (ADR 54) for fork PRs is tracked separately in [#5613](https://github.com/fullsend-ai/fullsend/issues/5613) and is not part of this contract. +**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. ## Forge operational constraints From 69127900c67f7d013abb52e7e4a610d3ea4b7281 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 05:55:48 +0000 Subject: [PATCH 3/5] test(#3732): add kill switch scenario and document dispatch steps Add a separate fork-PR scenario that engages kill_switch: true (the global emergency stop in config.yaml) and asserts no harness runs. This is distinct from the existing fork-pr-killed row which tests per-agent enabled: false. The new "the kill switch is active" step reads the repo config, sets kill_switch: true, and commits the change. Update the fork PR behaviour contract table in behaviour-testing.md to show kill switch and disabled harness as separate rows. Add a "Dispatch step reference" section documenting both "a disabled custom harness" and "the kill switch is active" steps with when-to-use guidance. Addresses review feedback on #5665 --- docs/guides/dev/behaviour-testing.md | 15 ++- .../features/dispatch/fork-dispatch.feature | 18 +++ pkg/behaviourtest/steps/dispatch.go | 26 ++++ pkg/behaviourtest/steps/dispatch_test.go | 113 ++++++++++++++++++ 4 files changed, 169 insertions(+), 3 deletions(-) diff --git a/docs/guides/dev/behaviour-testing.md b/docs/guides/dev/behaviour-testing.md index 3de79e679a..95907dacc6 100644 --- a/docs/guides/dev/behaviour-testing.md +++ b/docs/guides/dev/behaviour-testing.md @@ -177,14 +177,23 @@ The `fork-dispatch.feature` file defines the canonical fork-PR behaviour contrac | 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 on fork event | Empty matrix, exit 0 | Disabled harness in same scenario; assert agent did not run | +| 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` re-triggers | Agent runs again per concurrency rules | Separate scenario with sync + label | -| CEL `is_fork` exclusion (`!event.state.change_proposal.is_fork`) | Empty matrix, exit 0 | Harness with `is_fork` guard in same scenario; assert agent did not run | +| 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 | -**Consolidation pattern:** To conserve parallel execution slots, add negative-path harnesses (kill switch, 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`). +**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 a0bffa67aa..e5db9923df 100644 --- a/e2e/behaviour/features/dispatch/fork-dispatch.feature +++ b/e2e/behaviour/features/dispatch/fork-dispatch.feature @@ -67,6 +67,24 @@ Feature: Fork PR dispatch 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/dispatch.go b/pkg/behaviourtest/steps/dispatch.go index 482f10c3c1..509b8f8dce 100644 --- a/pkg/behaviourtest/steps/dispatch.go +++ b/pkg/behaviourtest/steps/dispatch.go @@ -35,6 +35,32 @@ 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. +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) + } + 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 c1d92ae14b..010612abfe 100644 --- a/pkg/behaviourtest/steps/dispatch_test.go +++ b/pkg/behaviourtest/steps/dispatch_test.go @@ -1,6 +1,8 @@ package steps import ( + "context" + "fmt" "testing" "time" @@ -36,6 +38,117 @@ 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") +} + +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 TestNegativeSettleDuration(t *testing.T) { now := time.Date(2026, 7, 23, 12, 0, 0, 0, time.UTC) From 021c38f7d76dd0a41cfa79ee8fcda6d439e2ba2e Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:12:07 +0000 Subject: [PATCH 4/5] fix: correct synchronize row in fork PR behaviour contract table Update the "Fork PR synchronize re-triggers" row to accurately reflect what the test actually verifies: the sync+label sequence dispatches the harness exactly 1 time with a successful workflow, rather than overspecifying "re-triggers" and "concurrency rules". Addresses review feedback on #5665 --- docs/guides/dev/behaviour-testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/dev/behaviour-testing.md b/docs/guides/dev/behaviour-testing.md index 24169c0058..f8b82205bb 100644 --- a/docs/guides/dev/behaviour-testing.md +++ b/docs/guides/dev/behaviour-testing.md @@ -235,7 +235,7 @@ The `fork-dispatch.feature` file defines the canonical fork-PR behaviour contrac | 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` re-triggers | Agent runs again per concurrency rules | Separate scenario with sync + label | +| 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. From 186e96c91b0a65e0f5d05c03946a627f134f8c0e Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 06:46:18 +0000 Subject: [PATCH 5/5] fix: deactivate kill switch in AfterScenario cleanup The kill-switch scenario sets kill_switch: true in the enrolled repo's config.yaml but never clears it. Since behaviour test slots are reused across scenarios within a suite run, subsequent scenarios on the same slot fail because the kill switch is still active. Add automatic cleanup tied to kill-switch activation: - Track activation via KillSwitchActivated field on World - Deactivate kill switch in CleanupScenario (same pattern as DummyOps) - Reset field in resetScenarioWorld for each new scenario Addresses review feedback on #5665 --- pkg/behaviourtest/steps/cleanup.go | 11 +++++ pkg/behaviourtest/steps/cleanup_test.go | 60 +++++++++++++++++++++++- pkg/behaviourtest/steps/dispatch.go | 27 +++++++++++ pkg/behaviourtest/steps/dispatch_test.go | 42 +++++++++++++++++ pkg/behaviourtest/suite/init.go | 1 + pkg/behaviourtest/world/world.go | 5 ++ 6 files changed, 145 insertions(+), 1 deletion(-) 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 b3c46d3c2b..a7008858e4 100644 --- a/pkg/behaviourtest/steps/dispatch.go +++ b/pkg/behaviourtest/steps/dispatch.go @@ -44,6 +44,8 @@ func registerDispatchSteps(sc *godog.ScenarioContext) { // 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) @@ -62,6 +64,31 @@ func givenKillSwitchActive(w *world.World) error { 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 } diff --git a/pkg/behaviourtest/steps/dispatch_test.go b/pkg/behaviourtest/steps/dispatch_test.go index 1b812b6219..001c39ff92 100644 --- a/pkg/behaviourtest/steps/dispatch_test.go +++ b/pkg/behaviourtest/steps/dispatch_test.go @@ -52,6 +52,48 @@ func TestGivenKillSwitchActive_SetsKillSwitch(t *testing.T) { 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) { 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