Skip to content

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

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

Open
shairevivo wants to merge 4 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58cbd92
fix(triage): skip code label for closed issues
shairevivo Sep 1, 2026
d976aca
fix(triage): address closed-issue review feedback
shairevivo Sep 1, 2026
d48ebc4
test(triage): satisfy shellcheck for state fixtures
shairevivo Sep 1, 2026
90bddb4
fix(triage): apply closed-issue guard across all trackers
shairevivo Sep 2, 2026
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
11 changes: 11 additions & 0 deletions scripts/lib/github-triage-ops.lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ tracker_close_issue() {
gh issue close "${ISSUE_NUMBER}" --repo "${REPO}" --reason "${reason}"
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# Returns non-zero if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local state
state=$(gh api "repos/${REPO}/issues/${ISSUE_NUMBER}" --jq '.state' 2>/dev/null) || return 1
case "${state}" in
open | closed) printf '%s\n' "${state}" ;;
*) return 1 ;;
esac
}

tracker_create_issue() {
local target_repo="$1"
local title="$2"
Expand Down
14 changes: 14 additions & 0 deletions scripts/lib/gitlab-triage-ops.lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,20 @@ tracker_close_issue() {
fi
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# GitLab reports "opened"/"closed"; map "opened" to "open" for a tracker-neutral
# contract. Returns non-zero if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local raw state
raw=$(_gitlab_api GET "/projects/${REPO_ENCODED}/issues/${ISSUE_NUMBER}" 2>/dev/null) || return 1
state=$(printf '%s' "${raw}" | jq -r '.state // empty' 2>/dev/null) || return 1
case "${state}" in
opened) printf 'open\n' ;;
closed) printf 'closed\n' ;;
*) return 1 ;;
esac
}

tracker_create_issue() {
local target_repo="$1"
local title="$2"
Expand Down
15 changes: 15 additions & 0 deletions scripts/lib/jira-triage-ops.lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,21 @@ tracker_close_issue() {
fi
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# Jira has no open/closed flag; its status category "done" is the terminal state,
# so map "done" to "closed" and every other category to "open". Returns non-zero
# if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local raw category
raw=$(_jira_api GET "/issue/${ISSUE_NUMBER}?fields=status" 2>/dev/null) || return 1
category=$(printf '%s' "${raw}" | jq -r '.fields.status.statusCategory.key // empty' 2>/dev/null) || return 1
case "${category}" in
done) printf 'closed\n' ;;
"") return 1 ;;
*) printf 'open\n' ;;

@waynesun09 waynesun09 Sep 2, 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] Jira undefined status category is routed as open, contradicting the fail-closed contract

Jira's statusCategory.key has four documented values: undefined ("No Category", id 1), new, indeterminate, done. The new tracker_issue_state returns 1 for an empty key, but this wildcard branch maps undefined (and any future/drifted key, or a case-shifted Done) to open. That is the one case where the tracker itself says it cannot categorise the status, yet the caller applies ready-to-code and dispatches the code agent. This contradicts the function's own doc comment ("Returns non-zero if the state cannot be determined (caller fails closed)") and the PR's stated fail-closed design. The same code is bundled into post-triage.sh:875 and pre-triage.sh:871.

Suggestion: allowlist the known in-flight keys and fail closed on everything else:

case "${category}" in
  done) printf 'closed\n' ;;
  new | indeterminate) printf 'open\n' ;;
  *) return 1 ;;
esac

Update the doc comment to enumerate the four keys, regenerate the bundles (make bundle / make check-bundle), and add Jira regression cases for new (still open) and undefined (skips ready-to-code).

esac
}

_text_to_adf() {
# Convert plain text to Atlassian Document Format (ADF) JSON.
# Splits on blank lines into separate paragraphs; within a paragraph,
Expand Down
254 changes: 252 additions & 2 deletions scripts/post-triage-test.sh
View file
Open in desktop

Large diffs are not rendered by default.

57 changes: 57 additions & 0 deletions scripts/post-triage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,17 @@ tracker_close_issue() {
gh issue close "${ISSUE_NUMBER}" --repo "${REPO}" --reason "${reason}"
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# Returns non-zero if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local state
state=$(gh api "repos/${REPO}/issues/${ISSUE_NUMBER}" --jq '.state' 2>/dev/null) || return 1
case "${state}" in
open | closed) printf '%s\n' "${state}" ;;
*) return 1 ;;
esac
}

tracker_create_issue() {
local target_repo="$1"
local title="$2"
Expand Down Expand Up @@ -492,6 +503,20 @@ tracker_close_issue() {
fi
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# GitLab reports "opened"/"closed"; map "opened" to "open" for a tracker-neutral
# contract. Returns non-zero if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local raw state
raw=$(_gitlab_api GET "/projects/${REPO_ENCODED}/issues/${ISSUE_NUMBER}" 2>/dev/null) || return 1
state=$(printf '%s' "${raw}" | jq -r '.state // empty' 2>/dev/null) || return 1
case "${state}" in
opened) printf 'open\n' ;;
closed) printf 'closed\n' ;;
*) return 1 ;;
esac
}

tracker_create_issue() {
local target_repo="$1"
local title="$2"
Expand Down Expand Up @@ -843,6 +868,21 @@ tracker_close_issue() {
fi
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# Jira has no open/closed flag; its status category "done" is the terminal state,
# so map "done" to "closed" and every other category to "open". Returns non-zero
# if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local raw category
raw=$(_jira_api GET "/issue/${ISSUE_NUMBER}?fields=status" 2>/dev/null) || return 1
category=$(printf '%s' "${raw}" | jq -r '.fields.status.statusCategory.key // empty' 2>/dev/null) || return 1
case "${category}" in
done) printf 'closed\n' ;;
"") return 1 ;;
*) printf 'open\n' ;;
esac
}

_text_to_adf() {
# Convert plain text to Atlassian Document Format (ADF) JSON.
# Splits on blank lines into separate paragraphs; within a paragraph,
Expand Down Expand Up @@ -1669,6 +1709,23 @@ fi

# --- Apply deferred label (must be last label mutation) ---

if [[ "${DEFERRED_LABEL}" == "ready-to-code" ]]; then
# Best-effort producer-side guard: don't route a closed issue to the code
# agent. tracker_issue_state normalizes each tracker's state to open/closed
# and returns non-zero when it cannot be determined, so we fail closed for
# both a verified-closed issue and an unverifiable one. The authoritative
# guard against the close-between-check-and-label race lives in the dispatch
# workflow (fullsend-ai/fullsend#6876), which reads the triggering event's
# issue state.
if ! ISSUE_STATE=$(tracker_issue_state); then
echo "::warning::Unable to verify issue #$(_gha_sanitize "${ISSUE_NUMBER}") state; skipping ready-to-code label"
DEFERRED_LABEL=""
elif [[ "${ISSUE_STATE}" != "open" ]]; then
echo "Issue #${ISSUE_NUMBER} is ${ISSUE_STATE}; skipping ready-to-code label"
DEFERRED_LABEL=""
fi
fi

if [[ -n "${DEFERRED_LABEL}" ]]; then
echo "Applying deferred label '${DEFERRED_LABEL}'..."
# forge_ensure_label creates the label via `gh` against REPO. On Jira, REPO
Expand Down
17 changes: 17 additions & 0 deletions scripts/post-triage.src.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,23 @@ fi

# --- Apply deferred label (must be last label mutation) ---

if [[ "${DEFERRED_LABEL}" == "ready-to-code" ]]; then
# Best-effort producer-side guard: don't route a closed issue to the code
# agent. tracker_issue_state normalizes each tracker's state to open/closed
# and returns non-zero when it cannot be determined, so we fail closed for
# both a verified-closed issue and an unverifiable one. The authoritative

@waynesun09 waynesun09 Sep 2, 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] Both PRs carry Fixes fullsend#6156, and this comment states an unmerged guard already exists

This block comment says the authoritative race guard "lives in the dispatch workflow (fullsend-ai/fullsend#6876)". As of today, fullsend#6876 is still OPEN (not merged) and fullsend#6156 is OPEN, so no such guard exists on either main branch. Both this PR (Fixes fullsend-ai/fullsend#6156) and #6876 (Fixes #6156) claim to close the same issue, so whichever merges first auto-closes #6156 with the other half of the fix still outstanding.

Additionally, consumers pin the floating v0 tag, which currently points at the v0.39.0 base commit c4e059f, so this producer-side guard does not reach fleet runs until the next agents release re-points the tag.

This is adjacent to the closure-race thread above (answered by pointing at #6876), but the point here is the accuracy of the cross-reference and issue-closing hygiene, which that thread does not address. Process-level rather than a code defect.

Suggestion: reword to "the authoritative guard is tracked in fullsend-ai/fullsend#6876" (or merge #6876 first). Keep Fixes on exactly one PR, preferably #6876 since it closes the race, and use Refs fullsend-ai/fullsend#6156 here. Note in the PR body that the change reaches fleet runs only after the next agents release.

# guard against the close-between-check-and-label race lives in the dispatch
# workflow (fullsend-ai/fullsend#6876), which reads the triggering event's
# issue state.
if ! ISSUE_STATE=$(tracker_issue_state); then
echo "::warning::Unable to verify issue #$(_gha_sanitize "${ISSUE_NUMBER}") state; skipping ready-to-code label"

@waynesun09 waynesun09 Sep 2, 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] Unverifiable issue state leaves an open bug with no routing label and a green run

When tracker_issue_state fails (transient 5xx/429/network, or the fail-closed *) return 1 branches), this guard emits ::warning:: and sets DEFERRED_LABEL="", so for an open, sufficient bug the run exits 0 having applied bug + the triage comment but neither ready-to-code nor triaged. The issue then has no routing label and nothing re-triggers triage; the only trace is a workflow annotation. The routing branch at lines 448-455 only has the triaged fallback for the WORKFLOW_BLOCKED/auto-code-off paths.

All three tracker_issue_state implementations discard stderr (2>/dev/null), so the warning cannot distinguish 401/403/404/5xx, and there is no retry. The PR body says the routing label is skipped when state cannot be verified, which covers withholding ready-to-code but does not address the absence of any fallback signal.

To be fair, the script's existing convention for read failures is ::warning:: + continue (lines 212, 525, 668), so the soft-fail itself is consistent; the gap is the missing fallback label.

Suggestion: on the lookup-failure branch (as opposed to a confirmed-closed result), fall back to tracker_add_label "triaged" instead of leaving DEFERRED_LABEL empty, mirroring the WORKFLOW_BLOCKED precedent at lines 439-440, so a transient blip visibly parks the issue for human follow-up. Optionally retry the lookup 2-3 times with a short backoff before failing closed, and capture stderr into a sanitized reason in the warning. Regenerate the bundle (make bundle) and add a test asserting triaged is applied on the api-error path for at least one tracker.

DEFERRED_LABEL=""
elif [[ "${ISSUE_STATE}" != "open" ]]; then
echo "Issue #${ISSUE_NUMBER} is ${ISSUE_STATE}; skipping ready-to-code label"
DEFERRED_LABEL=""
fi
fi

if [[ -n "${DEFERRED_LABEL}" ]]; then
echo "Applying deferred label '${DEFERRED_LABEL}'..."
# forge_ensure_label creates the label via `gh` against REPO. On Jira, REPO
Expand Down
40 changes: 40 additions & 0 deletions scripts/pre-triage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ tracker_close_issue() {
gh issue close "${ISSUE_NUMBER}" --repo "${REPO}" --reason "${reason}"
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# Returns non-zero if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local state
state=$(gh api "repos/${REPO}/issues/${ISSUE_NUMBER}" --jq '.state' 2>/dev/null) || return 1
case "${state}" in
open | closed) printf '%s\n' "${state}" ;;
*) return 1 ;;
esac
}

tracker_create_issue() {
local target_repo="$1"
local title="$2"
Expand Down Expand Up @@ -488,6 +499,20 @@ tracker_close_issue() {
fi
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# GitLab reports "opened"/"closed"; map "opened" to "open" for a tracker-neutral
# contract. Returns non-zero if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local raw state
raw=$(_gitlab_api GET "/projects/${REPO_ENCODED}/issues/${ISSUE_NUMBER}" 2>/dev/null) || return 1
state=$(printf '%s' "${raw}" | jq -r '.state // empty' 2>/dev/null) || return 1
case "${state}" in
opened) printf 'open\n' ;;
closed) printf 'closed\n' ;;
*) return 1 ;;
esac
}

tracker_create_issue() {
local target_repo="$1"
local title="$2"
Expand Down Expand Up @@ -839,6 +864,21 @@ tracker_close_issue() {
fi
}

# Echo the normalized issue state ("open" or "closed") and return 0 on success.
# Jira has no open/closed flag; its status category "done" is the terminal state,
# so map "done" to "closed" and every other category to "open". Returns non-zero
# if the state cannot be determined (caller fails closed).
tracker_issue_state() {
local raw category
raw=$(_jira_api GET "/issue/${ISSUE_NUMBER}?fields=status" 2>/dev/null) || return 1
category=$(printf '%s' "${raw}" | jq -r '.fields.status.statusCategory.key // empty' 2>/dev/null) || return 1
case "${category}" in
done) printf 'closed\n' ;;
"") return 1 ;;
*) printf 'open\n' ;;
esac
}

_text_to_adf() {
# Convert plain text to Atlassian Document Format (ADF) JSON.
# Splits on blank lines into separate paragraphs; within a paragraph,
Expand Down
Loading