feat(behaviour): add isolated dev behaviour tests

[ifireball]

Closes #1242

Summary

• add the agents behaviour suite, fixtures, and make target
• run only with ENVIRONMENT=dev
• vendor the exact pinned fullsend source in CI so the test binary uses fullsend assets
• use a unique suite name per workflow run
• map isolated agents credentials into the existing fullsend dev driver interface

Provisioning

• added a dedicated agents GitHub Actions WIF provider and service account in it-gcp-konflux-dev-fullsend
• added isolated PEM, actor PAT, Cloudflare, and GCP secrets to the agents dev Environment
• authorized fullsend-ai/agents in the dev pool organizations while preserving existing callers
• left existing credentials and all stage configuration unchanged

Validation

• actionlint passed
• compile-only behaviour test passed against the exact pinned fullsend commit
• git diff --check passed

[qodo-code-review]

PR Summary by Qodo

Add isolated dev behaviour tests for agents

🧪 Tests ✨ Enhancement ⚙️ Configuration changes 📝 Documentation 🕐 40+ Minutes

AI Description

• Adds isolated triage behaviour coverage using fullsend’s shared live test runner.
• Gates secret-bearing CI runs and restricts execution to the dev environment.
• Pins fullsend source and maps dedicated agents credentials into its driver.

Diagram

graph TD
  A["Workflow event"] --> B{"PR event?"} -->|Yes| C["Authorization gate"] --> D["Dev environment"] --> E["GCP WIF"] --> F["Pinned fullsend"] --> G["Behaviour suite"] --> H["Dev driver"]
  B -->|No| D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use the published module only

• ➕ Avoids cloning fullsend and generating a temporary module file.
• ➕ Simplifies CI setup and dependency resolution.
• ➖ Does not provide the fullsend working-tree assets required by the live runner.
• ➖ May compile against one revision while executing mismatched external assets.

2. Move scenarios into fullsend

• ➕ Keeps runner, fixtures, and source assets in one repository.
• ➕ Eliminates cross-repository checkout and fixture-linking logic.
• ➖ Moves agents-specific behavior ownership out of the agents repository.
• ➖ Makes agent changes dependent on coordinated fullsend updates.

3. Export fullsend test assets

• ➕ Could provide assets through a stable package or configurable path.
• ➕ Would eventually remove the checkout and symlink workaround.
• ➖ Requires an upstream fullsend interface change.
• ➖ Adds migration work before isolated agents tests can run.

Recommendation: Use the PR’s pinned-checkout approach now because it guarantees that the test runner and fullsend assets come from the exact dependency revision while keeping agents scenarios isolated. Exporting configurable test assets from fullsend would be a cleaner long-term option, but it requires upstream changes beyond this PR’s scope.

Files changed (9) +590 / -1

Tests (3) +61 / -0

triage.featureDefine ready-to-code triage scenario +13/-0

Define ready-to-code triage scenario

• Adds a Gherkin scenario that enrolls a repository, injects deterministic triage output, triggers triage, and verifies successful completion and the ready-to-code label.

behaviour/features/triage.feature

sufficient.jsonAdd sufficient-triage result fixture +24/-0

Add sufficient-triage result fixture

• Provides deterministic agent output representing a sufficiently specified issue, including clarity scores, summary data, and the expected comment.

behaviour/fixtures/triage/sufficient.json

suite_test.goRegister the agents behaviour suite +24/-0

Register the agents behaviour suite

• Adds a build-tagged Go test entry point that invokes fullsend’s shared behaviour runner with the agents feature and fixture locations.

behaviour/suite_test.go

Documentation (1) +77 / -0

behaviour-testing.mdDocument behaviour testing and provisioning +77/-0

Document behaviour testing and provisioning

• Documents local and live commands, pinned fullsend checkout requirements, isolated dev secrets, and dev-pool authorization. It explicitly excludes durable stage credentials and permissions.

docs/behaviour-testing.md

Other (5) +452 / -1

behaviour-tests.ymlAdd gated dev behaviour-test workflow +179/-0

Add gated dev behaviour-test workflow

• Adds live behaviour-test triggers for relevant pushes, merge groups, manual runs, and authorized pull requests. The workflow validates isolated credentials, authenticates through GCP WIF, checks out the exact pinned fullsend source, and runs the suite with a unique name.

.github/workflows/behaviour-tests.yml

MakefileAdd configurable behaviour-test target +8/-1

Add configurable behaviour-test target

• Adds a make target that runs the build-tagged behaviour package through the execution wrapper. Runner path, build tags, and Go module flags remain configurable for local and CI use.

Makefile

go.modInitialize Go module with pinned fullsend dependency +5/-0

Initialize Go module with pinned fullsend dependency

• Defines the agents Go module and pins the exact fullsend pseudo-version needed by the shared behaviour-test runner.

go.mod

go.sumLock behaviour runner dependency graph +229/-0

Lock behaviour runner dependency graph

• Records checksums for fullsend and its transitive dependencies so behaviour-test builds resolve reproducibly.

go.sum

run-behaviour-test-exec.shRun tests from the pinned fullsend checkout +31/-0

Run tests from the pinned fullsend checkout

• Adds a go test execution wrapper that resolves the test binary before changing directories. When a fullsend checkout is configured, it temporarily links agents fixtures into that checkout, runs there, and removes the link on exit.

scripts/run-behaviour-test-exec.sh

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:05 AM UTC · Completed 11:28 AM UTC

Commit: e8efba6 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $5.87

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (2)

1. Fork code receives development secrets ✗ Dismissed 📜 Skill insight ⛨ Security

Description

The behaviour job checks out github.event.pull_request.head.sha during a pull_request_target
run and then executes the checked-out repository through make behaviour-test. When the
authorization gate accepts a fork PR, its head-controlled Makefile and scripts run with the GCP,
Cloudflare, private-key, and actor-token secrets mapped into the job environment.

Code

.github/workflows/behaviour-tests.yml[R109-111]

+          ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }}
+          persist-credentials: false
+          allow-unsafe-pr-checkout: ${{ github.event_name == 'pull_request_target' }}

Relevance

●●● Strong

The workflow executes fork-controlled code with secrets, matching the repository’s accepted
privileged CI security concerns.

PR-#89
PR-#776

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 1538384 prohibits pull_request_target workflows from running fork code with repository
secrets. The workflow enables that event, exposes numerous environment secrets, performs an
explicitly unsafe checkout of the PR head, and subsequently runs code from that checkout.

.github/workflows/behaviour-tests.yml[26-27]
.github/workflows/behaviour-tests.yml[92-111]
.github/workflows/behaviour-tests.yml[178-179]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `pull_request_target` workflow executes fork-controlled PR code while development credentials are present in the job environment.

## Fix Focus Areas
- .github/workflows/behaviour-tests.yml[26-27]
- .github/workflows/behaviour-tests.yml[92-111]
- .github/workflows/behaviour-tests.yml[178-179]

## Recommended Fix
Do not execute PR-head code in a secret-bearing `pull_request_target` job. Restrict live secret-dependent tests to trusted push or merge-group revisions, and use a secret-free `pull_request` job for fork validation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Workflow error values stay unsanitized ⊘ Outdated 📜 Skill insight ⛨ Security

Description

Validate behaviour credentials interpolates ${name} into an ::error:: workflow command without
applying a workflow-command sanitizer. Every missing credential reaches this branch, so future
entries containing command delimiters, encoded newlines, ANSI escapes, or control characters would
be emitted without escaping.

Code

.github/workflows/behaviour-tests.yml[136]

+              echo "::error::missing behaviour credential: ${name}"

Relevance

●● Moderate

Sanitization findings have mixed outcomes: similar interpolations were both accepted and rejected
recently.

PR-#90
PR-#148
PR-#776

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rules 1538312 and 1538382 require every interpolated GitHub Actions workflow-command value to be
sanitized individually regardless of perceived risk. Line 136 places ${name} directly in the
command body without any sanitization.

.github/workflows/behaviour-tests.yml[127-139]
Skill: code-review
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The credential-validation error command directly interpolates its loop value without sanitizing every workflow-command-sensitive sequence.

## Fix Focus Areas
- .github/workflows/behaviour-tests.yml[127-139]

## Recommended Fix
Add a workflow-command sanitizer that escapes percent signs and newlines and removes command delimiters, ANSI escapes, and control characters. Apply it to `${name}` before constructing the `::error::` command.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Test scenarios embed agent instructions 📜 Skill insight ⛨ Security

Description

triage.feature defines dummy-agent behavior through the instruction-like phrase `a dummy agent
that would and the imperative write_fixture` operation. Loading the scenario feeds this newly
committed instruction pattern into the behavior runner's agent configuration surface.

Code

behaviour/features/triage.feature[R5-7]

+    And a dummy agent that would:
+      | description      | op            | args                                                      |
+      | Emit triage JSON | write_fixture | output/agent-result.json, fixtures/triage/sufficient.json |

Relevance

●● Moderate

No scoped history exists for behaviour scenarios, making this policy interpretation uncertain
despite the explicit instruction-like wording.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 1538322 prohibits agent instruction patterns in configuration values and string-like test
content. The Gherkin scenario explicitly introduces what a dummy agent should do and supplies an
operation for it to perform.

behaviour/features/triage.feature[5-7]
Skill: code-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new behavior scenario contains an explicit agent-instruction pattern prohibited in comments, strings, and configuration content.

## Fix Focus Areas
- behaviour/features/triage.feature[5-7]

## Recommended Fix
Represent the dummy output through a non-instructional fixture or structured test configuration supported by the behavior runner. Preserve the asserted triage outcome without embedding role-play or agent-directed wording.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View high (1)

4. Protected changes require human review 📜 Skill insight § Compliance

Description

The PR adds .github/workflows/behaviour-tests.yml and scripts/run-behaviour-test-exec.sh, both
paths that the governance rule classifies as protected. Issue #1242 and the PR description justify
the behavior-test infrastructure, but these additions still require human approval rather than
automated acceptance.

Code

.github/workflows/behaviour-tests.yml[1]

+name: Behaviour Tests

Relevance

●● Moderate

Protected-path precedent rejects adding approval comments, but does not clearly establish rejecting
the governance finding itself.

PR-#754

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 1538392 mandates a finding whenever protected governance or infrastructure paths are modified.
This PR adds both a workflow under .github/ and an executable under scripts/; the linked issue
supplies justification but does not remove the human-review requirement.

.github/workflows/behaviour-tests.yml[1-1]
scripts/run-behaviour-test-exec.sh[1-2]
Skill: pr-review

---

5. Schema changes skip live tests ✓ Resolved 🐞 Bug ≡ Correctness

Description

The push.paths filter omits schemas/** even though harness/triage.yaml validates the
scenario's generated result against schemas/triage-result.schema.json. A schema-only push to
main therefore changes whether the triage fixture is accepted without running the new live
behaviour test.

Code

.github/workflows/behaviour-tests.yml[R16-19]

+      - 'env/**'
+      - 'harness/**'
+      - 'policies/**'
+      - 'scripts/**'

Relevance

●●● Strong

PR #78 accepted the same workflow path-filter regression involving omitted runtime inputs.

PR-#78

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new workflow lists its main-push triggers without schemas/**, while the triage harness
explicitly uses the triage result schema and the scenario supplies output that must pass that
validation. Past PR #78 documents the same class of regression caused by omitting a runtime input
from a functional workflow's path filter.

.github/workflows/behaviour-tests.yml[12-25]
harness/triage.yaml[33-39]
behaviour/features/triage.feature[5-13]
PR-#78

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The behaviour workflow does not run on main-branch pushes that only change schemas, although the triage harness directly uses the triage result schema during this scenario.

## Fix Focus Areas
- .github/workflows/behaviour-tests.yml[12-25]

## Recommended Fix
Add `schemas/**` to the workflow's `on.push.paths` list so schema-only changes trigger the live behaviour suite.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 57 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

✅ Cross-repo context — repo relationships

  Explored: repo: fullsend-ai/fullsend (sha: e57fcc2e)

Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 11/18, lines 591/200; both must reach the floor). Router rationale: This introduces security-sensitive GitHub Actions secret handling and cloud authorization plus several independent workflow, test harness, dependency, and shell-script paths, creating a dense set of easy-to-miss defects.

Tip of the day

💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Review

Findings

Medium

• [logic-error] .github/workflows/behaviour-tests.yml:202 — In the "Resolve pinned fullsend source" step, go mod tidy -modfile="$modfile" still runs without -tags behaviour. The only Go file in this module, behaviour/suite_test.go, is guarded by //go:build behaviour, so a tag-less tidy does not see the github.com/fullsend-ai/fullsend import. The step also truncates ${RUNNER_TEMP}/agents.sum beforehand, so the temp module graph has nothing to fall back on — tidy will drop the copied require and never populate the empty sum with fullsend's transitives. go test defaults to -mod=readonly, so the subsequent go test -tags behaviour -modfile=... (via make behaviour-test / BEHAVIOUR_GOFLAGS) is likely to fail with an "updates to go.mod needed" / missing go.sum entry error rather than compiling against the pinned checkout. This is the same defect flagged in the prior review and persists unchanged in this round.
  Remediation: Run go mod tidy -modfile="$modfile" -tags behaviour so tidy resolves the build-tagged import and writes a complete temp module graph.

• [logic-error] docs/behaviour-testing.md:24 — The documented local compile-only recipe copies the same tag-less tidy: go mod tidy -modfile="$modfile" followed by go test -mod=readonly -modfile="$modfile" -tags behaviour. Because suite_test.go is build-tagged behaviour, tag-less tidy will drop the fullsend require from the temporary modfile (and leave the truncated companion sumfile empty), so the subsequent -mod=readonly test is expected to fail with "updates to go.mod needed" rather than compiling. This is a distinct copy of the same bug above and needs its own one-line fix.
  Remediation: Change the documented command to go mod tidy -modfile="$modfile" -tags behaviour.

• [protected-path] .github/workflows/behaviour-tests.yml — This PR modifies files under protected paths that require human approval: .github/workflows/behaviour-tests.yml and scripts/run-behaviour-test-exec.sh (both match entries in the active protected-paths list: .github/, scripts/). The PR links to issue feat(behaviour): integrate behaviour test runner in fullsend-ai/agents #1242 and its description explains the rationale (adding the CI workflow and test-exec wrapper the issue's merge criteria call for). Human approval is always required for protected-path changes, regardless of context.

Low

• [architectural-coherence] .github/workflows/behaviour-tests.yml — Unchanged since the prior review: this workflow reuses functional-tests.yml's gate/checkout/authorize pattern (pull_request_target, base-only gate, head checkout only after authorization) but still has only two jobs (gate, behaviour) and omits the stable-named roll-up job (functional-tests-complete in the sibling workflow, added for issue functional-tests-complete goes red when a label is added to a PR whose tests already passed #954) that guards against a skipped job trivially satisfying branch protection. This is only a problem if the behaviour job is ever added to required branch-protection checks; nothing in the PR or docs currently proposes that, so it remains advisory rather than an active bug.
  Remediation: If this workflow is ever intended to gate merges, add a roll-up job mirroring functional-tests-complete/check-rollup-result.sh; otherwise document explicitly (e.g. in docs/behaviour-testing.md) that it is advisory-only and must not be added to required checks.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run

Review

Findings

Medium

• [logic-error] .github/workflows/behaviour-tests.yml:140 — In the "Resolve pinned fullsend source" step, go mod tidy -modfile="$modfile" still runs without -tags behaviour. The only Go file in this module, behaviour/suite_test.go, is guarded by //go:build behaviour, so a tag-less tidy does not see the github.com/fullsend-ai/fullsend import and will drop that require from the temporary modfile (the step also truncates ${RUNNER_TEMP}/agents.sum beforehand, so the temp module graph has nothing to fall back on). go.mod's declared Go version defaults to -mod=readonly, so the subsequent go test -tags behaviour -modfile=... step is likely to fail with an "updates to go.mod needed" error rather than compiling against the pinned checkout. This is the same defect flagged in the prior review at commit e8efba62bfba11fbfc249c1e9eb505c7e629cf45; the workflow changed (secrets were re-scoped) but this tidy invocation did not.
  Remediation: Run go mod tidy -modfile="$modfile" -tags behaviour so tidy resolves the build-tagged import correctly.

• [protected-path] .github/workflows/behaviour-tests.yml — This PR modifies files under protected paths that require human approval: .github/workflows/behaviour-tests.yml and scripts/run-behaviour-test-exec.sh (both match entries in the active protected-paths list: .github/, scripts/). The PR links to issue feat(behaviour): integrate behaviour test runner in fullsend-ai/agents #1242 and its description explains the rationale (adding the CI workflow and test-exec wrapper the issue's merge criteria call for). Human approval is always required for protected-path changes, regardless of context.

Low

• [architectural-coherence] .github/workflows/behaviour-tests.yml — Unchanged since the prior review: this workflow copies functional-tests.yml's gate/checkout/authorize pattern (pull_request_target, base-only gate, head checkout only after authorization) but still has only two jobs (gate, behaviour) and omits the stable-named roll-up job (functional-tests-complete in the sibling workflow). That roll-up exists because a skipped job still posts a check run that can trivially satisfy branch protection — see functional-tests.yml's inline reference to issue functional-tests-complete goes red when a label is added to a PR whose tests already passed #954, including the labeled-event case where gate is skipped for a non-ok-to-test label. This is only a problem if the behaviour job is ever added to required branch-protection checks; as written it would reproduce that hole.
  Remediation: If this workflow is intended to gate merges, add a roll-up job mirroring functional-tests-complete/check-rollup-result.sh; otherwise document that it is advisory-only and must not be added to required checks.

---

Fixed since the prior review (verified against the current PR head):

• The behaviour-test Makefile target now runs go test -count=1 -timeout 40m -tags "$(BEHAVIOUR_TEST_TAGS)" $(BEHAVIOUR_GOFLAGS) -exec "$(BEHAVIOUR_TEST_EXEC)" ./behaviour.
• The behaviour job's secrets (AGENTS_BT_* PEMs, PATs, Cloudflare credentials) are now scoped to only the final "Run behaviour tests" step instead of the whole job.
• docs/README.md's Guides table, README.md's Repository structure and Testing sections, and CONTRIBUTING.md's ok-to-test gate description have all been updated to cover the new behaviour-testing surface.
• The behaviour-test target and its BEHAVIOUR_TEST_* variables were relocated next to the other test targets in the Makefile.
• scripts/run-behaviour-test-exec.sh now has a descriptive header comment.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (2)

Review

Findings

Medium

• [logic-error] Makefile:46 — The behaviour-test target runs go test with neither -count=1 nor an explicit -timeout. The default Go test timeout is 10 minutes while the CI job budget (.github/workflows/behaviour-tests.yml, behaviour job) is 45 minutes, so a live run legitimately waiting on the dev environment can be killed with most of the job budget unused, and a genuine hang is indistinguishable from that. Without -count=1, a cached pass from an unrelated prior run is also possible.
  Remediation: Change the target to go test -count=1 -timeout 40m -tags "$(BEHAVIOUR_TEST_TAGS)" $(BEHAVIOUR_GOFLAGS) -exec "$(BEHAVIOUR_TEST_EXEC)" ./behaviour.

• [logic-error] .github/workflows/behaviour-tests.yml:170 — In the "Resolve pinned fullsend source" step, go mod tidy -modfile="$modfile" runs without -tags behaviour. The only Go file in this module, behaviour/suite_test.go, is guarded by //go:build behaviour, so a tag-less go mod tidy does not see the github.com/fullsend-ai/fullsend import. Since go.mod declares go 1.26.5, -mod=readonly applies by default, so the subsequent go test -tags behaviour step is likely to fail with an "updates to go.mod needed" error rather than silently succeeding.
  Remediation: Run go mod tidy -modfile="$modfile" -tags behaviour so tidy resolves the build-tagged import correctly.

• [secret-exposure] .github/workflows/behaviour-tests.yml:92 — The behaviour job's job-level env: block (lines 85-105) injects six GitHub App private keys (TEST_FULLSEND_PEM, TEST_TRIAGE_PEM, TEST_CODER_PEM, TEST_REVIEW_PEM, TEST_RETRO_PEM, TEST_PRIORITIZE_PEM), three actor PATs, a Cloudflare account id/API token, and GCP WIF identifiers into every step of the job — including checkout, setup-go, and "Resolve pinned fullsend source", none of which need these secrets. Only the final "Run behaviour tests" step (which executes make behaviour-test, compiling and running PR-controlled Go code under ./behaviour) actually needs them. The sibling functional-tests.yml scopes its secrets (EVAL_GH_TOKEN, GCP identifiers) to only the step that uses them. Widening the exposure window increases blast radius unnecessarily, even though these are described as isolated dev-environment credentials and the job is already gated by check-e2e-authorization.
  Remediation: Move the AGENTS_BT_* secret mappings from the job-level env: block to the env: block of only the final "Run behaviour tests" step; keep non-secret BEHAVIOUR_* configuration knobs at job level.

• [documentation-currency] docs/README.md:15 — docs/README.md's "Guides" table previously indexed every guide file under docs/ (network-policy.md was the only entry, and it exists). This PR adds docs/behaviour-testing.md but does not add a corresponding row, breaking that previously-exhaustive coverage of guide files.
  Remediation: Add | [Behaviour testing](behaviour-testing.md) | Live behaviour test suite for agents | under the Guides table.

• [protected-path] .github/workflows/behaviour-tests.yml — This PR modifies files under protected paths that require human approval: .github/workflows/behaviour-tests.yml and scripts/run-behaviour-test-exec.sh (both match entries in the active protected-paths list: .github/, scripts/). The PR links to issue feat(behaviour): integrate behaviour test runner in fullsend-ai/agents #1242 and its description explains the rationale (adding the CI workflow and test-exec wrapper the issue's merge criteria call for). Human approval is always required for protected-path changes, regardless of context.

Low

• [architectural-coherence] .github/workflows/behaviour-tests.yml — This workflow explicitly models itself on functional-tests.yml's gate/checkout/authorize pattern (per its own header comment) but omits the stable-named roll-up job (functional-tests-complete in the sibling workflow) that exists specifically because a skipped gate job still posts a check run that can trivially satisfy branch protection (see functional-tests.yml's inline reference to issue functional-tests-complete goes red when a label is added to a PR whose tests already passed #954). This is only a problem if the behaviour job is ever added to required branch-protection checks, but as written it would reproduce the exact hole functional-tests-complete goes red when a label is added to a PR whose tests already passed #954 fixed.
  Remediation: If this workflow is intended to gate merges, add a roll-up job mirroring functional-tests-complete; otherwise document that it is advisory-only and must not be added to required checks.

• [code-organization] Makefile:43 — The new behaviour-test target (lines 45-46) is inserted between the script-build target (line 43) and its prerequisite pattern rule scripts/%.sh: ... (line 48), splitting a cohesive script-bundling unit. The other test targets (script-test, test) are grouped together later in the file.
  Remediation: Move the BEHAVIOUR_TEST_* variables and the behaviour-test target next to the other test targets, keeping script-build and scripts/%.sh contiguous.

• [documentation-comment-format] scripts/run-behaviour-test-exec.sh:1 — The script has no top-level header comment. Other scripts in scripts/ consistently start with a descriptive header documenting purpose, invocation contract, and expected environment variables.
  Remediation: Add a header comment describing the script's role as a go test -exec wrapper and the FULLSEND_CHECKOUT behavior.

• [documentation-currency] README.md:24 — README.md's "Repository structure" block documents top-level product/test directories (including eval/, the functional eval harness) but does not list the new top-level behaviour/ directory this PR adds.
  Remediation: Add behaviour/ Live behaviour test suite (BDD feature specs and runner) to the Repository structure block.

• [documentation-currency] README.md:48 — README.md's "Testing" section documents make test/make script-test and make lint but does not mention the new make behaviour-test target, even though make help now lists it alongside the others.
  Remediation: Add a short paragraph and make behaviour-test example to the Testing section, linking to docs/behaviour-testing.md.

• [documentation-currency] CONTRIBUTING.md:61 — CONTRIBUTING.md describes the ok-to-test label gate only in terms of the Functional Tests workflow. The new .github/workflows/behaviour-tests.yml reuses the identical pull_request_target + check-e2e-authorization + ok-to-test gate, but CONTRIBUTING.md is not updated to mention it.
  Remediation: Update the paragraph to note that both Functional Tests and Behaviour Tests require the ok-to-test label for external contributors.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:03 PM UTC · Completed 12:20 PM UTC

Commit: 1593b59 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $4.46

[fullsend-ai-review]

Risk Assessment: elevated (3/5)

Details

Composite ≈2.77 rounds to 3 (elevated), consistent with the prior assessment: Tier 1 remains dominated by large blast radius, 2 protected paths (CI workflow + script), a new dependency-affecting go.mod/go.sum, and a low test-file ratio -- LINES_CHANGED moved from the prior 610/300-799 bucket to the current 806/800-1999 bucket, nudging the metadata sub-score up slightly but not enough to shift the rounded composite; Tier 2 shows moderate-to-high churn/author-diversity and fix-commit history on the shared docs/Makefile files touched, with no reverts or workaround-style commits; Tier 3 stays low because the PR scope tightly matches the linked issue and the change is additive and dev-gated, though acceptance-criteria coverage is only partial since the full make behaviour-test green run is blocked on an upstream fullsend API change.

Previous run

Risk Assessment: elevated (3/5)

Details

Large blast-radius PR (12 files, 610 lines) touching two protected paths (CI workflow, CONTRIBUTING.md) with a new dependency-affecting go.mod/go.sum and low test-file ratio, but the shared docs/config files it modifies show high recent churn and multi-author activity; issue linkage shows scope aligned with acceptance criteria and an additive, dev-gated, easily revertible design, yielding an elevated but not high composite score.

[waynesun09]

Review-only pass on the behaviour-test infrastructure, verified against the pinned fullsend commit c85577b87895 and reproduced locally with go1.26.5 where a command is quoted. 8 inline findings (2 high, 6 medium); nothing here duplicates an open or already-addressed thread. No approval or change request implied — flagging for your judgement.

The two high findings are the ones I would weigh before merge: the suite resolves agent content from agents@main rather than the PR head (so harness/**, scripts/**, schemas/** and policies/** are not actually under test at PR time, despite the path filter and the new CONTRIBUTING wording), and the committed module cannot be built or tidied outside the CI-generated modfile, which Renovate's gomod manager will hit with patch automerge enabled.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:20 AM UTC · Completed 10:39 AM UTC

Commit: 9fbcce0 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $3.55

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:03 AM UTC · Completed 7:14 AM UTC

Commit: 9fbcce0 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.21

[fullsend-ai-retro]

Retro on fullsend-ai/agents PR #1244 (adds an isolated dev 'behaviour' test suite + CI workflow, human-authored with Codex assistance after the fullsend code agent correctly self-declined since its sandbox can't edit .github/workflows/**). Pipeline health was otherwise good: issue #1242's triage/prioritize cycle worked as intended, no CI flakiness across 4 re-runs, and the fullsend review agent's first-round findings (job-level secret over-scoping, missing -count=1/-timeout, a schema path-filter gap, doc staleness) were all correctly fixed in round 2. Two gaps stood out. (1) The review agent re-asserted an identical go mod tidy -tags behaviour finding unchanged across all 3 review rounds despite the PR author disputing it twice with a specific empirical rebuttal; a third, independent human reviewer (waynesun09) later confirmed the dismissal was correct, i.e. the persisted finding was very likely a false positive. This is a strong corroborating data point for already-open issue #1331 ('review agent should re-verify a disputed finding against ground truth before repeating it a 3rd+ round') — same failure mode (PR #126), now seen a second time with a second, independent human confirming the disputed finding was wrong. Not filed as a new proposal; recommend attaching this PR as evidence to #1331. (2) Neither qodo nor the fullsend review agent (3 full passes) caught two structural issues a single human reviewer found by manually diffing the new workflow against fullsend's own e2e.yml template and tracing runtime behavior into the fullsend dependency's source: the new test suite validated agent content (harness/scripts/schemas/policies) from a fixed agents@main ref rather than the PR's own head for most of its declared gated paths, and the workflow was missing several safeguards present in its stated template (cancel-in-progress guard, path-relevance filtering, unused hard-required secrets, a durable-resource leak). Both were fixed in the PR's final commit, so no residual risk, but the automated reviewers had 3 opportunities to catch them and did not. Filed two proposals below extending the correctness sub-agent's checklist to close this specific class of gap.

Proposals filed

• Correctness sub-agent should catch new CI/test gates that validate a stale/default ref instead of the PR's own code (in fullsend-ai/agents)
• Review agent should diff a new workflow against an explicitly-referenced sibling/template workflow to catch dropped safeguards (in fullsend-ai/agents)