fix(#6156): block closed issue code dispatch

[shairevivo]

Summary

• read the issue state from the triggering GitHub event
• route ready-to-code labels only when that event reports an open issue
• keep the reusable and scaffold dispatch implementations synchronized
• add a regression covering both routing implementations

This is the authoritative dispatch-side guard for the close-between-check-and-label race. The producer-side optimization and post-triage regressions are in fullsend-ai/agents#1120.

Testing

• GOCACHE=/private/tmp/fullsend-6156-go-cache go test ./internal/scaffold -count=1
• GOCACHE=/private/tmp/fullsend-6156-go-cache make lint

Fixes #6156

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[qodo-code-review]

PR Summary by Qodo

Block code dispatch for closed issues

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Read issue state directly from labeled GitHub events.
• Dispatch ready-to-code only when the issue remains open.
• Verify reusable and scaffold workflows enforce the same guard.

Diagram

graph TD
  E["Labeled event"] --> L{"Ready to code?"} -->|Yes| S{"Issue open?"} -->|Yes| A{"Actor authorized?"} -->|Yes| C["Code stage"]
  L -->|No| O["Other routing"]
  S -->|No| N["No dispatch"]
  A -->|No| N

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Guard only in the label producer

• ➕ Avoids emitting ready-to-code labels for issues already known to be closed.
• ➕ Keeps dispatch routing logic slightly simpler.
• ➖ Cannot prevent a close-between-check-and-label race.
• ➖ Relies on every producer implementing identical safeguards.

2. Fetch live issue state before dispatch

• ➕ Can observe state changes occurring after the event payload was created.
• ➕ Centralizes validation at the final routing boundary.
• ➖ Adds an API request, latency, and another failure mode.
• ➖ Requires extra permissions and rate-limit handling for a simple event-local check.

Recommendation: Keep the PR's event-state guard at the final dispatch boundary. It closes the identified race without extra API calls, and applying it to both reusable and scaffold workflows prevents generated repositories from diverging. Producer-side checks remain useful as an optimization but should not replace this authoritative guard.

Files changed (3) +27 / -2

Bug fix (2) +4 / -2

reusable-dispatch.ymlGuard reusable code dispatch by issue state +2/-1

Guard reusable code dispatch by issue state

• Exposes the triggering event's issue state to the route step. The ready-to-code path now selects the code stage only when that state is open, preventing dispatch after issue closure.

.github/workflows/reusable-dispatch.yml

dispatch.ymlMirror the closed-issue guard in scaffold dispatch +2/-1

Mirror the closed-issue guard in scaffold dispatch

• Adds issue state to the scaffold workflow environment and applies the same open-state requirement to ready-to-code routing. This keeps newly scaffolded repositories aligned with the reusable workflow.

internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml

Tests (1) +23 / -0

workflow_call_alignment_test.goTest closed-issue dispatch guards in both workflows +23/-0

Test closed-issue dispatch guards in both workflows

• Adds a regression test that inspects both workflow implementations for event-derived issue state and the open-state condition before code-stage selection.

internal/scaffold/workflow_call_alignment_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. State guard test passes falsely ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The regression regex only requires the label check, state check, and STAGE="code" to occur in
order anywhere in the workflow, so it still passes if the checks are separated or joined with
incorrect logic such as ||. This leaves the close-race behavior without reliable regression
protection.

Code

internal/scaffold/workflow_call_alignment_test.go[669]

+			assert.Regexp(t, `(?s)TRIGGERING_LABEL\}" == "ready-to-code".*ISSUE_STATE\}" == "open".*STAGE="code"`, s,

Relevance

●●● Strong

Accepted precedents favor tightening broad workflow-regression regexes to enforce specific
security-relevant conditions.

PR-#5244
PR-#6591

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test enables dot-all mode and places unrestricted .* between all three fragments, which proves
only file-wide ordering. The workflows show the intended invariant is specifically one elif
condition combining the label and open-state checks before assigning the code stage.

internal/scaffold/workflow_call_alignment_test.go[652-670]
.github/workflows/reusable-dispatch.yml[275-282]
internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[177-184]

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 closed-issue regression assertion uses unrestricted dot-all matching, so unrelated or incorrectly connected expressions can satisfy it. Replace it with an assertion that verifies the exact `ready-to-code && ISSUE_STATE == open` conditional and binds `STAGE="code"` to that branch.

## Issue Context
Both workflow implementations currently contain the correct adjacent condition, but the test would not detect changing `&&` to `||`, moving the state check elsewhere, or placing `STAGE="code"` in an independent branch. Prefer an exact `assert.Contains` for the conditional or a tightly bounded multiline regex without unrestricted `.*` across the file.

## Fix Focus Areas
- internal/scaffold/workflow_call_alignment_test.go[652-671]
- .github/workflows/reusable-dispatch.yml[275-282]
- internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[177-184]

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

---

Context sources

✅ Compliance rules (platform): 72 rules

✅ Skills: writing-user-docs, writing-adrs

Review mode: ⚖️ Balanced: This changes GitHub Actions dispatch behavior on a race-sensitive issue-routing path and synchronizes two implementations, so it warrants a careful single-pass review.

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ralphbean]

This is touching on a complicated area. The conditions that are used to trigger an agent (the "predicate" of the agent) needs to move out of these .github/ files into something that is handled between the NormalizedEvent and the trigger of the agent - but you've caught us mid-migration. BYOA agents (and newly authored agents in the default suite) can be written using a trigger field directly on the harness where you describe when the agent should or should not run - but, we haven't ported the default suite of agents (triage, code, etc..) over to this new scheme yet. I think I'd rather not extend the business logic in these github workflow conditionals unless we really need to - and avoiding running code on a closed issue sounds like a not-very-likely case. Not worth adding yet another conditional that we need to map through a migration soon.

[shairevivo]

This is touching on a complicated area. The conditions that are used to trigger an agent (the "predicate" of the agent) needs to move out of these .github/ files into something that is handled between the NormalizedEvent and the trigger of the agent - but you've caught us mid-migration. BYOA agents (and newly authored agents in the default suite) can be written using a trigger field directly on the harness where you describe when the agent should or should not run - but, we haven't ported the default suite of agents (triage, code, etc..) over to this new scheme yet. I think I'd rather not extend the business logic in these github workflow conditionals unless we really need to - and avoiding running code on a closed issue sounds like a not-very-likely case. Not worth adding yet another conditional that we need to map through a migration soon.

@ralphbean in this case both PRs need to close. I agree it is a "corner case" but the cleanest implementation need both sides.

1. Producer side (agents repo, my PR Follow-up from PR #1119: Error message hardcodes "too many repos (max 500)" instead of deriving from the maxRe... #1120) — the triage robot checks "is this issue open?" before it applies the ready-to-code label. This is a best-effort, early guard: catch obviously-closed issues cheaply and don't even bother ringing the bell.
2. Dispatch side (fullsend repo, fix(#6156): block closed issue code dispatch #6876) — the workflow that actually reacts to the label and launches the coding agent. It re-checks the issue state, but from the triggering event itself (.github/workflows/reusable-dispatch.yml + the scaffold copy). This is the authoritative, final guard.

Feel free to close it if you think it is too risky and not worthy to fix.

[waynesun09]

[MEDIUM] "Authoritative dispatch-side guard" only covers the GitHub webhook route; GitLab/Jira poll routing has the same race unguarded

Anchor: internal/dispatch/router.go:129 (not in this diff, so noting it here rather than inline).

The PR body calls this "the authoritative dispatch-side guard for the close-between-check-and-label race", but only the GitHub Actions shell router (reusable-dispatch.yml + scaffold dispatch.yml) gained the check. Verified on head 2cc8c60:

• internal/forge/gitlab/poll.go:42 lists issues with updated_after= only (no state=opened).
• internal/poll/state.go detectNewLabels emits a label-added event for any issue in the poll set whose labels changed, and only consults isIssueClosed (state.go:103-132) in the prune path for issues NOT in the current poll set — so a just-closed issue with a fresh ready-to-code still produces an event.
• HarnessRouter.routeLabel (router.go:129-130) maps ready-to-code -> code with no open/closed check.
• normevent.State (internal/normevent/event.go:129-133) carries only Labels/ChangeProposal/Conversation, so neither routeLabel nor a CEL trigger can check lifecycle state today. poll.Issue already has a State field (internal/poll/client.go:64).
• Jira's default JQL excludes statusCategory != Done, but a user-supplied --jql bypasses that (internal/jirapoll/poller.go:319-324).

Not raised in any existing thread — the discussion above covers the NormalizedEvent/trigger migration generally but does not name this gap. This finding actually reinforces that stated direction: an issue lifecycle field on NormalizedEvent would cover GitHub, GitLab, Jira and CEL triggers in one place instead of another workflow conditional.

Suggestion: Preferred: add an issue open/closed lifecycle field to normevent.State (populated from poll.Issue.State and the GitHub event) and check it in HarnessRouter.routeLabel, which is where agent predicates are migrating; that makes the guard genuinely cross-forge. Minimal alternative: skip ready-to-code in detectNewLabels/discoverAllEvents when iss.State == "closed" (field already exists). If deferred, soften the PR body to "GitHub dispatch-side guard", cross-link #1647, and file a follow-up for the poll/CEL route so the claim matches what ships.

[waynesun09]

[MEDIUM] /fs-code still dispatches the code stage on a closed issue, contradicting the documented "agents don't act on closed issues" contract

Verified on head 2cc8c60: the issue_comment /fs-code branch in this same run: step (reusable-dispatch.yml:221-226; scaffold dispatch.yml:123-128) checks only ISSUE_IS_PR/ISSUE_HAS_PR == "false", COMMENT_USER_TYPE != Bot and is_authorized — it never reads ISSUE_STATE, even though github.event.issue.state is populated on issue_comment events and the env var is now exported to this step. docs/guides/user/bugfix-workflow.md:170 states "Agents don't act on closed issues (except /fs-triage which explicitly reopens)" — an explicit exception list that does not include /fs-code — so the label path and the slash-command path now enforce different close-stops-agents contracts, and scripts/pre-code.sh in fullsend-ai/agents has no issue-state check either (its only state queries are on linked PRs).

Caveat: this is pre-existing behaviour in an unmodified branch of the same modified step, not a regression introduced by the diff, and treating /fs-code as a deliberate human override is arguably intentional.

Suggestion: Given the preference not to grow workflow conditionals, the cheapest fix is docs-only: add /fs-code to the exception list at bugfix-workflow.md:170 and note in docs/agents/code.md that /fs-code is an explicit human override that runs on closed issues. Otherwise mirror the guard — add [[ "${ISSUE_STATE}" == "open" ]] to the /fs-code branch in both dispatch copies and extend TestClosedIssueReadyToCodeDoesNotDispatch to cover it. Either way, state the design decision explicitly so the two paths agree.