Skip to content

rate-limit preflight: a failed issue create is swallowed, so the spike notice becomes a dangling #? #880

Description

@tend-agent

When gh issue create fails inside run_issue_create_and_reconcile, the failure is swallowed and the rate-limit preflight emits a literal #? where the issue number belongs. The notice that #874 exists to provide is never filed, and nothing says so.

Mechanism

Bash does not apply set -e inside a command substitution unless inherit_errexit is set, and neither rate-limit-preflight.sh nor lib/run-issue.sh sets it. So a failing gh issue create at run-issue.sh:119 does not abort the subshell. Execution continues to the reconcile, which finds nothing, and the function's last command — printf '%s' "$keep" at run-issue.sh:136 — succeeds. The substitution's exit status is that printf's, so the assignment at rate-limit-preflight.sh:182 sees success and set -e never fires.

PAUSE is left empty, and the next line renders ${PAUSE:-?}:

::error::Rate limit: bot created 47 items today, above the ceiling of 30 ... Refused runs are listed in #?; closing it doubles the ceiling.

Reduced to the shape of the real call:

set -eo pipefail
create_and_reconcile() {
  false                      # stands in for a failing `gh issue create`
  local open keep
  open=""                    # the post-create list finds nothing
  keep=${open%%$'\n'*}
  printf '%s' "$keep"        # last command succeeds -> substitution exits 0
}
PAUSE=$(printf 'body\n' | create_and_reconcile)
echo "reached line after assignment; PAUSE='${PAUSE}'"
$ bash repro.sh
reached line after assignment; PAUSE=''
script exit=0

$ bash -c 'shopt -s inherit_errexit; source repro.sh'
script exit=1

Why it matters here

The step still exits 1, so the run is still refused — this is not a case of the limit failing open. What is lost is the notice. #874's premise is that a bare ::error:: lands on a job nobody opens, which is why the spike trip files an issue at all; when the create fails, the run lands back in exactly that state, plus a dangling #? that reads as corruption. The maintainer has no issue to close, so the documented recovery — closing it to double the ceiling — is unavailable, and the bot stays halted for the rest of the UTC day with no route back.

A failing create is not hypothetical in this path: the preflight runs precisely when the bot is at abnormal volume, which is also when secondary-rate-limit and abuse-detection responses to gh issue create are most likely.

report-failure.sh reaches the same function through a pipeline rather than a command substitution (report-failure.sh:46) and discards stdout, so it is unaffected.

Filed rather than fixed

There is a plausible fix — have the function return non-zero when the create fails and let the caller distinguish "filed" from "could not file" — but it rewrites the body of run_issue_create_and_reconcile, which is exactly what open PR #836 is rewriting (settle-then-list reconcile → primary-key probe). A second PR touching the same function would conflict head-on and force one of them to be redone. Better to land #836 and then fix this on top of whatever that function ends up being.

Worth deciding as part of that: whether shopt -s inherit_errexit belongs at the top of these scripts generally, since the same swallow applies to every other $(...) in them, or whether this one call site should check explicitly. The blanket option is the smaller diff but changes the failure behaviour of substitutions that currently tolerate a non-zero intermediate.

Found by the nightly sweep while resolving merge conflicts on #836; noted there too.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions