fix(#2298): remove retro/prioritize→fullsend role mapping in dispatch - #2781
Conversation
PR Summary by QodoFix dispatch role-check: stop remapping retro/prioritize to fullsend
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Site previewPreview: https://0156b02f-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 3:05 PM UTC · Completed 3:25 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
Context used✅ Tickets:
🎫 dispatch: retro and prioritize skipped in per-repo mode — role check looks for fullsend✅ Compliance rules (platform):
61 rules✅ Skills:
writing-user-docs, writing-adrs 1.
|
ReviewFindingsMedium
Previous runReviewFindingsMedium
Previous runReviewFindingsMedium
Labels: PR modifies dispatch workflow routing and includes documentation updates Labels: PR modifies dispatch workflow routing logic and getting-started documentation Labels: PR modifies dispatch workflow routing and getting-started documentation Labels: PR modifies dispatch workflow routing and getting-started documentation Previous runReviewFindingsMedium
Labels: PR modifies dispatch workflow routing logic and getting-started documentation Previous run (2)ReviewFindingsMedium
Labels: PR modifies dispatch workflow routing and includes documentation updates Previous run (3)ReviewFindingsMedium
Previous run (4)ReviewFindingsMedium
Low
Labels: PR modifies dispatch workflow routing and includes documentation updates |
d8eb586 to
cbab251
Compare
|
🤖 Finished Review · ❌ Failure · Started 3:54 PM UTC · Completed 4:01 PM UTC |
ralphbean
left a comment
There was a problem hiding this comment.
LGTM. A couple non-blocking notes inline.
|
🤖 Review · |
2071822 to
644597e
Compare
|
🤖 Review · ❌ Terminated · Started 3:23 PM UTC · Ended 3:36 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 3:23 PM UTC · Completed 3:36 PM UTC |
644597e to
dd83093
Compare
|
🤖 Finished Review · ✅ Success · Started 3:46 PM UTC · Completed 3:56 PM UTC |
dd83093 to
b279f19
Compare
|
🤖 Review · |
Remove the `retro|prioritize) STAGE_ROLE="fullsend"` mapping from the role-check step in both dispatch workflows. In per-repo mode, `PerRepoDefaultRoles()` includes retro and prioritize directly but excludes fullsend, so the remapping caused these stages to be silently skipped. Add backward-compat fallback: when retro/prioritize is not in the roles list but "fullsend" is, allow the stage through. This preserves org-mode configs that used fullsend as an umbrella role. Update docs: the fullsend dispatch app is only required for org-mode installations — per-repo mode no longer maps retro/prioritize to the fullsend role, so the app is not needed. Fixes #2298 Assisted-by: Claude (fix), Gemini (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
The backward-compat path that allows retro/prioritize stages when "fullsend" is in roles was silently passing via no-op. Emit a ::notice:: annotation so operators can audit which path was taken during migration from the fullsend catch-all role. Add TestRoleCheckCaseBranches to validate: - code|fix maps to coder - retro/prioritize are not remapped to fullsend - compat path emits a ::notice:: Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
b279f19 to
dbca731
Compare
|
🤖 Finished Review · ✅ Success · Started 4:57 PM UTC · Completed 5:08 PM UTC |
|
🤖 Finished Retro · ✅ Success · Started 7:24 PM UTC · Completed 7:34 PM UTC |
Retro: PR #2781 — fix dispatch role-check for retro/prioritizeTimeline
Workflow quality assessmentWhat went well:
Waste areas:
ProposalsNo new proposals filed. All identified improvement opportunities are already covered by existing open issues: |
Summary
retro|prioritize) STAGE_ROLE="fullsend"mapping from the role-check step in both dispatch workflows (reusable-dispatch.ymland scaffolddispatch.yml)PerRepoDefaultRoles()includesretroandprioritizedirectly but excludesfullsend, so the remapping caused these stages to be silently skipped — and required thefullsend-ai-fullsendapp to be installed even in per-repo moderetro/prioritizeis not in the roles list butfullsendis, allow the stage with a::notice::annotation — preserves org-mode configs that usedfullsendas an umbrella rolefullsenddispatch app is only required for org-mode installations; per-repo mode no longer needs itTestRoleCheckCaseBranchesto guard against regressionTest plan
make lintpassesmake go-testpasses (newTestRoleCheckCaseBranchescovers both workflows)fullsend-playground/hello-pages) that/fs-retrodispatches correctly with default rolesfullsendindefaults.roles)Fixes #2298