feat(#6339): replace jira-poll HarnessRouter with CEL trigger evaluation - #6340
Conversation
Replace the hardcoded HarnessRouter in the Jira poll input driver with CEL trigger evaluation from the harness dispatch system (ADR 0061). The HarnessRouter pre-determined stages (triage, code, review, etc.) and the downstream shell glue required per-stage workflow files with `# fullsend-stage: <stage>` markers. This broke repos using the workflow_call shim mode (the default scaffold from `fullsend github setup`), which has a single fullsend.yaml → reusable-dispatch.yml with no per-stage files. Changes: - Define EventMatcher interface in jirapoll package with Match(ctx, *normevent.Event) → []DispatchRecord signature - Replace dispatch.EventRouter dependency with EventMatcher in Poller - Convert JiraEvent → normevent.Event (typed enums) instead of dispatch.NormalizedEvent (plain strings) for CEL evaluation - Add celMatcher in CLI layer that loads harness CEL triggers once per cycle and evaluates events via harnessdispatch.MatchHarnesses - Output DispatchRecord (execution refs) instead of poll.Dispatch (stage-based), compatible with the workflow_call shim - Add routerMatcher adapter in behaviour tests for backward compat The HarnessRouter and buildRouter remain for the GitLab poll path. Closes #6339
|
🤖 Finished Review · ✅ Success · Started 7:50 PM UTC · Completed 8:19 PM UTC Commit: |
ReviewFindingsMedium
Low
Labels: PR modifies dispatch routing code (internal/jirapoll, internal/cli/poll.go) and user-facing Jira integration documentation Previous runReviewFindingsHigh
Medium
Low
Next steps:
|
|
/fs-fix address unresolved findings |
|
/fs-fix address unresolved findings and fix CI |
|
🤖 Finished Fix · ✅ Success · Started 8:30 PM UTC · Completed 8:44 PM UTC Commit: |
|
Filed #6343 to track the [architectural-divergence] finding (GitLab poll → CEL trigger migration). |
- [stale-doc] Update jira-integration.md: replace removed field names (stage, resource_key, iid, event_payload_b64) with new DispatchRecord fields (agent, event_type, event_payload, source_repo, status_number, status_repo, role, trigger_source) - [logic-error] Restore per-issue filtering in thenDispatchNotContains and thenDispatchContains by parsing entity.key from EventPayload - [authorization-bypass-in-test-adapter] Add IsAuthorized gate to routerMatcher.Match() to match production celMatcher behaviour - [error-handling-idiom] Change unconventional "projecting" verb to "build execution ref for" in celMatcher error message - [naming-convention] Rename abbreviated toNormEvent back to toNormalizedEvent for consistency with codebase conventions Addresses review feedback on #6340
🔧 Fix agent — iteration 1 (human-triggered)Addressed 6 of 8 review findings with code fixes. Fixed the CI-failing logic error in thenDispatchNotContains (restored per-issue key filtering), updated stale documentation with new DispatchRecord field names, added IsAuthorized gate to test adapter, fixed error message idiom, and renamed abbreviated function. Disagreed with 2 findings: architectural-divergence (follow-up issue filing is out of scope) and poller_test payload assertions (removed assertions tested a field that no longer exists). Fixed (6):
Disagreed (2):
Tests: passedNext steps:
|
Site previewPreview: https://3863f55d-site.fullsend-ai.workers.dev Commit: |
|
🤖 Review · Commit: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 8:59 PM UTC · Completed 9:15 PM UTC Commit: |
Superseded by updated review
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
pkg/behaviourtest/steps/jirapoll.go:73: [medium] test-integrity
routerMatcher adapter marshals a dispatch.NormalizedEvent as the EventPayload (containing entity.key), but the production celMatcher calls harnessdispatch.ProjectExecutionRef → buildEventPayload, which produces {"issue":{"number":N,"html_url":"..."}} with no top-level entity key. dispatchMatchesIssue (line 285) parses entity.key from EventPayload — this works against the test adapter format but would silently return false for production-format payloads. The behaviour tests exercise per-issue assertions against a payload schema that production never produces.
Suggested fix: Make the routerMatcher produce EventPayload in the same format as the production celMatcher (call harnessdispatch.ProjectExecutionRef or buildEventPayload), and adjust dispatchMatchesIssue to use a field that exists in the production payload format (e.g., status_number).
internal/cli/poll.go(file-level): Line 654 · [low] architectural-divergence
GitLab poll path still uses hardcoded HarnessRouter while Jira poll now uses CEL triggers, creating architectural divergence. The code comment acknowledges this as intentional interim state. This is documented tech debt, not a defect introduced by this PR.
internal/jirapoll/poller_test.go:359: [low] test-integrity
TestRunHappyPath_CommentWithSlashCommand had payload-integrity assertions removed. The stubMatcher does not produce an EventPayload, so the test no longer verifies the event-to-payload pipeline at the poller level.
pkg/behaviourtest/steps/jirapoll.go(file-level): Line 192 · [low] test-integrity
thenDispatchActorRole no longer filters dispatches by issueKey before checking actor role. In a multi-issue test scenario, this could match a dispatch from the wrong issue and verify the wrong role.
|
🤖 Finished Retro · ❌ Failure · Started 12:12 PM UTC · Completed 12:12 PM UTC Commit: |
Merge origin/main to incorporate recent changes including: - PER_REPO_WIF_REPOS=* for GCP public mint mode (PR #6377) - Base dispatch e2e tests (PR #6367) - CEL trigger jira-poll (PR #6340) - Other main-branch updates Resolved conflict in docs/cli/mint.md: adopted main's updated --public flag description (PER_REPO_WIF_REPOS=* replaces ALLOWED_ORGS=* for GCP) while preserving the new --status-auth, --status-github-group, and --status-github-client-id flags from this PR. Addresses review feedback on #6365
Summary
Replace the hardcoded
HarnessRouterin the Jira poll input driver with CEL trigger evaluation from the harness dispatch system (ADR 0061). The hardcoded router pre-determined stages and required per-stage workflow files, which broke repos using the defaultworkflow_callshim mode fromfullsend github setup. This change routes Jira poll events through harness CELtrigger:expressions — the same evaluation path as webhook-driven dispatch.Changes
EventMatcherinterface in thejirapollpackage replacingdispatch.EventRouterJiraEvent→normevent.Event(typed enums) for CEL evaluation instead ofdispatch.NormalizedEvent(plain strings)celMatcherin the CLI layer that loads harness CEL triggers once per cycle and evaluates each event viaharnessdispatch.MatchHarnessesDispatchRecord(execution refs) instead ofpoll.Dispatch(stage-based), compatible with the workflow_call shimrouterMatcheradapter for backward compatHarnessRouterandbuildRouterremain for the GitLab poll pathTesting
go vetclean on all affected packagesNote:
pre-commitcould not run in the sandbox due to network restrictions (git fetch blocked). The post-script runs it authoritatively.Closes #6339
Post-script verification
agent/6339-cel-trigger-jira-poll)7917eb6306949a94da6ecc5dc7105d70a2f70206..HEAD)