test(#3733): add fork PR bash-routing smoke scenario

[fullsend-ai-coder]

Summary

Add one behaviour scenario (dummy runtime) that exercises fork PR bash routing on per-repo installs as a time-boxed guard until harness CEL cutover (#2902).

Related Issue

Part of #3733 — interim bash-routing fork smoke until harness cutover.

Changes

• New feature file e2e/behaviour/features/dispatch/fork-bash-routing.feature with one scenario:
  • Opens a fork PR against the enrolled per-repo pool test repo
  • Labels the fork PR ready-for-review to trigger pull_request_target labeled event
  • Asserts the bash router dispatches the review stage end-to-end via the fullsend-review artifact
  • Verifies the dummy agent's write_fixture operation succeeded
• New step definition the fullsend workflow dispatches the review stage in dispatch.go — polls for the fullsend-review artifact to confirm the bash router dispatched the review stage
• Unit test for the new step's precondition check

Testing

• go build ./... passes
• go vet ./... passes
• go test ./pkg/behaviourtest/... passes (all existing + new test)
• Pre-commit could not run (sandbox network restriction); post-script handles this authoritatively
• golangci-lint not available in sandbox; deferred to CI

---

Closes #3733

Post-script verification

• Branch is not main/master (agent/3733-fork-bash-routing-smoke)
• Secret scan passed (gitleaks — 0b0263ab84ceefc47fb8263e7b92a64d8278e9e4..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:53 PM UTC · Completed 9:06 PM UTC
Commit: 9a33ac8 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Low

• [code duplication / maintenance risk] pkg/behaviourtest/steps/dispatch.go:259 — thenFullsendWorkflowDispatchesReview is a near-exact copy of thenHarnessWorkflowCompletes with the agent hardcoded to "review". The body could delegate: return thenHarnessWorkflowCompletes(w, "review"). The separate zero-parameter function is intentional for Gherkin readability, but the body need not be duplicated.

---

Labels: PR adds e2e behaviour test for fork PR dispatch routing

[ifireball]

/fs-fix

Remove the new near-duplicate step the fullsend workflow dispatches the review stage / thenFullsendWorkflowDispatchesReview (and its unit test). In e2e/behaviour/features/dispatch/fork-bash-routing.feature, use the existing step instead:

Then the harness "review" workflow completes successfully

That already calls thenHarnessWorkflowCompletes(w, "review") and is behaviorally identical (waits for fullsend-review, requires success, downloads artifacts). Keep the scenario intent; only drop the redundant Go step and wire the feature to the existing harness-completes phrasing.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 8:02 AM UTC · Completed 8:08 AM UTC
Commit: 9a33ac8 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Removed the near-duplicate step thenFullsendWorkflowDispatchesReview and its unit test. The feature file now uses the existing harness review workflow completes successfully step, which is behaviorally identical.

Fixed (3):

1. Remove near-duplicate step and reuse existing harness step (e2e/behaviour/features/dispatch/fork-bash-routing.feature): Replaced 'Then the fullsend workflow dispatches the review stage' with 'Then the harness "review" workflow completes successfully' which calls the existing thenHarnessWorkflowCompletes function
2. Remove thenFullsendWorkflowDispatchesReview step registration (pkg/behaviourtest/steps/dispatch.go): Removed the step registration for 'the fullsend workflow dispatches the review stage' from registerDispatchSteps and deleted the thenFullsendWorkflowDispatchesReview function, which was a near-duplicate of thenHarnessWorkflowCompletes with 'review' hardcoded
3. Remove unit test for deleted function (pkg/behaviourtest/steps/dispatch_test.go): Removed TestFullsendWorkflowDispatchesReview_RequiresScenarioStart test since the function it tested no longer exists; the equivalent precondition check is already covered by TestDispatchSteps_RequireScenarioStart

Tests: passed

_{Updated by fullsend fix agent}

[ifireball]

/fs-fix

Root cause (BT failure)

Bash routing does dispatch the built-in review stage. The Review job fails harness validation because the dummy agent never writes output/agent-result.json:

✗ Validation failed: FAIL: output/agent-result.json not found
Error: validation failed after 2 iteration(s)

Evidence: https://github.com/halfsend-05/test-repo-05/actions/runs/30344149877
(Route success → Review runs dummy → writes bash-routing-ok.json only → schema validation fails.)

Fix

Mirror the triage behaviour pattern: the dummy agent must emit a schema-valid review agent-result.json.

1. Add something like e2e/behaviour/fixtures/review/comment.json that validates against internal/scaffold/fullsend-repo/schemas/review-result.schema.json. Minimal comment (or approve) shape is fine, e.g.:

{
  "action": "comment",
  "pr_number": 1,
  "repo": "owner/repo",
  "head_sha": "0000000",
  "body": "Behaviour smoke: bash-routed review completed."
}

2. Update e2e/behaviour/features/dispatch/fork-bash-routing.feature so the dummy agent table writes that fixture to output/agent-result.json (keep the existing bash-routing-ok.json proof row if useful, or assert success on the agent-result write). Example:

Given a dummy agent that would:
  | description          | op            | args                                                       |
  | Prove bash routing   | write_fixture | output/bash-routing-ok.json, fixtures/dispatch/ok.json     |
  | Emit review JSON     | write_fixture | output/agent-result.json, fixtures/review/comment.json     |

3. Ensure the chosen action won’t break the review post-script under dummy runtime (prefer comment if approve/request-changes mutate too aggressively). If post-script needs real pr_number/repo/head_sha, either use placeholders the post-script overrides from env, or document/adjust so validation + post-script both succeed.

Goal: BT scenario passes end-to-end because review validation (and post-script) succeed after bash routing dispatches review.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 9:06 AM UTC · Completed 9:13 AM UTC
Commit: 773f2b3 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Added a schema-valid review comment fixture (e2e/behaviour/fixtures/review/comment.json) and updated the fork bash-routing feature to write output/agent-result.json via write_fixture, fixing the harness validation failure.

Fixed (1):

1. Dummy agent missing agent-result.json for review harness validation: Added fixtures/review/comment.json with a schema-valid review comment result and a write_fixture row in the fork-bash-routing feature to emit output/agent-result.json, mirroring the triage scenario pattern. Also added an assertion step to verify the fixture write succeeds.

Tests: passed

Decision points

• Chose comment action over approve or request-changes for the review fixture (alternatives: approve, request-changes; rationale: The human instruction recommended comment as the safest action that won't mutate PR state aggressively under the dummy runtime)

_{Updated by fullsend fix agent}

[ifireball]

/fs-fix

Root cause (latest BT failure)

Schema validation now passes. Review post-script fails:

Error: head SHA must be a 40 or 64 character hex string, got "0000000"
##[error]fullsend post-review failed with exit code 1 (PR fullsend-ai/fullsend#3 in halfsend-11/test-repo-08)

Evidence: https://github.com/halfsend-11/test-repo-08/actions/runs/30345793151

e2e/behaviour/fixtures/review/comment.json has "head_sha": "0000000" — enough for review-result.schema.json (minLength: 7) but fullsend post-review requires a full 40- or 64-char hex SHA (internal/cli/postreview.go).

Fix

Update e2e/behaviour/fixtures/review/comment.json:

1. Set head_sha to a 40-character hex string (same shape as schema/unit fixtures), e.g. abcdef0123456789abcdef0123456789abcdef01.
2. Keep action: "comment" (post-script uses env PR_NUMBER / REPO_FULL_NAME; placeholders for pr_number/repo are OK if they still satisfy the schema).
3. Do not invent dynamic SHA wiring unless needed — a static valid-length hex is enough to get past the CLI check for this smoke.

Goal: Review job completes successfully after bash routing so the behaviour scenario passes.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 10:09 AM UTC · Completed 10:15 AM UTC
Commit: 0218400 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 3 (human-triggered)

Fixed head_sha in review fixture from 7-char to 40-char hex string to satisfy the post-review CLI validation regex. The fixture now passes both schema validation and the CLI's hexSHARe check.

Fixed (1):

1. head_sha too short for post-review CLI (e2e/behaviour/fixtures/review/comment.json): Replaced 7-char head_sha "0000000" with 40-char hex string "abcdef0123456789abcdef0123456789abcdef01". The schema (minLength: 7) accepted the short value but the CLI regex in postreview.go requires exactly 40 or 64 hex chars.

Tests: passed

_{Updated by fullsend fix agent}

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 11:34 AM UTC · Completed 11:48 AM UTC
Commit: 47066a6 · View workflow run →

[fullsend-ai-retro]

Retro: PR #5654 -- fork PR bash-routing smoke scenario

Workflow: Issue #3733 -> code agent (run 30303294460) -> PR #5654 -> review agent approved with 1 low finding -> 3 human-driven /fs-fix iterations -> human approval -> merged.

Rework: 3 fix iterations on a 27-line change. All three fixes were diagnosed and directed by the human reviewer (ifireball), not discovered by the agents:

1. Fix 1 (code duplication): Code agent created a near-duplicate of thenHarnessWorkflowCompletes. The review agent caught this as a low finding, but the human had to trigger /fs-fix to resolve it.
2. Fix 2 (missing agent-result.json): The dummy agent in the BT scenario didn't emit output/agent-result.json, causing harness validation failure. Discovered during BT run, not during review.
3. Fix 3 (head_sha too short): The review fixture used a 7-char SHA placeholder. The JSON schema (minLength: 7) accepted it, but the CLI (fullsend post-review) requires exactly 40 or 64 hex chars. Discovered during BT run.

Review agent performance: The review agent correctly identified the code duplication issue (fix 1). It could not reasonably have caught fixes 2-3 without deep knowledge of the BT infrastructure and the schema/CLI validation mismatch.

Evidence for existing open issues:

• #2414 (code agent should study sibling files): The code agent duplicated an existing step function and missed the agent-result.json fixture pattern already used by triage BT scenarios in the same directory.
• #1305 (code agent: prefer extending existing functions over duplicating): Same evidence -- code agent created thenFullsendWorkflowDispatchesReview instead of calling thenHarnessWorkflowCompletes(w, "review").

2 proposals filed below. Both target fullsend-ai/fullsend.

Proposals filed

• Tighten head_sha validation in review-result.schema.json to match CLI requirements (in fullsend-ai/fullsend)
• Add fixture authoring checklist to behaviour-testing.md (in fullsend-ai/fullsend)