Skip to content

feat(pr-pipeline): file fix issues for red-main checks and send them to intake - #8805

Merged
kyle-sexton merged 9 commits into
mainfrom
feat/7535-red-main-fix-issues
Oct 21, 2026
Merged

kyle-sexton merged 9 commits into
mainfrom
feat/7535-red-main-fix-issues

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Closes #7535

Summary

When a full pr-require-checks run on main fails, the alert job already files a needs-human demotion record. It now also files one fix issue per newly failing check and dispatches intake triage for each, so a red main reaches the intake lane instead of waiting on a person to open issues. Built to the owner's answers on #7535 (relayed 2026-10-21): D12 (a), Q1-Q6, with Q6 changed to reopen. Owner decision 2026-10-21: a reopened fix issue keeps agent-ready and is not re-triaged, so a repeat goes straight back to agents; first failures still wait for approval per D12 (a); the record wording stays "failure record #N".

Fix

  • One fix issue per newly failing job, after the record POST (which now returns the record number), named for the job only (job names come from the workflow file on main; annotation titles, which any step can print, never reach a fix issue). Title Fix main: <check> fails since <sha12>, first line <!-- post-merge-fix check=<check> -->, fixed text only: check name, SHA, run link, merge range (<base12>..<sha12> or "unknown"), and the record's number. No label, no closing keyword, no commit subject or annotation text (Q1, D12 (a)).
  • Validator for the only free-text input, the check name: ^[A-Za-z0-9][- A-Za-z0-9._:,/()]{0,99}$ and no babysit-merge-demotion / post-merge-failure substring; every other input is already hex or numeric-checked (Q2: bot-authored intake approved, gated on Security & Cost passing this template and validator).
  • At most 5 per run, with an overflow line on the record (Q3); post-merge-failure records file too, worded "failure record #N" since that record demotes nothing (Q4); filing only alongside a new record (Q5).
  • A check whose fix issue was closed reopens that issue: comment with the new range, then reopen, keeping agent-ready and dispatching no intake, mirroring fix(ci): reopen a monitor's closed issue for its key instead of filing a recurrence #8786's one-issue-per-key monitors; closed as not planned or duplicate mutes the check; an open one stands (Q6).
  • Each fix issue is locked before intake triage is dispatched (first filing; a reopen dispatches nothing), so no outsider comment reaches the triage prompt; a failed lock sends nothing.
  • Any fix issue that could not be filed, locked, reopened or dispatched is named in one comment on the needs-human record, since a re-run takes the existing-record path and cannot retry; then ::error and exit 1 after every check is handled. Canary runs file nothing. No permissions:, on: or token change.
  • README: one paragraph on the fix issues beside the demotion record.

Verification

  • bash scripts/post-merge-demotion-alert.test.sh: 100 pass, 0 fail; the new or extended cases fail against the old workflow (including the lock, job-name and undelivered-record cases against c760e24); 8 mutants (charset check removed, cap 5 to 9, open-skip, mute regex, needs-human label dropped, marker injected, 404 branch, newly gate) were each killed before the reopen change removed the 404 branch. The reopen case (comment then reopen, no DELETE, no lock, no intake dispatch) fails against c58da79 and passes at da2a392.
  • Report-job node tests: 43 pass. scripts/check-affected.sh --base origin/main: passed; actionlint passed.
  • Intake dispatch gate checks only the sender's actor id, and github-actions[bot] (41898282) is listed (ci-workflows intake-triage.yml at the pinned sha, :104-117, :250-271).
  • Left out: a live red-main run, and what intake returns for a bot-authored issue in this template. work-resolve-issue.yml triggers on issues: labeled only, so a reopen that keeps agent-ready starts no hosted run by itself.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01B4THuN6Tafisi3H5ndrim8

kyle-sexton and others added 4 commits October 20, 2026 23:47
…main run

Beside a new demotion or post-merge-failure record of a red scheduled or
dispatched full run, post-merge-demotion-alert.yml files one fix issue per
newly failing check (at most 5, an overflow line on the record otherwise)
and dispatches intake-triage.yml for each. The issue's text is fixed: a
check name in a checked charset, hex SHAs, the run link built from its
numeric id, and the record's number; no label, no closing keyword, and
neither record marker. One issue per check across SHAs: an open one stands,
a closed one is reopened with the new range in a comment and agent-ready
removed, and one closed as not planned or a duplicate mutes the check. A
failed dispatch or filing fails the job after every check is handled.

Refs: #7535

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4THuN6Tafisi3H5ndrim8
A closed fix issue that no longer carries agent-ready answers its removal
with a 404 and is still reopened; any other failure leaves it closed, fails
the job, and the other checks still file.

Refs: #7535

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4THuN6Tafisi3H5ndrim8
…on record

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4THuN6Tafisi3H5ndrim8
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 21, 2026 03:55
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 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-10-21T03:57:27.291510Z c760e24 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 Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed da2a392613a5f8c952f1e2c325c64c21b5774349: 0 finding(s), 0 as inline comments.

No security issues found in PR #8805 (head da2a392). I reviewed the four changed files by reading the diff and the surrounding workflow; I did not run the test script.

  • Injection into the fix issues: The check name is the only variable text. It comes from the job name in a completed run on main, cut at the first ": ", so annotation titles never get in. It is then checked against ^[A-Za-z0-9][- A-Za-z0-9._:,/()]{0,99}$, which rules out >, backslash, quotes, $, backticks and newlines. The marker comment therefore can't be closed early, awk -v can't mangle the match, and the title and body can't carry markup or a closing keyword. Names containing either record marker are rejected, so a fix issue can't be taken for a demotion record. The run id, run attempt and record number are checked as numbers before they build the run link or an API path.
  • Spoofing the marker match: Fix issues are looked up only in github-actions[bot]'s own issues, so a user-planted marker can't mute or hijack a check. The not_planned/duplicate mute relies on the triage permission of whoever closed the issue. That is deliberate and documented.
  • Intake ordering: The issue is locked before the intake-triage dispatch, and a failed lock sends nothing, so outsider comments can't reach the triage prompt. The body holds only fixed text and the issue has no label, so the first filing waits for the owner's ratification.
  • Reopen path: A reopened issue keeps agent-ready and gets no second dispatch. The reopen comment is fixed text built from the SHA, the merge range and the run link. The only trigger is a failure on main's scheduled or dispatched full run, which an outsider can't cause without merge access. Returning a ratified issue to agents without a new ratification is the intended, documented behavior, so it is not a finding.
  • Permissions and triggers: The diff doesn't widen permissions: or change triggers, and the workflow's action pins are unchanged. That area is zizmor's, so I did not re-report it.

The diff touches no standing instruction file, so the instruction-surface deletion check did not apply and the exception register was not needed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c760e249f3

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed da2a392613a5f8c952f1e2c325c64c21b5774349: 0 finding(s), 0 as inline comments.

Code review: no findings

I read the full diff (4 files) and the PR context. I did not run scripts/post-merge-demotion-alert.test.sh, so its pass counts are the author's claims and I have not confirmed them.

Closing issue #7535. The change meets its criteria, based on reading the diff:

  • A separate fix issue is filed per newly failing check and sent to intake triage.
  • The demotion record keeps needs-human and its marker.
  • Closing a fix issue changes nothing on the record, which is the "separate fix item" constraint.
  • The new test cases cover each of these.

Checked and found sound:

  • Job permissions. issues: write covers lock, create, comment and reopen. actions: write covers the intake-triage.yml dispatch. The issue-number input exists in intake-triage.yml.
  • Untrusted text. The check name is the only free text reaching a fix issue. It is validated against the charset and the two record-marker substrings. The charset excludes backtick, >, [, @ and #, so it cannot close the HTML-comment marker or add mentions or links. Annotation text is cut off by ${check%%: *}.
  • Reading and filtering the issue list.
    • The extra state_reason // "none" column keeps read from collapsing tab-separated fields.
    • The awk lookup on the fourth column picks an open issue first, then the newest closed one.
    • A fix issue closed as not_planned or duplicate stays muted.
  • Failure handling. Failed lock, dispatch, reopen and file steps are collected, named in one comment on the record, and fail the job only after every check has been handled. dispatch runs in the main shell, so its fix_failed and undelivered updates persist.
  • Test stub. The numbering (900 plus the log line) matches what record_number and fix_number assume in the tests.

I considered whether ${check%%: *} would cut a job name that itself contains : . No job in pr-require-checks.yml has one, so I did not report it.

kyle-sexton and others added 2 commits October 21, 2026 00:04
…d report undelivered ones

Security & Cost and Codex findings on #8805:
- A gate title is any step's annotation text, so a fix issue now names
  only the failing job, whose name comes from the workflow file on main.
- Each fix issue is locked before intake triage is dispatched (and again
  on reopen), so no outsider comment reaches the triage prompt; a failed
  lock sends nothing.
- A re-run takes the existing-record path and never retries fix work, so
  any fix issue that could not be filed, locked, reopened or dispatched is
  named in a comment on the needs-human record before the job fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B4THuN6Tafisi3H5ndrim8
Comment thread docs/conventions/pr-pipeline/README.md Resolved
kyle-sexton and others added 3 commits October 21, 2026 00:21
…e fix-issue README line

The job-name test piped into grep -q, which pipefail-grep-q rejects
(lint-shell); it now reads the fix text into a variable first. The README
parenthetical read as banning the SHA, run link, range and record number;
it now lists what the issue carries and excludes only annotation titles.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A fix issue reopened because its check failed on main again keeps its
agent-ready label and is not dispatched to intake triage again, so a
repeat goes straight back to agents. First failures still file a new
issue that waits for the owner's ratification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit ff0f417 Oct 21, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the feat/7535-red-main-fix-issues branch October 21, 2026 15:03
github-merge-queue Bot pushed a commit that referenced this pull request Oct 21, 2026
#8897)

Closes #8896

## Summary

pr-merge starts a full `pr-require-checks` run after every merge with
`GITHUB_TOKEN`. That run's completion raises no `workflow_run` event, so
`post-merge-demotion-alert.yml` never saw a red one. Its record, its fix
issues (#8805) and the red-main report had to wait for the next
scheduled run, which could be up to about 17 hours later. This PR
implements option A from the issue. Red means every conclusion
`babysit_postmerge.py` counts as failed (`FAILED_CONCLUSIONS`:
`failure`, `timed_out`, `cancelled`, `startup_failure`,
`action_required`): the red main of 2026-10-21 05:01-13:34Z was runs
that concluded `cancelled` on a test-bash time-budget overrun, which a
failure-only send would have missed. Owner decision (b) below still
filters superseded-run cancellations once a newer run on main is green.

Owner decision (option b, extended to failed re-runs at the GitHub
Security & Cost P4 request): a dispatched red per-merge run, whether its
re-run failed or never finished within `red-main.confirm-wait-minutes`,
files nothing once a newer `pr-require-checks` run on this repository's
`main` has succeeded: the alert logs that main already recovered and
files no record, fix issue or report; if main is still red it files as
before, and the `workflow_run` path is unchanged.

## Fix

- **pr-merge.yml `dispatch-full-run`** (push wait step): when the run it
waited for concludes red (any of `FAILED_CONCLUSIONS`), the step
dispatches `post-merge-demotion-alert.yml` with `inputs[run-id]` and
`inputs[head-sha]`, the head SHA read from the run it waited on. It uses
the same `GITHUB_TOKEN`, and the job's existing `actions: write` covers
it, so no permission was added. A failed send fails the job. The merge
path, holds and re-run logic are unchanged. The header comment and the
remediation comment now describe this.
- **post-merge-demotion-alert.yml**
- Adds a `workflow_dispatch` trigger with two inputs, `run-id` and
`head-sha`.
- A new first step, `Read the failed run`, passes a `workflow_run`
event's fields through unchanged. On a dispatch it:
- requires `run-id` to be digits only and `head-sha` to be 40 lowercase
hex digits, before any read;
- accepts the dispatch only from `github-actions[bot]` (id 41898282) or
a listed human in `trusted-actors.json` on `main`, never the lanes App.
This follows the convention in `eval-browser-dispatch.yml`.
- reads `runs/<id>/attempts/1` and requires: path
`.github/workflows/pr-require-checks.yml`, event `workflow_dispatch`,
actor `github-actions[bot]`, `head_branch` main, this repository for
both the repository and the head, status completed, conclusion in
`FAILED_CONCLUSIONS`, and `head_sha` equal to the `head-sha` input. Runs
that raise their own `workflow_run` event (scheduled runs, runs a person
dispatched) are refused, so no run is processed twice.
- The filing step and `report-red-main` read the run from that step's
outputs and the `alert` job's outputs. The report job's lanes App sender
skip is unchanged.
- The filing step also files a dispatched `action_required` run; a
`workflow_run` event's `action_required` still files nothing.
`report-red-main` keeps `conclusion == 'failure'`: its culprit rule
(README "Red main", rule 1) confirms only a `failure`, so a dispatched
`cancelled` or `timed_out` red gets its record and fix issues but no
culprit report.
- For a dispatched bot run, the alert starts no re-run of its own and
waits up to `red-main.confirm-wait-minutes` (40) for a re-run.
pr-merge's `post-merge-dispatch` job re-runs a red run only inside a
lane merge's attribution window (`babysit_postmerge.py` ~:950-980); a
red with no lane merge to charge gets none, and the alert judges attempt
1 after the wait. Workflow_run behavior is unchanged, and the existing
bot-run test still passes. The job timeout is 50 minutes on a dispatch
and still 30 otherwise.
- The concurrency group stays keyed on signal and head SHA, as on
`main`: a dispatch uses `inputs['head-sha']`, so a dispatch and a
`workflow_run` event of the same signal and SHA share one group, and the
read-then-create record dedupe stays serialized.
- **Dedupe** already exists and is unchanged:
- records match by signal and SHA, or by failing run across both filers
(`babysit_postmerge._same_run`);
  - fix issues are one per check across SHAs;
  - report comments are one per run and attempt.
- The docs (README "Red main") and a changelog fragment are updated.

## Verification

- `scripts/post-merge-demotion-alert.test.sh`: 135 pass, 0 fail. Against
`origin/main`'s workflows, the 23 dispatch checks fail. The red-set
checks (dispatched `timed_out`, `cancelled`, `startup_failure`,
`action_required` read as red and filed; `success`, `skipped`, `neutral`
refused) failed 5 against e88af25 and pass on 7509e8c.
- `node --test` on `.github/scripts/pr-merge/workflow.test.mjs` and
`.github/scripts/post-merge-demotion-alert/*.test.mjs`: 54 pass. Against
`origin/main`'s workflows, the 4 dispatch tests fail; "every red
conclusion of a full run is sent" failed against e88af25 and passes on
7509e8c.
- `actionlint` passes on both workflows. `zizmor --offline` reports no
findings.
- The fields the read step checks match the real `attempts/1` response
for red run 39638634453 (path, name, event, actor id 41898282, branch,
repositories, status, conclusion).
- The other tests that read these workflows pass: `pr-merge/*`,
`pr-verify/workflow`, `maintenance-*`, 284 in all.
- **Not verified:** the `workflow_dispatch` path and the pr-merge send
can only run once this is on `main`. #8898 tracks that check.

## Open question

In the 7 days to 2026-10-21, 22 per-merge reds stopped at attempt 1, and
only 7 reached attempt 2. That is likely because post-merge-dispatch
re-runs only lane-dispatched runs; this is not verified. After this
change, each red with no re-run waits 40 minutes, then files a record
and fix issues from attempt 1, under the existing "re-run did not
finish" rule. The red-main report never confirms one. The options:

- **(a)** Accept this.
- **(b)** Re-run every red run that pr-merge dispatched. That changes
the re-run logic, so it is out of scope here.

## Related

- #8898: post-merge verification of the dispatch path
- #8805: fix-issue filing
- #7535: red-main fix issues
- pr-merge.yml is a merge-gate file, so the owner merges this change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

Route scheduled-run failure issues to an agent fix lane

1 participant