fix(triage): skip code label for closed issues by shairevivo · Pull Request #1120 · fullsend-ai/agents

shairevivo · 2026-09-01T16:37:29Z

Summary

check the live GitHub issue state immediately before applying the deferred ready-to-code label
skip that routing label when the issue is closed or its state cannot be verified
preserve informational labels and the triage comment
sanitize values interpolated into workflow commands
add regressions for concurrent closure and state lookup failure

The authoritative dispatch-side guard for the remaining read/write race is in fullsend-ai/fullsend#6876.

Testing

PATH=/opt/homebrew/bin:$PATH make check-bundle
PATH=/opt/homebrew/bin:$PATH make lint
SCRIPT_TEST_TARGET=source bash scripts/post-triage-test.sh
SCRIPT_TEST_TARGET=bundled bash scripts/post-triage-test.sh

The focused source and bundled suites pass. The broader local make script-test matrix reached the gitlint test and stopped because gitlint is not installed locally; CI runs the dependency-equipped matrix.

Fixes fullsend-ai/fullsend#6156

github-actions · 2026-09-01T16:37:41Z

Functional tests did not run

Functional 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.

qodo-code-review · 2026-09-01T16:38:33Z

PR Summary by Qodo

Skip ready-to-code labeling for closed or unverifiable issues

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Verify GitHub issue state immediately before applying deferred ready-to-code labels.
• Skip routing labels when state is closed or unverifiable while preserving triage output.
• Cover closed-issue and API-failure behavior with mocked shell tests.

Diagram

graph TD
  A["Triage result"] --> B["Informational updates"] --> C["GitHub issue state"] --> D{"Open and verified?"}
  D -- Yes --> E["Apply ready-to-code"] --> F["Post triage comment"]
  D -- No --> G["Skip routing label"] --> F

High-Level Assessment

The last-moment, fail-closed guard is the appropriate approach because it minimizes the race window without discarding useful informational labels or comments. Checking earlier would allow closure during processing, while aborting the entire script would unnecessarily suppress valid triage output; a tracker-wide abstraction is also unnecessary for this GitHub-specific routing label.

Files changed (3) +75 / -0

Bug fix (2) +20 / -0

post-triage.shGuard deferred routing in the generated script +10/-0

Guard deferred routing in the generated script

• Updates the generated post-triage bundle to fetch the current GitHub issue state immediately before applying ready-to-code. The label is suppressed with a diagnostic when the issue is not open or its state cannot be verified.

scripts/post-triage.sh

post-triage.src.shVerify issue state before ready-to-code routing +10/-0

Verify issue state before ready-to-code routing
• Adds the source-of-truth guard for GitHub ready-to-code labeling. Other trackers and deferred labels retain their existing behavior.
scripts/post-triage.src.sh

Tests (1) +55 / -0

post-triage-test.shTest closed and unverifiable issue routing +55/-0

Test closed and unverifiable issue routing

• Extends the GitHub CLI mock to return configurable issue states or simulate API failure. Adds coverage proving closed and unverifiable issues skip ready-to-code while closed issues still receive informational labels and triage comments.

scripts/post-triage-test.sh

Check the current GitHub issue state immediately before applying the deferred ready-to-code label. Fail closed for that routing label when the state cannot be verified. Refs fullsend-ai/fullsend#6156 Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Shai Revivo <srevivo@redhat.com>

qodo-code-review · 2026-09-01T16:40:34Z

Code Review by Qodo

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

1. ~~Protected scripts lack authorization~~ ✗ Dismissed 📜 Skill insight § Compliance

Description

This non-trivial PR modifies three files under the protected scripts/ path without a linked
authorizing issue. Protected-path changes require human approval, and the missing authorization must
be resolved before approval.

Code

scripts/post-triage.src.sh[R722-724]

+if [[ "${DEFERRED_LABEL}" == "ready-to-code" ]] && [[ "${FULLSEND_TRACKER}" == "github" ]]; then
+  if ! ISSUE_STATE=$(gh api "repos/${REPO}/issues/${ISSUE_NUMBER}" --jq '.state'); then
+    echo "::warning::Unable to verify issue #${ISSUE_NUMBER} state; skipping ready-to-code label"

Relevance

●●● Strong
Protected-path enforcement findings are accepted when changes lack documented authorization or
required configuration.
PR-#569

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1538392 explicitly classifies scripts/ as a protected path and requires a finding and human
approval whenever it is modified. Rule 1538390 additionally requires a linked issue for this
non-trivial change, while the supplied PR metadata contains no linked issue.
scripts/post-triage.src.sh[722-730]
scripts/post-triage.sh[1672-1680]
scripts/post-triage-test.sh[651-688]
Skill: pr-review

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 PR modifies protected infrastructure scripts but does not link an issue authorizing the non-trivial behavioral change.

## Issue Context
Add the linked issue or ADR that authorizes the closed-issue routing guard and ensure the protected-path changes receive human review rather than automated approval.

## Fix Focus Areas
- scripts/post-triage.src.sh[722-730]
- scripts/post-triage.sh[1672-1680]
- scripts/post-triage-test.sh[651-688]

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

2. ISSUE_NUMBER bypasses sanitization ✓ Resolved 📜 Skill insight ⛨ Security

Description

The new ::warning:: command interpolates ISSUE_NUMBER directly in both the source and bundled
scripts. Every value interpolated into a GitHub Actions workflow command must be sanitized
individually to prevent command or log injection.

Code

scripts/post-triage.src.sh[724]

+    echo "::warning::Unable to verify issue #${ISSUE_NUMBER} state; skipping ready-to-code label"

Relevance

●●● Strong
Recent precedents consistently accept sanitization of values interpolated into GitHub Actions
workflow commands.
PR-#508
PR-#573
PR-#757

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rules 1538312 and 1538382 require every interpolated workflow-command value to be sanitized
individually. Both changed warning commands insert ${ISSUE_NUMBER} directly rather than passing it
through the available sanitization mechanism.
scripts/post-triage.src.sh[724-724]
scripts/post-triage.sh[1674-1674]
Skill: code-review
Skill: pr-review

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 new GitHub Actions `::warning::` command directly interpolates `ISSUE_NUMBER` without sanitization.

## Issue Context
Use the repository's workflow-command sanitization mechanism for `ISSUE_NUMBER`, then regenerate the bundled `post-triage.sh` so both copies remain synchronized. Ensure the resulting sanitization covers workflow-command delimiters, encoded newlines, ANSI escapes, and control characters.

## Fix Focus Areas
- scripts/post-triage.src.sh[722-725]
- scripts/post-triage.sh[1672-1675]

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

3. Closure race still routes 🐞 Bug ☼ Reliability

Description

The issue can close after the new state lookup reports open but before the separate label POST, so
ready-to-code can still be applied to a closed issue. Because label mutations emit
issues.labeled and the workflow routes that event, the core closed-issue routing invariant remains
unenforced.

Code

scripts/post-triage.src.sh[R722-723]

+if [[ "${DEFERRED_LABEL}" == "ready-to-code" ]] && [[ "${FULLSEND_TRACKER}" == "github" ]]; then
+  if ! ISSUE_STATE=$(gh api "repos/${REPO}/issues/${ISSUE_NUMBER}" --jq '.state'); then

Relevance

●● Moderate
Routing-label invariants are accepted, but no close precedent specifically addresses the state-read
versus label-write race.
PR-#567
PR-#827

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added block performs only a standalone state read; forge_ensure_label and tracker_add_label
execute afterward, with the latter issuing a separate POST. Repository documentation confirms this
POST emits issues.labeled, and the shim subscribes to and dispatches labeled issue events without
a local open-state condition.
scripts/post-triage.src.sh[722-740]
scripts/lib/github-triage-ops.lib.sh[35-43]
scripts/post-triage.src.sh[14-18]
.github/workflows/fullsend.yaml[22-48]

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 new issue-state check is separated from the `ready-to-code` label mutation by additional API work. If the issue closes after the GET but before the label POST, the closed issue still receives the routing label and emits the routing event.

## Issue Context
GitHub does not provide an atomic conditional label mutation. Keep the producer-side check as an optimization, but enforce the closed-state invariant authoritatively in the workflow/event-dispatch path using the issue state from the event payload.

## Fix Focus Areas
- scripts/post-triage.src.sh[722-740]
- scripts/post-triage.sh[1672-1690]
- .github/workflows/fullsend.yaml[35-48]

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

4. ~~API-error continuation untested~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The API-error test checks only successful exit and absence of ready-to-code, so it passes if the
script silently skips the comment or other remaining processing. This fails to verify the test's
explicit requirement that post-triage processing continue after the lookup error.

Code

scripts/post-triage-test.sh[R989-994]

+# State lookup failures must fail closed for the routing label while allowing
+# the rest of post-triage processing to complete.
+run_test_no_pattern_with_env "issue-state-api-error-skips-ready-to-code" \
+  "${AUTO_CODE_BUG_FIXTURE}" \
+  "labels[]=ready-to-code" \
+  "MOCK_ISSUE_STATE=api-error"

Relevance

●●● Strong
Recent accepted precedents require tests to observe the behavior they claim to cover.
PR-#1068
PR-#1075

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The invoked helper validates only exit status and absence of one log pattern, whereas the
neighboring closed-state test explicitly checks the informational label, comment, and diagnostic.
Thus the new API-error test cannot detect loss of the continuation behavior it documents.
scripts/post-triage-test.sh[949-984]
scripts/post-triage-test.sh[989-994]
scripts/post-triage-test.sh[670-685]
PR-#1068

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 API-error regression test only asserts that `ready-to-code` is absent. It does not prove that comments and informational mutations still execute after the lookup failure, despite claiming to cover that behavior.

## Issue Context
Add an informational `label_actions` entry to the fixture and assert both its API call and the triage comment call, along with the warning and absence of `ready-to-code`. A dedicated helper or explicit test block can support these positive and negative assertions.

## Fix Focus Areas
- scripts/post-triage-test.sh[949-994]

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

Context sources

✅ Compliance rules (platform): 57 rules

✅ Skills: 4 invoked

code-review

code-implementation

pr-review

docs-review

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Sanitize the issue number in the workflow warning and verify that API lookup failures preserve informational labels and the triage comment. Refs fullsend-ai/fullsend#6156 Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Shai Revivo <srevivo@redhat.com>

Write JSON fixtures with printf and scope mock issue state to each command. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Shai Revivo <srevivo@redhat.com>

ralphbean · 2026-09-01T21:17:57Z

Can you modify this to not be special-cased only for github?

shairevivo · 2026-09-02T09:36:35Z

Can you modify this to not be special-cased only for github?

Yes, good point. on it. Adding Jira and Gitlab as well

The ready-to-code closed-issue guard was gated on FULLSEND_TRACKER=github, so GitLab and Jira issues could still be routed to the code agent after being closed. Add a tracker_issue_state operation to each tracker ops library (github/gitlab/jira) that normalizes the native state to open/closed and fails non-zero when it cannot be determined, then have post-triage call it tracker-agnostically. The guard continues to fail closed for both a verified-closed issue and an unverifiable one. Add GitLab and Jira regression tests covering the closed-issue skip and the state-lookup-failure skip, and regenerate the bundled scripts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Shai Revivo <srevivo@redhat.com>

shairevivo requested a review from a team as a code owner September 1, 2026 16:37

shairevivo force-pushed the codex/6156-closed-issue-guard branch from b3d9185 to 58cbd92 Compare September 1, 2026 16:39

qodo-code-review Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread scripts/post-triage.src.sh Outdated

Comment thread scripts/post-triage.src.sh Outdated

Comment thread scripts/post-triage.src.sh Outdated

Comment thread scripts/post-triage-test.sh Outdated

shairevivo mentioned this pull request Sep 1, 2026

fix(#6156): block closed issue code dispatch fullsend-ai/fullsend#6876

Open

test(triage): satisfy shellcheck for state fixtures

d48ebc4

Write JSON fixtures with printf and scope mock issue state to each command. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Shai Revivo <srevivo@redhat.com>

Navigation Menu

Uh oh!

fix(triage): skip code label for closed issues - #1120

fix(triage): skip code label for closed issues#1120
shairevivo wants to merge 4 commits into
fullsend-ai:mainfrom
shairevivo:codex/6156-closed-issue-guard

shairevivo commented Sep 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 1, 2026

Uh oh!

qodo-code-review Bot commented Sep 1, 2026

Uh oh!

qodo-code-review Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ralphbean commented Sep 1, 2026

Uh oh!

shairevivo commented Sep 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

shairevivo commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

github-actions Bot commented Sep 1, 2026

Functional tests did not run

Uh oh!

qodo-code-review Bot commented Sep 1, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ralphbean commented Sep 1, 2026

Uh oh!

shairevivo commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shairevivo commented Sep 1, 2026 •

edited

Loading

qodo-code-review Bot commented Sep 1, 2026 •

edited

Loading

shairevivo commented Sep 2, 2026 •

edited

Loading