Skip to content

feat(dispatch): route /fs-plan-tests to the qualityflow stage #6284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
guyoron1 wants to merge 1 commit into from
+17 −5
Closed

feat(dispatch): route /fs-plan-tests to the qualityflow stage #6284

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
# lint-workflow-size: max-lines=610
# lint-workflow-size: max-lines=625
# Dispatcher workflow that routes events to agent workflows based on stage.
# Routing logic determines the stage from event context — the shim only
# forwards the raw event. Adding a new stage requires only a case branch
Expand Down Expand Up @@ -163,6 +163,15 @@ jobs:
STAGE="prioritize"
fi
;;
/fs-plan-tests)

@waynesun09 waynesun09 Aug 17, 2026

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.

MEDIUM — No test coverage added for the new /fs-plan-tests routing or its gates

The diff only touches dispatch.yml and the lint-workflow-size max-lines directive; internal/scaffold/workflow_call_alignment_test.go gains no new test cases. The PR description substitutes an ad hoc, out-of-repo script run (a manual verification table) for actual in-repo coverage, and offers to port it into the behaviour-test harness later. As-is, nothing in the Go test suite asserts /fs-plan-tests routes to stage qualityflow, requires write-level (not triage) auth, requires ISSUE_HAS_PR, fails closed on PR-head fetch failure for the qualityflow stage, or is blocked on fork PRs by the extended regex.

Suggestion: add unit test cases mirroring existing /fs-fix/-review coverage in workflow_call_alignment_test.go: assert /fs-plan-tests requires write-level auth, requires ISSUE_HAS_PR == true, fails closed (exit 1) on PR-head fetch failure, and is blocked on fork PRs via the same regex as fix/qualityflow.

# QualityFlow custom agent. Mirrors its harness CEL trigger
# (change_proposal, non-fork); fork gate is in "Resolve PR
# head" below. Interim until CEL dispatch is primary (#2902).
if [[ "${COMMENT_USER_TYPE}" != "Bot" ]] && is_authorized \

@waynesun09 waynesun09 Aug 17, 2026

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.

MEDIUM — /fs-plan-tests gate order runs the GitHub permission-API call before the cheap ISSUE_HAS_PR check

Because bash && short-circuits left-to-right, is_authorized (a gh api repos/.../collaborators/<user>/permission network call) runs for every non-bot /fs-plan-tests comment regardless of whether the comment is even on a PR, since the cheap ISSUE_HAS_PR check is evaluated last. The existing /fs-review and /fs-fix cases in this same file put ISSUE_HAS_PR as the outer/first condition specifically to skip the API call when it can't route anywhere, so this is a real deviation from the established pattern, adding avoidable GitHub API load on every plain-issue use of the command.

Suggestion: reorder to check ISSUE_HAS_PR first, matching /fs-review and /fs-fix:

if [[ "${ISSUE_HAS_PR}" == "true" ]]; then
  if [[ "${COMMENT_USER_TYPE}" != "Bot" ]] && is_authorized; then
    STAGE="qualityflow"
  fi
fi

&& [[ "${ISSUE_HAS_PR}" == "true" ]]; then
STAGE="qualityflow"
fi
Comment on lines +166 to +173

@qodo-code-review qodo-code-review Bot Aug 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. /fs-plan-tests routing out-of-sync 📘 Rule violation ⚙ Maintainability

internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml adds routing for /fs-plan-tests
to STAGE="qualityflow", but .github/workflows/reusable-dispatch.yml does not implement the same
routing (and related stage handling), violating the requirement to keep the per-org and per-repo
dispatch workflows in sync. This drift makes /fs-plan-tests behave differently across install
modes and can result in per-repo dispatch being skipped when STAGE remains empty.
Agent Prompt 
## Issue description
The per-org dispatcher scaffold (`internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml`) now routes the `/fs-plan-tests` slash command to `STAGE=qualityflow`, but the per-repo dispatcher (`.github/workflows/reusable-dispatch.yml`) does not include the same `/fs-plan-tests` routing and associated stage handling (role mapping and relevant gating). This creates stage-routing drift between the two dispatch workflows and causes per-repo installations to ignore `/fs-plan-tests` (logging that no stage matched and skipping dispatch).

## Issue Context
This repository maintains two dispatch workflows:
- Per-org dispatcher scaffold: `internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml`
- Per-repo dispatcher: `.github/workflows/reusable-dispatch.yml`

Compliance requires jq payload/stage routing/secret threading logic to remain in sync across these dispatch workflows unless an explicit inline comment documents an intentional divergence. When introducing a new built-in command/stage, both dispatchers should support the same slash-command surface; otherwise behavior drifts across install modes.

## Fix Focus Areas
- internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[166-173]
- .github/workflows/reusable-dispatch.yml[185-245]
- .github/workflows/reusable-dispatch.yml[440-454]
- .github/workflows/reusable-dispatch.yml[494-520]

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

;;
*)
# Intentionally weaker gate: allows external reporters to
# re-trigger triage by providing clarification on needs-info
Expand Down Expand Up @@ -423,7 +432,9 @@ jobs:
set -euo pipefail
STAGE_ROLE="$STAGE"
case "$STAGE" in
code|fix) STAGE_ROLE="coder" ;;
# qualityflow declares "role: coder" in its harness — it commits
# generated tests to the PR branch, same trust level as code/fix.
code|fix|qualityflow) STAGE_ROLE="coder" ;;
esac

ROLES=$(yq '.defaults.roles[]' config.yaml 2>/dev/null || echo "")
Expand Down Expand Up @@ -477,18 +488,19 @@ jobs:
--jq '{number, html_url,
head: {ref: .head.ref, sha: .head.sha, repo: {full_name: .head.repo.full_name}},
base: {ref: .base.ref, repo: {full_name: .base.repo.full_name}}}') || {
if [[ "${STAGE}" =~ ^(fix|review)$ ]]; then
if [[ "${STAGE}" =~ ^(fix|review|qualityflow)$ ]]; then
echo "::error::Failed to fetch PR #${PR_NUMBER} head info"
exit 1
fi
echo "::warning::Failed to fetch PR #${PR_NUMBER} head info — continuing without PR context"
exit 0
}
if [[ "${STAGE}" == "fix" ]]; then
# Mutating agents that push to the PR branch must not run on forks.
if [[ "${STAGE}" =~ ^(fix|qualityflow)$ ]]; then
HEAD_REPO=$(printf '%s' "${PR_JSON}" | jq -r '.head.repo.full_name')
BASE_REPO=$(printf '%s' "${PR_JSON}" | jq -r '.base.repo.full_name')
if [[ "${HEAD_REPO}" != "${BASE_REPO}" ]]; then
echo "::error::Fork PR detected (head=${HEAD_REPO}, base=${BASE_REPO}) — fix agent blocked"
echo "::error::Fork PR detected (head=${HEAD_REPO}, base=${BASE_REPO}) — ${STAGE} agent blocked"
exit 1
fi
fi
Expand Down
Loading