Skip to content

feat(dispatch): harness CEL dispatch for custom agents (#2889) - #3820

Merged
ifireball merged 25 commits into
fullsend-ai:mainfrom
ifireball:feat/2889-harness-dispatch
Jul 14, 2026
Merged

feat(dispatch): harness CEL dispatch for custom agents (#2889)#3820
ifireball merged 25 commits into
fullsend-ai:mainfrom
ifireball:feat/2889-harness-dispatch

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

  • Add fullsend dispatch CLI with NormalizedEvent types, ADR 0054 auth gate, kill switch, and harness trigger: CEL evaluation
  • Add parallel harness-dispatch / harness-run jobs to reusable-dispatch.yml (bash route job unchanged)
  • Add behaviour tests for issue/PR CEL filtering, wrong-label negatives, and dummy-runtime wiring validation (assert_env, assert_file, assert_json)

Related Issue

Closes #2889

Changes

  • internal/normevent/ — forge-neutral event types + golden tests from schema fixtures
  • internal/harnessdispatch/ — dispatch core, gha-event/json input drivers, gha-matrix output
  • internal/harnesstrigger: field with CEL compile/lint
  • internal/forgeGetCollaboratorPermission for actor role enrichment
  • e2e/behaviour/features/dispatch/ — five scenarios (issue label, wrong label, PR-on-issue negative, PR label, wrong PR label)

Testing

  • go test on changed packages
  • go vet on changed packages
  • make behaviour-test (CI)

Checklist

  • Bash route job untouched — parallel harness path only
  • Deny semantics: unauthorized / kill switch / no CEL match → empty matrix, exit 0
  • harness-run skips validate-enrollment (per-repo installs)

Made with Cursor

…2889)

Introduce fullsend dispatch with NormalizedEvent types, harness trigger
CEL evaluation, parallel harness-dispatch/harness-run jobs in
reusable-dispatch (bash route unchanged), behaviour tests, and dummy
runtime wiring validation ops.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ifireball
ifireball requested a review from a team as a code owner July 9, 2026 08:18
@ifireball ifireball self-assigned this Jul 9, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:19 AM UTC · Completed 8:36 AM UTC
Commit: 634a65b · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add CEL-based harness dispatch CLI and GitHub Actions harness-run pipeline

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

Grey Divider

AI Description

• Add fullsend dispatch to evaluate harness trigger: CEL and emit a GHA matrix
• Introduce NormalizedEvent mapping + ADR-0054 auth gate and per-repo kill switch
• Add reusable workflow jobs + behaviour tests validating dispatch positives/negatives
Diagram

graph TD
  A["reusable-dispatch.yml"] --> B["harness-dispatch job"] --> C["fullsend dispatch CLI"] --> D["Input driver (gha-event/json)"] --> E[("NormalizedEvent")]
  E --> F["Auth gate + kill switch + CEL match"] --> G["GHA matrix JSON"] --> H["harness-run job"]
  H --> I["fullsend run (agent)"] --> J[("event-payload.json")]
  C --> K[(".fullsend config")]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. GitHub Actions `if:` / label-based routing in workflow YAML
  • ➕ No new CLI surface area or Go packages
  • ➕ Routing stays close to workflow execution
  • ➖ Harder to express rich conditions and reuse across forges
  • ➖ Authorization and kill-switch logic becomes fragmented across workflows
  • ➖ Limited testability vs unit-tested dispatch core
2. Precompile triggers at harness load-time and cache programs
  • ➕ Reduces per-dispatch CEL compile cost when many harnesses exist
  • ➕ Makes trigger validation and execution share a single compiled artifact
  • ➖ Adds complexity (cache invalidation, program lifecycle)
  • ➖ Premature optimization for a vertical-slice implementation
3. Use JSONPath/JQ-like expressions instead of CEL
  • ➕ Smaller dependency footprint than CEL in some ecosystems
  • ➕ Familiar to many users for JSON filtering
  • ➖ Weaker typing/validation guarantees (boolean output enforcement is harder)
  • ➖ Less expressive for future logic (functions, richer semantics)

Recommendation: Keep the PR’s approach (NormalizedEvent + CEL + explicit dispatch stage). It cleanly separates event normalization, authorization gating (ADR-0054), kill-switch handling, and trigger evaluation, while producing a stable matrix contract for workflow fan-out. Consider trigger program caching later if dispatch time becomes an issue with large harness inventories.

Files changed (42) +2340 / -0

Enhancement (18) +1274 / -0
dispatch.goIntroduce 'fullsend dispatch' command +140/-0

Introduce 'fullsend dispatch' command

• Adds a new CLI command that loads a NormalizedEvent (GHA webhook or JSON), runs dispatch matching against '.fullsend', and outputs either a GitHub Actions matrix object or JSON refs.

internal/cli/dispatch.go

root.goRegister dispatch command in CLI root +1/-0

Register dispatch command in CLI root

• Wires 'dispatch' into the main CLI command tree so it is available as 'fullsend dispatch'.

internal/cli/root.go

forge.goExpose collaborator permission API in forge client interface +5/-0

Expose collaborator permission API in forge client interface

• Adds 'GetCollaboratorPermission' to the forge client interface to support ADR-0054 permission enrichment for dispatch authorization decisions.

internal/forge/forge.go

github.goImplement collaborator permission lookup via GitHub API +19/-0

Implement collaborator permission lookup via GitHub API

• Implements 'GetCollaboratorPermission' using '/repos/{owner}/{repo}/collaborators/{user}/permission' and maps 'role_name' into the dispatch authorization pipeline.

internal/forge/github/github.go

harness.goAdd harness 'trigger:' CEL field and validation +4/-0

Add harness 'trigger:' CEL field and validation

• Adds a 'Trigger' field to harness schema and validates it during harness validation using the new CEL trigger compiler.

internal/harness/harness.go

lint.goLint trigger expression for early feedback +10/-0

Lint trigger expression for early feedback

• Adds lint diagnostics for invalid trigger expressions so harness authors get actionable errors without running dispatch.

internal/harness/lint.go

trigger.goAdd CEL trigger compile and evaluation utilities +72/-0

Add CEL trigger compile and evaluation utilities

• Introduces CEL environment creation, trigger compilation/type-checking (must return bool), and evaluation against a map-form event structure.

internal/harness/trigger.go

auth.goAdd ADR-0054 authorization gate +10/-0

Add ADR-0054 authorization gate

• Adds a simple authorization function that requires write-level actor role before dispatch can proceed.

internal/harnessdispatch/auth.go

core.goImplement dispatch orchestration (kill switch + auth + match) +63/-0

Implement dispatch orchestration (kill switch + auth + match)

• Adds the main dispatch flow: validate inputs, check kill switch, enforce authorization, enumerate triggered harnesses from config, evaluate CEL triggers, and project matches into execution refs.

internal/harnessdispatch/core.go

enumerate.goEnumerate triggered harnesses and evaluate matches +94/-0

Enumerate triggered harnesses and evaluate matches

• Loads agent entries from config, resolves harness paths, filters to harnesses with non-empty triggers, and evaluates triggers against event.ToMap() to produce the matched set.

internal/harnessdispatch/enumerate.go

ghaevent.goMap GitHub Actions webhook payloads to NormalizedEvent +372/-0

Map GitHub Actions webhook payloads to NormalizedEvent

• Implements an input driver that reads 'GITHUB_EVENT_PATH' JSON and maps issues/PRs/issue_comment events into NormalizedEvent, including label transitions, comment command extraction, and collaborator permission enrichment when a forge client is available.

internal/harnessdispatch/input/ghaevent.go

json.goAdd JSON input driver for NormalizedEvent +30/-0

Add JSON input driver for NormalizedEvent

• Adds an input driver that reads a NormalizedEvent from a file or stdin and parses/validates it.

internal/harnessdispatch/input/json.go

killswitch.goImplement per-config kill switch check for dispatch +31/-0

Implement per-config kill switch check for dispatch

• Reads 'kill_switch' from config and treats missing config as kill-switch disabled; includes a fallback parse path for org config format to simplify tests.

internal/harnessdispatch/killswitch.go

ghamatrix.goAdd GitHub Actions matrix output writer +20/-0

Add GitHub Actions matrix output writer

• Encodes execution refs into a stable '{include: [...]}' matrix JSON object, normalizing nil refs to an empty list.

internal/harnessdispatch/output/ghamatrix.go

json.goAdd JSON output writer for execution refs +15/-0

Add JSON output writer for execution refs

• Provides an indented JSON output mode primarily for debugging or non-GHA consumers.

internal/harnessdispatch/output/json.go

project.goProject matched harnesses into execution refs and payloads +105/-0

Project matched harnesses into execution refs and payloads

• Builds execution refs consumed by 'harness-run', including event type, status routing, and a minimal 'event_payload' containing issue/PR context and optional trigger source for comment/review-driven dispatch.

internal/harnessdispatch/project.go

ref.goDefine execution ref contract for dispatch matrix +13/-0

Define execution ref contract for dispatch matrix

• Defines the 'ExecutionRef' struct that becomes the matrix element consumed by the harness-run workflow and downstream run invocation.

internal/harnessdispatch/ref.go

event.goIntroduce forge-neutral NormalizedEvent schema + helpers +270/-0

Introduce forge-neutral NormalizedEvent schema + helpers

• Adds a validated NormalizedEvent model with entity/transition/actor/state/source structures, JSON parsing, map conversion for CEL, and permission mapping helpers used by dispatch and authorization logic.

internal/normevent/event.go

Tests (18) +780 / -0
driver.goExtend CI driver interface for harness dispatch assertions +2/-0

Extend CI driver interface for harness dispatch assertions

• Adds methods to wait for a harness-run completion for a specific agent and to assert that no harness artifact was produced after a trigger time.

e2e/behaviour/drivers/ci/driver.go

githubactions.goImplement harness-run polling and negative artifact checks +88/-0

Implement harness-run polling and negative artifact checks

• Implements 'WaitForHarnessAgent' by polling recent workflow runs for a matching harness-run name and success conclusion. Implements 'AssertNoHarnessAgentArtifact' to ensure no 'fullsend-{agent}' artifacts appear on successful runs or as repository artifacts after the trigger.

e2e/behaviour/drivers/ci/githubactions/githubactions.go

driver.goAdd SCM driver primitives for PR creation and file reads +4/-0

Add SCM driver primitives for PR creation and file reads

• Extends the SCM driver interface to fetch repository file contents, create branches, commit to a branch, and open a change proposal (PR) for dispatch scenarios.

e2e/behaviour/drivers/scm/driver.go

github.goWire new SCM driver methods to GitHub client +16/-0

Wire new SCM driver methods to GitHub client

• Implements the new SCM driver methods by delegating to the underlying GitHub forge client for file content, branching, committing, and PR creation.

e2e/behaviour/drivers/scm/github/github.go

dispatch.featureAdd behaviour scenarios for CEL-based dispatch +85/-0

Add behaviour scenarios for CEL-based dispatch

• Adds feature scenarios validating that issue/PR label transitions trigger the correct harness and that wrong-label or wrong-entity cases do not trigger a run. Uses dummy runtime assertions to validate environment variables and event payload wiring.

e2e/behaviour/features/dispatch/dispatch.feature

ok.jsonAdd dispatch behaviour fixture output marker +1/-0

Add dispatch behaviour fixture output marker

• Adds a simple JSON fixture used by dummy runtime to prove the harness agent executed in behaviour tests.

e2e/behaviour/fixtures/dispatch/ok.json

artifacts.goAvoid re-fetching artifacts when already present +3/-0

Avoid re-fetching artifacts when already present

• Short-circuits artifact download when 'World.ArtifactDir' is already set, supporting dispatch scenarios that manage artifacts separately.

e2e/behaviour/steps/artifacts.go

dispatch.goAdd step definitions for dispatch-driven harness runs +157/-0

Add step definitions for dispatch-driven harness runs

• Introduces steps to commit a custom harness (with 'trigger:'), register it in config, open and label PRs, and assert harness-run success or non-execution. Downloads the agent artifact from the harness-run workflow run for verification.

e2e/behaviour/steps/dispatch.go

registry.goRegister dispatch behaviour steps +1/-0

Register dispatch behaviour steps

• Adds dispatch step registration so the new dispatch feature scenarios are executed by the behaviour test suite.

e2e/behaviour/steps/registry.go

suite_test.goReset dispatch scenario state between runs +2/-0

Reset dispatch scenario state between runs

• Extends scenario initialization to clear PR-related and dispatch-agent fields in the shared world state.

e2e/behaviour/suite_test.go

world.goAdd dispatch-related fields to test world state +3/-0

Add dispatch-related fields to test world state

• Adds 'DispatchAgent' and 'PRNumber' to maintain scenario context for dispatch behaviour tests.

e2e/behaviour/world/world.go

github_test.goAdd unit tests for collaborator permission lookup +27/-0

Add unit tests for collaborator permission lookup

• Adds tests for the success case and the not-found case, ensuring errors are recognized as forge not-found when appropriate.

internal/forge/github/github_test.go

trigger_test.goTest trigger validation and evaluation +37/-0

Test trigger validation and evaluation

• Adds unit tests covering empty/invalid expressions, non-bool outputs, and a basic match/non-match evaluation against example event maps.

internal/harness/trigger_test.go

core_test.goAdd dispatch core tests for deny and match semantics +110/-0

Add dispatch core tests for deny and match semantics

• Adds tests for kill switch denial, authorization denial, issue label match, and ensuring issue-only harness does not match PR events. Also validates execution ref projection for issue payload shape.

internal/harnessdispatch/core_test.go

ghaevent_test.goTest GHA event mapping for labeled issue and opened PR +90/-0

Test GHA event mapping for labeled issue and opened PR

• Adds tests verifying issues labeled mapping (including role enrichment via fake collaborator permissions) and PR opened mapping (including change proposal fields).

internal/harnessdispatch/input/ghaevent_test.go

ghamatrix_test.goTest matrix output encoding +26/-0

Test matrix output encoding

• Adds unit tests covering empty matrix output and the presence of agent names for non-empty refs.

internal/harnessdispatch/output/ghamatrix_test.go

event_test.goAdd golden-style parsing tests against schema examples +68/-0

Add golden-style parsing tests against schema examples

• Parses all example JSON fixtures (expecting the path traversal fixture to fail), tests write-authorization logic and GitHub permission mapping, and verifies ToMap round-trip structure.

internal/normevent/event_test.go

dummy_test.goTest new dummy runtime assertion operations +60/-0

Test new dummy runtime assertion operations

• Adds unit tests for 'assert_env', 'assert_file', and 'assert_json', covering success and bad-args cases to ensure predictable failure modes in behaviour tests.

internal/runtime/dummy_test.go

Documentation (1) +10 / -0
runtimes.mdDocument dummy runtime assertion operations +10/-0

Document dummy runtime assertion operations

• Adds documentation for 'assert_env', 'assert_file', and 'assert_json' operations used by dispatch behaviour tests to validate wiring inside the sandbox.

docs/runtimes.md

Other (5) +276 / -0
reusable-dispatch.ymlAdd harness-dispatch and harness-run jobs using dispatch matrix +187/-0

Add harness-dispatch and harness-run jobs using dispatch matrix

• Introduces a new 'harness-dispatch' job that runs 'fullsend dispatch' and exports a matrix output. Adds a parallel 'harness-run' job that fans out runs per agent, writes event payload to disk, mints tokens, and launches the harness agent with dispatch context env.

.github/workflows/reusable-dispatch.yml

go.modAdd CEL dependency for trigger evaluation +6/-0

Add CEL dependency for trigger evaluation

• Adds 'github.com/google/cel-go' and related indirect dependencies to support compiling and evaluating harness trigger expressions.

go.mod

go.sumRecord CEL and transitive dependency checksums +11/-0

Record CEL and transitive dependency checksums

• Adds checksums for CEL and its transitive dependencies introduced for trigger compilation/evaluation.

go.sum

fake.goAdd fake collaborator permission lookup for tests +20/-0

Add fake collaborator permission lookup for tests

• Extends the fake forge client with a 'CollaboratorPermissions' map and implements 'GetCollaboratorPermission' for dispatch/auth unit tests.

internal/forge/fake.go

dummy.goAdd dummy runtime assertions for dispatch behaviour tests +52/-0

Add dummy runtime assertions for dispatch behaviour tests

• Adds behaviour-test-only operations to assert environment variables, file presence, and JSON field presence inside the sandbox, enabling validation of dispatch wiring without real agent logic.

internal/runtime/dummy.go

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Site preview

Preview: https://b9015090-site.fullsend-ai.workers.dev

Commit: b22abfa05542de00128f2805c5428efa21bf2c49

@qodo-code-review

qodo-code-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider


Action required

1. Scaffold dispatch.yml not synced ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
.github/workflows/reusable-dispatch.yml adds new harness-dispatch/harness-run logic, but the
scaffolded dispatcher workflow internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml is
not updated to match. This breaks the required sync between the two dispatch workflows for
routing/payload/secret-threading behavior and can cause divergent production behavior between
scaffolded repos and the reusable workflow.
Code

.github/workflows/reusable-dispatch.yml[R564-615]

+  harness-dispatch:
+    name: Harness dispatch
+    runs-on: ${{ inputs.runner_image }}
+    permissions:
+      contents: read
+      pull-requests: read
+    outputs:
+      matrix: ${{ steps.dispatch.outputs.matrix }}
+    steps:
+      - name: Checkout caller repository
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.base.sha || github.sha }}
+          persist-credentials: false
+          allow-unsafe-pr-checkout: ${{ github.event_name == 'pull_request_target' }}
+          sparse-checkout: |
+            .fullsend/
+          sparse-checkout-cone-mode: false
+
+      - name: Run fullsend dispatch
+        id: dispatch
+        env:
+          GH_TOKEN: ${{ github.token }}
+          GITHUB_EVENT_NAME: ${{ github.event_name }}
+          EVENT_ACTION: ${{ inputs.event_action }}
+        run: |
+          set -euo pipefail
+          FULLSEND=""
+          if [[ -f ".fullsend/bin/fullsend" ]]; then
+            FULLSEND=".fullsend/bin/fullsend"
+          elif [[ -f "bin/fullsend" ]]; then
+            FULLSEND="bin/fullsend"
+          else
+            echo "::error::vendored fullsend binary not found"
+            exit 1
+          fi
+          chmod +x "${FULLSEND}"
+          MATRIX=$("${FULLSEND}" dispatch \
+            --input-driver gha-event \
+            --output-driver gha-matrix \
+            --config-dir .fullsend \
+            --event-action "${EVENT_ACTION}")
+          echo "matrix=${MATRIX}" >> "${GITHUB_OUTPUT}"
+
+  harness-run:
+    name: Harness run (${{ matrix.agent }})
+    needs: harness-dispatch
+    if: ${{ needs.harness-dispatch.outputs.matrix != '' && fromJSON(needs.harness-dispatch.outputs.matrix).include[0] != null }}
+    strategy:
+      fail-fast: false
+      matrix: ${{ fromJSON(needs.harness-dispatch.outputs.matrix) }}
+    concurrency:
Relevance

⭐⭐⭐ High

Team repeatedly enforces reusable vs scaffold dispatch parity; changes applied to both in prior PRs.

PR-#1688
PR-#2168
PR-#2781

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062045 requires the reusable dispatch workflow and the scaffold dispatch workflow
to keep their dispatch payload/routing/secret-threading logic in sync unless divergence is
explicitly documented inline. The PR introduces new jobs harness-dispatch and harness-run in
.github/workflows/reusable-dispatch.yml, while the scaffolded
internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml contains no corresponding
harness-dispatch logic, creating an unsynced divergence.

Rule 1062045: Keep jq payload, stage routing, and secret threading logic in dispatch workflows in sync
.github/workflows/reusable-dispatch.yml[564-615]
internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[19-60]

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 reusable dispatch workflow gained a new dispatch path (`harness-dispatch` + `harness-run`), but the scaffold dispatcher workflow was not updated accordingly. Compliance requires these two dispatch workflows to remain in sync for payload construction, routing conditions, and input/secret threading (or to explicitly document intentional divergence inline).

## Issue Context
The PR adds the new harness dispatch vertical slice via `.github/workflows/reusable-dispatch.yml`, but `internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml` still only implements the legacy stage-routing dispatcher.

## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[564-615]
- internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[19-60]

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


2. Dispatch binary not checked out ✓ Resolved 🐞 Bug ≡ Correctness
Description
The harness-dispatch job sparse-checks out only .fullsend/, but per-org installs place the
vendored binary at bin/fullsend, so fullsend dispatch can fail with “vendored fullsend binary
not found”. This breaks harness dispatch for a supported install layout.
Code

.github/workflows/reusable-dispatch.yml[R579-596]

+          sparse-checkout: |
+            .fullsend/
+          sparse-checkout-cone-mode: false
+
+      - name: Run fullsend dispatch
+        id: dispatch
+        env:
+          GH_TOKEN: ${{ github.token }}
+          GITHUB_EVENT_NAME: ${{ github.event_name }}
+          EVENT_ACTION: ${{ inputs.event_action }}
+        run: |
+          set -euo pipefail
+          FULLSEND=""
+          if [[ -f ".fullsend/bin/fullsend" ]]; then
+            FULLSEND=".fullsend/bin/fullsend"
+          elif [[ -f "bin/fullsend" ]]; then
+            FULLSEND="bin/fullsend"
+          else
Relevance

⭐⭐ Medium

No direct history on sparse-checkout needing bin/fullsend; install/vendoring changes exist but not
this exact issue.

PR-#1954

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow restricts checkout to .fullsend/ but still attempts to run bin/fullsend; the repo’s
action documentation states per-org vendoring stores the binary at bin/fullsend, so it won’t be
present under the current sparse checkout.

.github/workflows/reusable-dispatch.yml[564-606]
action.yml[79-88]

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

## Issue description
`harness-dispatch` uses sparse checkout limited to `.fullsend/`, but the dispatch step also supports `bin/fullsend` (per-org vendoring). With the current sparse checkout, `bin/fullsend` will never be present, causing the job to error for per-org installs.

## Issue Context
`action.yml` explicitly documents that per-org mode stores the vendored binary at `bin/fullsend` in the config repo.

## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[564-606]
- action.yml[79-88]

## What to change
- Update the `Checkout caller repository` sparse-checkout to also include `bin/fullsend` (or `bin/`).
- Alternatively, make sparse-checkout conditional on `inputs.install_mode` (include `bin/` for `per-org`).
- Keep `persist-credentials: false` and other security settings unchanged.

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


3. PR comment link never set ✓ Resolved 🐞 Bug ≡ Correctness
Description
mapIssueCommentEvent tries to read issue.pull_request.number, so it usually fails to populate
entity.linked_change_proposal for PR issue_comment events. This prevents the adapter from
producing the documented NormalizedEvent shape needed for /fs-fix and other PR-on-issue_comment
routing/projection.
Code

internal/harnessdispatch/input/ghaevent.go[R216-223]

+	if pr := nestedMap(issue, "pull_request"); pr != nil {
+		num := intField(pr, "number")
+		if num > 0 {
+			ev.Entity.LinkedChangeProposal = &normevent.LinkedChangeProposal{
+				ID:  num,
+				URL: stringField(issue, "html_url"),
+			}
+		}
Relevance

⭐⭐ Medium

No historical evidence for harnessdispatch issue_comment PR linking; new subsystem so acceptance
unclear.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The adapter code only sets linked_change_proposal when it can read a number field from
issue.pull_request, but the repo’s own v1 example/spec shows linked_change_proposal must be set
for PR issue comments and that adapters must fill webhook gaps via API calls.

internal/harnessdispatch/input/ghaevent.go[207-224]
docs/normative/normalized-event/v1/examples/fs-fix-comment.json[2-10]
docs/normative/normalized-event/v1/README.md[56-62]
docs/normative/normalized-event/v1/README.md[233-247]

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 GitHub `issue_comment` webhook’s `issue.pull_request` object is not a full PR object and does not reliably contain a PR number. The current code reads `issue.pull_request.number`, so `entity.linked_change_proposal` is typically left unset, diverging from the repo’s own NormalizedEvent v1 examples/spec.

## Issue Context
The normative example `fs-fix-comment.json` shows `linked_change_proposal` populated for `issue_comment` events on PRs, and the README specifies that adapters should fill webhook gaps via API calls and emit the correct linked PR fields.

## Fix Focus Areas
- internal/harnessdispatch/input/ghaevent.go[207-224]
- docs/normative/normalized-event/v1/examples/fs-fix-comment.json[2-10]
- docs/normative/normalized-event/v1/README.md[56-62]
- docs/normative/normalized-event/v1/README.md[233-247]

## What to change
- Populate `entity.linked_change_proposal` for PR issue comments using reliable data:
 - Use `issue.number` as the PR number when `issue.pull_request` is present (GitHub PRs share the issue number), and derive the PR HTML URL (e.g., replace `/issues/` with `/pull/` in `issue.html_url`, or construct from repo+number).
- To meet the README contract for `issue_comment` on PRs, also populate `state.change_proposal` by fetching PR details via GitHub API before returning the event (this may require adding a forge client method for PR details and implementing it in the live+fake clients).
- Add unit coverage for this PR `issue_comment` mapping (including linked_change_proposal + change_proposal fields).

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



Remediation recommended

4. assert_env shell injection ✓ Resolved 🐞 Bug ⛨ Security
Description
Dummy runtime’s assert_env interpolates an unvalidated variable name into a shell command (`test
-n "${...}"), so a crafted op.Args` can change the command executed in the sandbox. Dummy runtime
is selectable via config, so this is not purely a compile-time-only test helper.
Code

internal/runtime/dummy.go[R252-258]

+	case "assert_env":
+		varName := strings.TrimSpace(op.Args)
+		if varName == "" {
+			return fmt.Errorf("assert_env requires a variable name")
+		}
+		cmd := fmt.Sprintf("test -n \"${%s}\"", varName)
+		_, stderr, exitCode, err := rt.execFn()(sandboxName, cmd, 30*time.Second)
Relevance

⭐⭐⭐ High

Security hardening in dummy/runtime has been accepted recently; likely to fix shell-injection style
issues.

PR-#764
PR-#1982

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The command string includes varName directly inside shell syntax, with no validation or quoting,
and DummyRuntime is available through runtime resolution.

internal/runtime/dummy.go[252-265]
internal/runtime/registry.go[10-20]

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

## Issue description
`assert_env` builds a shell command by embedding `op.Args` inside `${...}` without validation/escaping. This enables shell injection if `op.Args` contains characters like `}` or `"`.

## Issue Context
Even though DummyRuntime is intended for behaviour tests, it is still selectable via config (`runtime: dummy`). Treat behaviour script inputs as potentially untrusted.

## Fix Focus Areas
- internal/runtime/dummy.go[252-265]
- internal/runtime/registry.go[16-20]

## What to change
- Enforce a strict variable-name regex (e.g. `^[A-Za-z_][A-Za-z0-9_]*$`) and error if it doesn’t match.
- Avoid shell parameter expansion entirely where possible; e.g., run `printenv -- <var>` with proper shell quoting and check for non-empty output.
- Add a unit test that `assert_env` rejects invalid var names containing quotes/braces.

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


5. Negative harness check false-pass ✓ Resolved 🐞 Bug ☼ Reliability
Description
AssertNoHarnessAgentArtifact ignores harness-run workflows that exist but are not
completed/success, so a harness that ran and failed/cancelled can be reported as “did not run”.
This can hide real dispatch filtering regressions in behaviour tests.
Code

e2e/behaviour/drivers/ci/githubactions/githubactions.go[R484-494]

+	for _, run := range runs {
+		runTime, parseErr := time.Parse(time.RFC3339, run.CreatedAt)
+		if parseErr != nil || runTime.Before(after) {
+			continue
+		}
+		if run.Name != want {
+			continue
+		}
+		if run.Status != "completed" || run.Conclusion != "success" {
+			continue
+		}
Relevance

⭐⭐ Medium

No historical evidence for negative harness assertions treating failed/cancelled runs as “ran”; new
behaviour CI code.

PR-#1982

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function explicitly continues when the run is not completed/success, so failed or cancelled
harness runs are ignored and treated as absence.

e2e/behaviour/drivers/ci/githubactions/githubactions.go[476-520]

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 negative assertion only checks artifacts for runs that are `completed` and `success`. If a harness run is created after the trigger time but fails/cancels (or uploads no artifacts), the test passes even though the harness did run.

## Issue Context
The step text is “agent did not run”, which semantically should fail if a matching `Harness run (<agent>)` workflow exists at all after the scenario start.

## Fix Focus Areas
- e2e/behaviour/drivers/ci/githubactions/githubactions.go[476-520]

## What to change
- Change `AssertNoHarnessAgentArtifact` to fail as soon as it detects any workflow run with `run.Name == "Harness run (<agent>)"` created after `after` (regardless of conclusion).
- Optionally keep the artifact checks as extra signal, but the primary condition should be presence of the run.

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


Grey Divider

Qodo Logo

Comment thread .github/workflows/reusable-dispatch.yml
Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread internal/harnessdispatch/input/ghaevent.go Outdated
Comment thread e2e/behaviour/drivers/ci/githubactions/githubactions.go Outdated
Comment thread internal/runtime/dummy.go
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [false-negative] .github/workflows/reusable-dispatch.yml — The trigger pre-check heuristic added in commit 097a130 greps for '^[[:space:]]*-[[:space:]]*name:|^[[:space:]]*name:' to detect registered agents, but AgentEntry.Name uses yaml:"name,omitempty" (internal/config/config.go:24). When local-path agents are registered without --name (e.g., fullsend agent add harness/custom.yaml --fullsend-dir .fullsend), the marshalled YAML contains only source: harness/custom.yaml with no name: field. If no URL-based agents are present (i.e., check 1 — source:[[:space:]]*https?:// — also fails), the pre-check sets has_triggers=false, and the entire harness-dispatch job is skipped: no CLI install, no dispatch, no agent execution. The false-negative scenario is limited to repos with only local-path agents and no explicit names (scaffolded installs use URL agents caught by check 1), but it contradicts the commit's stated intent of avoiding false negatives. Fix: drop the name: grep and keep only the agents: key check (false positives are acceptable for this optimization gate), or also match source: and the string-shorthand list-item pattern. Anchored at medium from prior review — code unchanged.

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit (000a6c5) corrects the actions/cache pin SHA from an incorrect hash to the verified v4.2.3 tag commit (5a3ec84eff668545956fd18022155c47e93e2684). The overall workflow changes add parallel harness-dispatch/harness-run jobs, trigger detection with early-exit, CLI binary caching via actions/cache, a separate checkout for the upstream install action, and a new composite action for CLI installation. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect the SHA pin correction commit.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. Anchored at low from prior review — code unchanged.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review — code unchanged.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review — code unchanged.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review — code unchanged.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.


Compared to prior review (SHA 097a130): One new commit (000a6c5). Commit 000a6c5 corrects the actions/cache pin SHAs from an incorrect hash (5a3ec84e3dbd861d4c4b439f47a3d2b4b9f0c5e0) to the verified v4.2.3 tag commit (5a3ec84eff668545956fd18022155c47e93e2684). Both actions/cache/restore and actions/cache/save references are updated. No prior findings resolved. No new findings. Protected-path finding updated to note the SHA pin correction.

Previous run

Review

Findings

Medium

  • [false-negative] .github/workflows/reusable-dispatch.yml — The trigger pre-check heuristic added in commit 097a130 greps for '^[[:space:]]*-[[:space:]]*name:|^[[:space:]]*name:' to detect registered agents, but AgentEntry.Name uses yaml:"name,omitempty" (internal/config/config.go:24). When local-path agents are registered without --name (e.g., fullsend agent add harness/custom.yaml --fullsend-dir .fullsend), the marshalled YAML contains only source: harness/custom.yaml with no name: field. If no URL-based agents are present (i.e., check 1 — source:[[:space:]]*https?:// — also fails), the pre-check sets has_triggers=false, and the entire harness-dispatch job is skipped: no CLI install, no dispatch, no agent execution. The false-negative scenario is limited to repos with only local-path agents and no explicit names (scaffolded installs use URL agents caught by check 1), but it contradicts the commit's stated intent of avoiding false negatives. Fix: drop the name: grep and keep only the agents: key check (false positives are acceptable for this optimization gate), or also match source: and the string-shorthand list-item pattern.

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit (097a130) simplifies the trigger pre-check in reusable-dispatch.yml: it replaces the file-by-file while loop (which parsed source: values and grepped individual harness files for trigger:) with a heuristic that checks for agents: and name: keys in config.yaml. The overall workflow changes add parallel harness-dispatch/harness-run jobs, trigger detection with early-exit, CLI binary caching via actions/cache, a separate checkout for the upstream install action, and a new composite action for CLI installation. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect the simplified trigger pre-check commit.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. Anchored at low from prior review — code unchanged.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review — code unchanged.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review — code unchanged.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review — code unchanged.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.

Previous run

Review

Findings

Medium

  • [false-negative] .github/workflows/reusable-dispatch.yml — The trigger pre-check heuristic added in commit 097a130 greps for '^[[:space:]]*-[[:space:]]*name:|^[[:space:]]*name:' to detect registered agents, but AgentEntry.Name uses yaml:"name,omitempty" (internal/config/config.go:24). When local-path agents are registered without --name (e.g., fullsend agent add harness/custom.yaml --fullsend-dir .fullsend), the marshalled YAML contains only source: harness/custom.yaml with no name: field. If no URL-based agents are present (i.e., check 1 — source:[[:space:]]*https?:// — also fails), the pre-check sets has_triggers=false, and the entire harness-dispatch job is skipped: no CLI install, no dispatch, no agent execution. The false-negative scenario is limited to repos with only local-path agents and no explicit names (scaffolded installs use URL agents caught by check 1), but it contradicts the commit's stated intent of avoiding false negatives. Fix: drop the name: grep and keep only the agents: key check (false positives are acceptable for this optimization gate), or also match source: and the string-shorthand list-item pattern.

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit (097a130) simplifies the trigger pre-check in reusable-dispatch.yml: it replaces the file-by-file while loop (which parsed source: values and grepped individual harness files for trigger:) with a heuristic that checks for agents: and name: keys in config.yaml. The overall workflow changes add parallel harness-dispatch/harness-run jobs, trigger detection with early-exit, CLI binary caching via actions/cache, a separate checkout for the upstream install action, and a new composite action for CLI installation. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect the simplified trigger pre-check commit.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. Anchored at low from prior review — code unchanged.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review — code unchanged.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review — code unchanged.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review — code unchanged.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.


Compared to prior review (SHA 904d122): One new commit (097a130). Commit 097a130 simplifies the harness trigger pre-check: it replaces the while IFS= read -r src loop (which parsed source: values from config.yaml, resolved file paths, and grepped each harness file for ^trigger:) with a two-grep heuristic that checks for agents: and name: keys. The intent is to avoid false negatives from harness path layout, but the heuristic introduces a narrower class of false negatives for local-path agents without explicit names (see new [false-negative] finding). No prior findings resolved. One new medium finding.

Previous run (2)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit (904d122) replaces a for ... in $(...) loop with a shellcheck-compliant while IFS= read -r loop in the harness trigger pre-check step, adding POSIX whitespace trimming and empty-line guards. The overall workflow changes add parallel harness-dispatch/harness-run jobs, trigger detection with early-exit, CLI binary caching via actions/cache, a separate checkout for the upstream install action, and a new composite action for CLI installation. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect shellcheck compliance commit.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. Anchored at low from prior review — code unchanged.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review — code unchanged.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review — code unchanged.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review — code unchanged.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.

Previous run (3)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit (904d122) replaces a for ... in $(...) loop with a shellcheck-compliant while IFS= read -r loop in the harness trigger pre-check step, adding POSIX whitespace trimming and empty-line guards. The overall workflow changes add parallel harness-dispatch/harness-run jobs, trigger detection with early-exit, CLI binary caching via actions/cache, a separate checkout for the upstream install action, and a new composite action for CLI installation. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect shellcheck compliance commit.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. Anchored at low from prior review — code unchanged.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review — code unchanged.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review — code unchanged.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review — code unchanged.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.


Compared to prior review (SHA cbf57dd): One new commit (904d122). Commit 904d122 is a shellcheck compliance fix in the harness trigger pre-check step of reusable-dispatch.yml. It replaces a for src in $(grep ...) loop (SC2044/SC2086-prone) with a while IFS= read -r src loop reading from a process substitution, adds POSIX parameter-expansion whitespace trimming, adds an empty-line guard, and removes the now-unnecessary shopt -s nullglob. The new pattern correctly handles filenames with spaces and glob characters that the old for loop would have misinterpreted. No prior findings resolved. No new findings. Protected-path finding updated to note the shellcheck commit.

Previous run (4)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit adds a trigger detection heuristic (grep-based check for harness triggers to skip CLI installation when unnecessary), CLI binary caching via actions/cache, a separate checkout for the upstream install action (.workflow-actions/ path), and early-exit logic in the dispatch step for repos without triggers. These additions expand the workflow surface in reusable-dispatch.yml. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect trigger detection, caching, and upstream checkout additions.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent now builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. All other event handlers (mapIssuesEvent, mapPREvent, mapPRReviewEvent) continue to reuse the caller-provided event. New finding for commit cbf57dd.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.

Previous run (5)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The latest commit adds a trigger detection heuristic (grep-based check for harness triggers to skip CLI installation when unnecessary), CLI binary caching via actions/cache, a separate checkout for the upstream install action (.workflow-actions/ path), and early-exit logic in the dispatch step for repos without triggers. These additions expand the workflow surface in reusable-dispatch.yml. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect trigger detection, caching, and upstream checkout additions.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot and only changes_requested reviews. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the merge of upstream/main removed the paragraph previously added by this PR. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — code unchanged.
    Remediation: Restore documentation of the Go-level IsAuthorized bot bypass in ADR 0054 (or add equivalent text to ADR 0061) and its rationale relative to the bash routing's narrower check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern.

  • [redundant-api-call] internal/harnessdispatch/input/ghaevent.go — The refactored mapIssueCommentEvent now builds its own normevent.Event from scratch instead of reusing the pre-populated event from mapGitHubWebhook. As a result, mapGitHubWebhook performs a collaborator permission lookup (via GetCollaboratorPermission) that is immediately discarded, and mapIssueCommentEvent makes the same API call again. For issue_comment events, this produces two identical HTTP calls to the GitHub API. All other event handlers (mapIssuesEvent, mapPREvent, mapPRReviewEvent) continue to reuse the caller-provided event. New finding for commit cbf57dd.
    Remediation: Either have mapIssueCommentEvent accept the pre-built event (like the other map functions), or skip the permission lookup in mapGitHubWebhook when the event name is issue_comment.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. This pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.


Compared to prior review (SHA 417e1ec): One new commit (cbf57dd). Key changes: (1) reusable-dispatch.yml adds trigger detection to skip CLI install when no harness triggers exist, adds CLI binary caching via actions/cache, adds separate checkout for upstream install action at .workflow-actions/ path, and adds early-exit in the dispatch step for repos without triggers; (2) enumerate.go changes ListTriggeredHarnesses from hard-fail to log-and-skip when individual agent resolution or loading fails — improving resilience so one misconfigured agent does not block all dispatch, with updated test assertions; (3) ghaevent.go refactors mapIssueCommentEvent to build its own event from scratch (adding comment edited/deleted transition support) and extracts ComputeChangeProposalIsFork for fail-closed fork detection when head/base repo metadata is missing; (4) orgconfig.go fixes a latent bug where a duplicate if dirCfg.IsOrg block caused requireFullsendConfig to return org configs without applying default allowed remote resources — verified by new TestRequireFullsendConfig_OrgGetsDefaultAllowlist; (5) new tests cover comment edited/deleted transitions, ghost fork fail-closed behavior, clearDirContents inode preservation, and org config default allowlist. No prior findings resolved. One new low finding: redundant API call in the refactored issue comment mapping path.

Previous run (6)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The changes add parallel harness-dispatch/harness-run jobs to reusable-dispatch.yml, introduce a new composite action for CLI installation, extend the e2e behaviour path filter and grep pattern in e2e.yml, and harden the issue_url output with a randomized heredoc delimiter. The latest commit splits the install step into vendored (.defaults/.github/actions/install-fullsend-cli) and upstream (./.github/actions/install-fullsend-cli) paths, and adds .defaults/.github/actions/install-fullsend-cli/ to the sparse checkout. Human approval is required for protected-path changes regardless of review outcome. Unchanged from prior review.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot (${ORG_NAME}-review[bot]) and only changes_requested reviews. Note: The merge of upstream/main (commit 417e1ec) removed the paragraph previously added to ADR 0054 documenting this bypass as intentional design. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the event table at line 125 describes pull_request_review.submitted as "Already gated (requires review-bot authorship)" which accurately describes the bash routing's specific-bot check but not the Go code's broader any-ActorBot bypass. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface, which is why severity remains anchored at low. Anchored at low from prior review — code unchanged, but documenting ADR paragraph removed in merge resolution.
    Remediation: Restore the removed ADR 0054 paragraph (or add equivalent text to ADR 0061) documenting the Go-level IsAuthorized bot bypass, its rationale, and how it relates to the bash routing's narrower REVIEW_USER_LOGIN check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern. Harness authors writing CEL triggers for review_submitted events should include bot identity checks (e.g., event.actor.id == "org-review[bot]") when the trigger should only match the org's own review bot.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. Note: this pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.

Previous run (7)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The changes add parallel harness-dispatch/harness-run jobs to reusable-dispatch.yml, introduce a new composite action for CLI installation, extend the e2e behaviour path filter and grep pattern in e2e.yml, and harden the issue_url output with a randomized heredoc delimiter. The latest commit splits the install step into vendored (.defaults/.github/actions/install-fullsend-cli) and upstream (./.github/actions/install-fullsend-cli) paths, and adds .defaults/.github/actions/install-fullsend-cli/ to the sparse checkout. Human approval is required for protected-path changes regardless of review outcome. Unchanged from prior review.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot (${ORG_NAME}-review[bot]) and only changes_requested reviews. Note: The merge of upstream/main (commit 417e1ec) removed the paragraph previously added to ADR 0054 documenting this bypass as intentional design. ADR 0054 on main no longer contains documentation of the Go-level bot bypass — the event table at line 125 describes pull_request_review.submitted as "Already gated (requires review-bot authorship)" which accurately describes the bash routing's specific-bot check but not the Go code's broader any-ActorBot bypass. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface, which is why severity remains anchored at low. Anchored at low from prior review — code unchanged, but documenting ADR paragraph removed in merge resolution.
    Remediation: Restore the removed ADR 0054 paragraph (or add equivalent text to ADR 0061) documenting the Go-level IsAuthorized bot bypass, its rationale, and how it relates to the bash routing's narrower REVIEW_USER_LOGIN check. Alternatively, tighten the Go bypass to verify bot identity matches the org's review bot pattern. Harness authors writing CEL triggers for review_submitted events should include bot identity checks (e.g., event.actor.id == "org-review[bot]") when the trigger should only match the org's own review bot.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. Note: this pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.


Compared to prior review (SHA 88211b5): One new commit (417e1ec). Commit 417e1ec merges upstream/main into the feature branch. The merge introduces no code changes to PR-scoped files — all Go source, workflow files, and test files introduced by this PR are unmodified. The merge resolved conflicts by dropping the PR's additions to ADR 0054 (CEL harness dispatch authorization documentation) and ADR 0047 (harness-dispatch install behavior paragraph); these ADR files are no longer changed in the PR's final diff. The merge also brought in upstream documentation changes (roadmap rewrite, new docs, glossary additions, OTEL tracing docs, repos CLI docs) and dependency updates that are not PR-scoped. Security review confirms: OTEL secret passthrough added by upstream to reusable-dispatch.yml is properly handled via GitHub Actions secret masking; checkout ref safety verified (uses base.sha for pull_request_target). No prior findings resolved. Authorization-bypass finding updated to note the removed ADR 0054 documentation.

Previous run (8)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The changes add parallel harness-dispatch/harness-run jobs to reusable-dispatch.yml, introduce a new composite action for CLI installation, extend the e2e behaviour path filter and grep pattern in e2e.yml, and harden the issue_url output with a randomized heredoc delimiter. The latest commit splits the install step into vendored (.defaults/.github/actions/install-fullsend-cli) and upstream (./.github/actions/install-fullsend-cli) paths, and adds .defaults/.github/actions/install-fullsend-cli/ to the sparse checkout. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect vendored/upstream install split.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot (${ORG_NAME}-review[bot]) and only changes_requested reviews. However, ADR 0054 (as updated by this PR) now explicitly documents this as intentional design: the Go authorization gate stays agent-generic, and CEL trigger expressions are the designated layer for enforcing stage-specific constraints. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — documented as intentional design in updated ADR 0054.
    Remediation: Harness authors writing CEL triggers for review_submitted events should include bot identity checks (e.g., event.actor.id == "org-review[bot]") when the trigger should only match the org's own review bot.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. Note: this pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.

Previous run (9)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The changes add parallel harness-dispatch/harness-run jobs to reusable-dispatch.yml, introduce a new composite action for CLI installation, extend the e2e behaviour path filter and grep pattern in e2e.yml, and harden the issue_url output with a randomized heredoc delimiter. The latest commit splits the install step into vendored (.defaults/.github/actions/install-fullsend-cli) and upstream (./.github/actions/install-fullsend-cli) paths, and adds .defaults/.github/actions/install-fullsend-cli/ to the sparse checkout. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect vendored/upstream install split.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot (${ORG_NAME}-review[bot]) and only changes_requested reviews. However, ADR 0054 (as updated by this PR) now explicitly documents this as intentional design: the Go authorization gate stays agent-generic, and CEL trigger expressions are the designated layer for enforcing stage-specific constraints. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Anchored at low from prior review — documented as intentional design in updated ADR 0054.
    Remediation: Harness authors writing CEL triggers for review_submitted events should include bot identity checks (e.g., event.actor.id == "org-review[bot]") when the trigger should only match the org's own review bot.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. Note: this pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Anchored at low from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Anchored at low from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.


Compared to prior review (SHA 68ac8a2): One new commit (88211b5). Commit 88211b5 splits the single harness-dispatch install step into two conditional paths — vendored mode now resolves the composite action from .defaults/.github/actions/install-fullsend-cli (the vendored defaults copy in the caller repo), while upstream mode continues to use ./.github/actions/install-fullsend-cli (from the workflow repository). This fixes a correctness issue where vendored installs would fail to find the composite action at the workflow repo path. The sparse checkout is updated to include .defaults/.github/actions/install-fullsend-cli/. Security review confirms the checkout ref safety (base branch SHA for pull_request_target), no path traversal risk (hardcoded literal path), and no new secrets exposure (github_token already ambient). No prior findings resolved. Protected-path finding updated to reflect the install split.

Previous run (10)

Review

Resolved from prior review

[duplicate-identifier] .github/actions/install-fullsend-cli/action.yml — The duplicate id: install was split into id: install-vendored (line 38) and id: install-upstream (line 87), with the output combining both via ${{ steps.install-vendored.outputs.fullsend-path || steps.install-upstream.outputs.fullsend-path }}. High-severity finding resolved.

[injection-vuln] .github/actions/install-fullsend-cli/action.yml — The ::error:: workflow command interpolating ${VENDORED} was replaced with echo "vendored binary not found" >&2, eliminating the workflow command injection surface.

[injection-vuln] internal/runtime/dummy.go — The assert_json jq path is now validated against jsonPathPattern (^[a-zA-Z_][a-zA-Z0-9_]*(\.[a-zA-Z_][a-zA-Z0-9_]*)*$) before constructing the jq command. A test (TestExecuteBehaviourOp_AssertJSONInvalidPath) verifies that paths containing shell metacharacters are rejected.

[authorization-policy-coherence] docs/ADRs/0054-require-authorization-on-all-agent-dispatch-paths.md — ADR 0054 now includes an explicit paragraph documenting the CEL harness dispatch authorization design: the Go gate stays agent-generic, GitHub installation bots bypass the write-role check on review_submitted and label-added events, and CEL trigger expressions enforce stage-specific constraints.

[behavioral-change] internal/cli/agent.goloadAgentConfig was refactored to use config.LoadFromDirparseConfigDataIsPerRepoYAML, the same classification logic used by tryLoadFullsendConfig. Both paths now consistently classify configs with dispatch: (even with empty platform) as org config. A new test in internal/config/load_test.go confirms this classification.

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/actions/install-fullsend-cli/action.yml, .github/workflows/e2e.yml — This PR modifies files under the .github/ protected path. The changes add parallel harness-dispatch/harness-run jobs to reusable-dispatch.yml, introduce a new composite action for CLI installation, extend the e2e behaviour path filter and grep pattern in e2e.yml, and harden the issue_url output with a randomized heredoc delimiter. Human approval is required for protected-path changes regardless of review outcome. Updated from prior review to reflect heredoc hardening and e2e filter extension.

Low

  • [authorization-bypass] internal/harnessdispatch/auth.go, line 26 — The bot review_submitted authorization bypass accepts any GitHub bot submitting any review state (approved, changes_requested, commented, dismissed). The existing bash dispatch restricts this to only the org's own review bot (${ORG_NAME}-review[bot]) and only changes_requested reviews. However, ADR 0054 (as updated by this PR) now explicitly documents this as intentional design: the Go authorization gate stays agent-generic, and CEL trigger expressions are the designated layer for enforcing stage-specific constraints. Installing a third-party GitHub App on a repository requires admin access, limiting the attack surface. Downgraded from medium — documented as intentional design in updated ADR 0054.
    Remediation: Harness authors writing CEL triggers for review_submitted events should include bot identity checks (e.g., event.actor.id == "org-review[bot]") when the trigger should only match the org's own review bot.

  • [test-integrity] internal/harnessdispatch/input/ghaevent_test.goTestLoadGHAEvent_IssueComment uses comment body /fs-fix please repair which should produce Instruction: "please repair" via extractCommentCommand. The test asserts Command ("/fs-fix") and LinkedChangeProposal but does not assert Instruction. This field's extraction logic has no test coverage in this file. Anchored at low from prior review.
    Remediation: Add an Instruction assertion alongside the existing Command and LinkedChangeProposal assertions.

  • [error-handling-idiom] internal/config/load.goparseConfigData performs triple YAML unmarshal: (1) into interface{} for syntax validation, (2) in IsPerRepoYAML into map[string]interface{} for key probing, (3) in ParsePerRepoConfig/ParseOrgConfig for actual parsing. The initial probe is redundant because IsPerRepoYAML already returns false on YAML syntax errors. Note: this pattern is inherited from the pre-existing tryLoadFullsendConfig in orgconfig.go; the PR extracts it without refactoring it. Unchanged from prior review.
    Remediation: Remove the initial var probe interface{} unmarshal and let IsPerRepoYAML + the typed parser handle error reporting.

  • [incomplete-doc] docs/guides/dev/cli-internals.md — The new fullsend dispatch command (added via cmd.AddCommand(newDispatchCmd()) in root.go) is not documented in the CLI Command Tree section. The Key Source Files Reference section also does not include the new dispatch-related implementation files (internal/cli/dispatch.go, internal/harnessdispatch/, internal/normevent/). Unchanged from prior review.
    Remediation: Add a dispatch entry to the CLI Command Tree with its flags and add entries to the Key Source Files Reference table.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added component/dispatch Workflow dispatch and triggers component/harness Agent harness, config, and skills loading component/e2e End-to-end tests go Pull requests that update go code labels Jul 9, 2026
Add unit tests for projection, enumerate, killswitch, gha-event adapter,
JSON drivers, and CLI dispatch command.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:45 AM UTC · Completed 8:59 AM UTC
Commit: 0367564 · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review July 9, 2026 08:59

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 9, 2026
@ifireball

Copy link
Copy Markdown
Member Author

Re: qodo review item #1 (scaffold dispatch.yml sync): intentional for slice 1 per issue #2889 / epic #2888 — parallel harness-dispatch path lands in reusable-dispatch.yml only; scaffold dispatcher sync is tracked for cutover in #2902.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:08 AM UTC · Ended 9:10 AM UTC
Commit: e8381e3 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:13 AM UTC · Ended 9:16 AM UTC
Commit: e8381e3 · View workflow run →

@ifireball
ifireball force-pushed the feat/2889-harness-dispatch branch 2 times, most recently from 99ab566 to 3a5b95c Compare July 9, 2026 09:15
…end-ai#2889)

- Sparse-checkout bin/fullsend for per-org harness-dispatch
- Fix issue_comment PR linked_change_proposal and enrich change_proposal via forge
- Harden assert_env against shell injection; tighten negative harness CI assertion
- Extend behaviour path filter for harnessdispatch/normevent packages

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ifireball
ifireball force-pushed the feat/2889-harness-dispatch branch from 3a5b95c to 47a417c Compare July 9, 2026 09:15
Resolve conflicts in go.mod, orgconfig, and github tests; use
GitHubExtensions for collaborator permission lookup; document shared
auth rationale in code comments instead of ADR edits.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:07 AM UTC · Completed 11:19 AM UTC
Commit: 417e1ec · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review pass on 417e1ec. Findings below are deduplicated against the existing review threads — items already fixed (cd9f3a5, 574ca83, 47a417c) or explicitly decided in earlier threads (label-added bypass scope, fork gating living in harness CEL, no role-enablement gate) are not repeated. Four remaining unique findings at medium+; the orgconfig.go one looks like an unintentional refactor artifact and is the most clear-cut.

Comment thread internal/cli/orgconfig.go Outdated
Comment thread internal/harnessdispatch/input/ghaevent.go Outdated
Comment thread .github/workflows/reusable-dispatch.yml
Comment thread .github/workflows/reusable-dispatch.yml

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Squad — 4 new findings (deduplicated against prior rounds)

Ran a fresh 4-agent pass (claude-coder, claude-researcher, gemini-code-review, cursor-code-review). Cross-checked all findings against this PR's existing review history before posting — most of what the agents surfaced independently duplicates already-resolved or already-declared-intentional threads (label-added auth scoping, the review_submitted bot bypass covered by ADR 0054, the e2e deny-semantics gap, and the orgconfig.go dead-code finding already posted in the last round). Only posting what wasn't already raised:

  • HIGH — upstream sparse-checkout doesn't check out the install action it references (.github/workflows/reusable-dispatch.yml)
  • MEDIUM — one malformed harness aborts dispatch for every agent (internal/harnessdispatch/enumerate.go)
  • MEDIUM — custom GitHub roles silently map to none (internal/normevent/event.go)
  • MEDIUM — custom role: values untested against token-minting/WIF scope (internal/harnessdispatch/project.go)

See inline comments for details.

Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread internal/harnessdispatch/enumerate.go Outdated
Comment thread internal/normevent/event.go
Comment thread internal/harnessdispatch/project.go

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One non-blocking note inline.

Comment thread internal/harnessdispatch/input/ghaevent.go Outdated
Merge upstream/main and resolve review threads: orgconfig dead branch,
fail-closed fork detection, comment_edited/deleted transitions,
per-harness enumerate tolerance, upstream install-action checkout,
harness-trigger pre-check with CLI cache, and custom-role docs.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:02 PM UTC · Completed 10:12 PM UTC
Commit: cbf57dd · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 10:16 PM UTC · Ended 10:27 PM UTC
Commit: bd28f41 · View workflow run →

Use while-read instead of for-in word splitting when scanning agent
source paths in reusable-dispatch harness-dispatch job.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:28 PM UTC · Completed 10:36 PM UTC
Commit: 904d122 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Treat any registered config agents (or URL sources) as having triggers
instead of parsing harness files in bash, which skipped dispatch for
behaviour test repos.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:09 PM UTC · Completed 11:20 PM UTC
Commit: 097a130 · View workflow run →

Use the full v4.2.3 commit SHA so harness-dispatch can resolve
actions/cache restore/save in behaviour and production runs.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:04 AM UTC · Completed 12:12 AM UTC
Commit: 000a6c5 · View workflow run →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks component/dispatch Workflow dispatch and triggers component/e2e End-to-end tests component/harness Agent harness, config, and skills loading component/install CLI install and app setup go Pull requests that update go code ok-to-test Allow e2e CI to run after maintainer review (must be re-applied after each push) requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(dispatch): add harness-dispatch job with ADR 54 auth, kill switch, and custom-agent behaviour test

3 participants