Skip to content

fix(ci): strip queue labels when an issue closes - #12865

Merged
kyle-sexton merged 9 commits into
mainfrom
fix/strip-queue-labels-on-close
Nov 3, 2026
Merged

kyle-sexton merged 9 commits into
mainfrom
fix/strip-queue-labels-on-close

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: hand-closed issues keep queue labels (ledger L105, e.g. #12583)

Summary

An issue closed by hand kept agent-ready, status: ready and admission-deferred. Example: #12583, closed with "Fixed by #12585". In a sample of 24 closed issues that still carry agent-ready, 19 were closed by a listed human and 5 by the lanes App. Before this change, the only code that removed both labels was work-unblock's lane-PR hand-off. That hand-off runs only on an open issue whose lane PR merged with Refs: or closed unmerged.

Fix

  • work-unblock.yml already runs on issues: [closed]. Its label-unblocked.mjs close path now strips the queue labels from the closed issue itself (stripClosed), so no new workflow, job or token mint is added.

  • It reads the issue fresh and removes only the queue labels present, with agent-ready last. A 404 counts as removed.

  • An issue that is open again by the time the run reads it keeps its labels.

  • A post-merge fix issue keeps its labels, matched by github-actions[bot] as author plus a <!-- post-merge-fix check=... --> first line. post-merge-demotion-alert.yml reopens such an issue with agent-ready kept, and the sweep relabels it.

  • The close strip writes nothing to the Work status field, because none of that field's states fits a closed issue.

  • Where the lanes App closes an issue itself, it strips the labels right after the close, with the token it already holds. Its close starts no close run, so the close path above never sees it.

    • report-outcome.mjs: the close route (already-done, report-delivered), reading the labels from the close's own answer.
    • label-unblocked.mjs closeDoneParent: the split-parent close.
  • One helper: stripQueueLabels and its list (ADMISSION_LABELS) live in report-outcome.mjs, which label-unblocked.mjs already imports. The close path, both App close sites and the lane-PR hand-off all use it. The hand-off now also removes admission-deferred when present, and sends no DELETE for a status: ready it does not carry. report-outcome.mjs's API errors now carry their HTTP status, so a 404 counts as removed there too.

  • Left as they are:

    • post-merge-demotion-alert.yml's close of a fix issue, which uses GITHUB_TOKEN and must keep agent-ready for the reopen.
    • The maintenance signal and digest closes in monitor.mjs and file-findings.mjs. They already drop agent-ready, and admission never gives those issues status: ready.
  • Token: unchanged. The close run already gets a lanes App token from the token broker (request-lane-token, lane work-unblock, activity label-unblocked), limited to this repository with the grant resolved from docs/conventions/pr-pipeline.yaml. The step only runs when the closer and the re-runner are listed humans.

  • Trigger safety: an App label removal fires issues: unlabeled. The workflows that listen for it do nothing on a closed issue:

    • intake-reenter.yml requires github.event.issue.state == 'open'.
    • security-outside-actor.yml acts on an unlabel only for security: outside-actor.
    • work-resolve-issue.yml triggers on labeled only.
    • pr-merge.yml's unlabeled trigger is pull_request_target.

    A label DELETE never reopens an issue.

  • Merge closes: GitHub closes an issue when a pull request the lanes App merged lands, and no close run follows a bot's close. work-unblock's existing sweep already walks each issue a listed human or a merge closed in its window, after every work-resolve-issue run and hourly. It now strips their queue labels there with stripClosed, so this adds no new poller.

Verification

  • node --test on label-unblocked.test.mjs and report-outcome.test.mjs: 681 of 681 pass. The new tests cover:
    • a closed issue losing only the queue labels it carries;
    • no write for an issue without them;
    • a 404 treated as success;
    • a failed removal failing the run;
    • a reopened issue kept as is;
    • the post-merge fix-issue exemption;
    • a hand-off removing only the labels present, admission-deferred included;
    • a split-parent close removing the labels present, agent-ready last;
    • report-outcome's close removing the labels present after the close, agent-ready last, with no write when none is left and a 404 counted as removed;
    • the sweep stripping a merge-closed issue's queue labels, agent-ready last, with no write for one without them or for a close by neither a listed human nor a merge.
  • actionlint on work-unblock.yml: clean, apart from the existing concurrency.queue key, which the local actionlint 1.7.12 does not know.
  • runner-policy.mjs --repository-visibility public: passed.
  • scripts/check-affected.sh --base origin/main --no-fetch: 10 passed, 5 skipped, after merging main (the first run failed stale-base-overlap on perf(lanes): configure work-unblock's stranded re-fire backoff per cause and shorten it for transient causes #12804, now merged in).

Found

Related

🤖 Generated with Claude Code

kyle-sexton and others added 6 commits November 3, 2026 11:10
A hand-closed issue kept agent-ready, status: ready and
admission-deferred, so queue counts and listings still read it as
waiting. work-unblock's close run now removes those labels from the
closed issue itself, reading it fresh: only the labels present come
off, agent-ready last, and a 404 counts as removed. An issue open again
by the time the run reads it, or a post-merge fix issue (which keeps
agent-ready for its reopen on a repeat failure), keeps them.

The list is report-outcome's ADMISSION_LABELS, now exported, and the
lane-PR hand-off to triage removes its labels through the same helper,
so both paths share one source.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s-on-close

# Conflicts:
#	.github/scripts/work-unblock/label-unblocked.mjs
An App close starts no work-unblock close run, so the issue kept its
queue labels. The two places the App closes an issue now strip them
right after the close, with the token each already holds:
report-outcome's close route (already-done, report-delivered) and
work-unblock's split-parent close. stripQueueLabels and the list move
to report-outcome.mjs, which label-unblocked already imports, so every
path shares one helper; report-outcome's API errors now carry their
HTTP status so a 404 counts as removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub closes an issue when a pull request the lanes App merged lands,
and no close run follows a bot's close, so such an issue kept its queue
labels. work-unblock's existing sweep, which runs after every
work-resolve-issue run and hourly, already walks each issue a listed
human or a merge closed in its window; it now strips their queue labels
there too, through the shared helper, with the same exemptions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a sweep pass, each closed issue was read three times: by the close
check, by the label strip, and by the split-parent pass. The close
check (readClose) now caches the issue it read beside its verdict; the
strip takes that read, and the split-parent pass shares the same cache,
so a merge-closed issue is read once per run. The close-event path keeps
its own fresh read, since that job can run after a reopen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 16:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T16:29:18.924612Z 4949017 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 911b5906235da2f63fc1da601503378ad8fdf18d: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12865, incremental from f7bbe7af to 911b5906

I found no security issues. This review covers only the push since f7bbe7af. I read every hunk of both files in scope: report-outcome.mjs (a trusted path) and report-outcome.test.mjs.

What changed

On the close route in finishStop, stripQueueLabels now gets a small wrapper around the API client. The wrapper marks every request it sends safeToResend: true, so a label DELETE that gets a 5xx is sent again by the retry logic in retry-fetch.mjs. That retry logic is unchanged. The tests now expect three tries before the run fails.

What I checked

  • The wrapper only reaches label removals. stripQueueLabels (report-outcome.mjs#L686-L697) calls the client only for DELETE ${base}/labels/<name>.
    • The names come from the constant QUEUE_LABELS and are passed through encodeURIComponent.
    • The wrapper drops the request body, which is harmless because a DELETE sends none.
    • No other caller gets the resend wrapper. The fresh label GET and the PATCH close still use the plain client, and the PATCH was already resendable.
  • Resending is idempotent. Removing a label twice changes nothing more, and a 404 still counts as removed. Retries are capped by the unchanged retryFetch.
  • Failure order still holds. A removal that keeps failing still throws before the PATCH close, so the issue stays open with agent-ready for work-unblock's sweep. This is the fix described in the resolved thread on this file. The new resend is one of the fixes that thread's own finding proposed, and it does not re-open that point.
  • Token, triggers and trust controls: unchanged. No token request, permission, trusted-actor check, kill-switch read or merge hold is touched.
  • Instruction surfaces and plugin manifests: this push touches neither.

Spend: at most two extra GitHub REST DELETE calls for each label removal that gets a 5xx. There is no new run, no model call and nothing billable.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 494901751d238039bd1b86763621b3235327c7c8; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 911b5906235da2f63fc1da601503378ad8fdf18d: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review: incremental, f7bbe7a → 911b590

Scope: I read every hunk of the 2 files this push changed: .github/scripts/work-resolve-issue/report-outcome.mjs (a trusted path, given extra scrutiny) and report-outcome.test.mjs. The other files are unchanged since f7bbe7a, which this lane already reviewed.

Findings: none.

Prior finding resolved

The advisory finding at discussion_r4458535389 said the close route's label DELETEs were not resent after a 5xx. This push fixes that:

  • report-outcome.mjs L2230-L2231 wraps github in a local resend that passes { safeToResend: true }, and hands that wrapper to stripQueueLabels.
  • stripQueueLabels (L686-L697) only ever calls github("DELETE", path) with two arguments. So the wrapper dropping body changes nothing, and the shared helper still works with label-unblocked.mjs's client, which takes different arguments.
  • api() (L1896-L1913) passes safeToResend on to retryFetch. Per the header of retry-fetch.mjs, that client makes at most 3 attempts in all.
  • Resending is safe here. If a DELETE that answered 5xx had in fact gone through, the retry answers 404, and stripQueueLabels counts a 404 as removed.
  • The labels are still removed before the PATCH that closes the issue. A removal that fails on every try still leaves the issue open with agent-ready, so work-unblock's sweep can find it.

Tests

Both 502 tests now expect FAILED_STRIP: three DELETE /labels/status%3A%20ready calls, then a nonzero exit and no PATCH. One is the close route test; the other is the verified-done close test. That is the 3-attempt limit, and it shows the DELETE is now resent. I did not run the tests.

Notes (not counted)

  • The PR description is still out of date (noted last round). It says report-outcome strips the labels "right after the close", reading them "from the close's own answer". The code now strips before the close, from a fresh GET, and resends a DELETE after a 5xx.
  • Closing issue: the PR closes none, so there are no acceptance criteria to check.
  • Spend: a failing label DELETE can now be sent up to 3 times, on the existing lanes App token. This adds no paid resources.

kyle-sexton and others added 2 commits November 3, 2026 11:54
report-outcome's close route and closeDoneParent stripped the queue labels
after their PATCH, so a failed label DELETE (not resent on a 5xx) left a
closed issue with agent-ready that no sweep reads. Both now strip first; a
failed removal fails the run with the issue still open and agent-ready on
it, for work-unblock's sweep.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s-on-close

Compose #12710's verify-and-close flow for a finished split parent with
this branch's strip-before-close rule:

- report-outcome finishStop keeps main's verifyClose argument; its
  verified-done close goes through the same close route, so the queue
  labels come off on a fresh read before the PATCH. A new test covers
  that order and a 502 on a label DELETE leaving the issue open.
- label-unblocked closeSplitParents and closeSplitParent take both the
  shared close-check cache and main's mirror; main keeps the stripClosed
  kick pass and passes closes and mirror together.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on f7bbe7af506b69ebcf983b56cd94d7f08da5b880; the summary is in this lane's pull request comment.

claude-lane: claude-review

A label DELETE is idempotent and a 404 already counts as removed, so the
close route sends its removals as safeToResend. One transient 5xx no
longer fails a finished stop and sends the issue back for another lane
run; a removal that keeps failing still fails the run before the close.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 31dda9e Nov 3, 2026
64 checks passed
@kyle-sexton
kyle-sexton deleted the fix/strip-queue-labels-on-close branch November 3, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant