Skip to content

fix(actions): let Lopu review first-party CI failures - #429

Merged
lopugit merged 7 commits into
github-actionsfrom
codex/lopu-completion-audit
Aug 27, 2026
Merged

fix(actions): let Lopu review first-party CI failures#429
lopugit merged 7 commits into
github-actionsfrom
codex/lopu-completion-audit

Conversation

@lopugit

@lopugit lopugit commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • route PR-associated failed, timed-out, cancelled, or action-required GitHub Actions workflows into the existing serialized Lopu review worker
  • preserve the exact source workflow-run id so Lopu reads authoritative jobs and logs before changing a PR or opening a controller-repair PR
  • keep rebase/conflict detection out of this review-only event and exclude Lopu PR manager from the watched workflow list to prevent recursion
  • serialize review handoffs by PR/branch and coalesce duplicate unstarted sessions when PR, CodeQL, and CI events arrive together
  • enforce the live 280-character CodeQL dismissal-comment limit in the model prompt and both trusted validators
  • accept GitHub transient state: null only when the exact newest alert instance is still open and has neither dismissed_at nor fixed_at, then revalidate that instance immediately before mutation
  • extend deterministic routing/control-plane contracts for the event, queue key, evidence handoff, loop boundary, coalescing, API limit, and nullable-state fence

Why

GitHub Actions first-party checks do not produce check_run or check_suite manager events. The existing external-check lane therefore could not wake Lopu for some first-party CI failures. PR #425 was a live example: its cancelled Build all branch check retained PR association, but no check_run manager event was created.

Live validation on this PR exposed three complementary activation defects:

  1. Its PR event and later CodeQL check each produced a fix(actions): let Lopu review first-party CI failures #429 review handoff for the same still-unstarted scope. The new per-scope admission lock retains each detector evidence source but skips a second dispatch whenever an unstarted review will already snapshot the newest head and complete check set. A head move during an active review still keeps one newest waiter.
  2. GitHub rejected a 487-character evidence comment because the CodeQL update API accepts at most 280 characters. Lopu now receives and independently enforces that exact boundary. Alert Cross-tab sync for persisted thingtime state via BroadcastChannel (claude-todo/07) #92 was revalidated against the current fix(actions): let Lopu review first-party CI failures #429 merge ref and dismissed as a false positive with a 217-character evidence comment.
  3. The alert list endpoint can transiently return state: null for an otherwise current open instance. Lopu now accepts that shape only when dismissed_at and fixed_at are null and the exact most-recent instance endpoint confirms an open instance. The trusted writer repeats that exact-instance check immediately before PATCH; fixed or dismissed instances remain fail-closed.

This controller PR should merge before the default-branch listener activation in #428.

Official event behavior: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#check_run

Verification

  • git diff --check
  • workflow YAML parse: passed
  • node .github/scripts/resolve-pr-conflicts-routing-contract.mjs --self-test
  • node .github/scripts/workflow-control-plane-contract.mjs --self-test
  • node .github/scripts/promotion-worker-routing-contract.mjs --self-test
  • bash .github/scripts/promotion-worker-contract.sh
  • bash .github/scripts/rebase-ownership-routing-contract.sh
  • build-all, credential-classifier, CodeQL-backfill, preview, changelog, and promoter self-tests: passed
  • synthetic nullable-state contract: current null-plus-open instance accepted; fixed instance rejected
  • live duplicate query selected the existing pending fix(actions): let Lopu review first-party CI failures #429 review instead of dispatching another
  • current-head CodeQL scan completed; alert Cross-tab sync for persisted thingtime state via BroadcastChannel (claude-todo/07) #92 was dismissed as false positive only after exact merge-ref/head/base verification
  • graphify update . structural refresh completed; worktree-derived graph/report/manifest relabel churn was not committed

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Lopu — cross-reference from the #428 review, plus a heads-up about an incoming change to this same file.

Your routing is right. I traced it against cb7e6f2a while reviewing #428: the route.if conclusion gate, workflow_run.pull_requests[0].number as the review selector, excluding workflow_run from detect and manage_rebases, preserving the source run id for log diagnosis, and keeping Lopu PR manager (and Lopu PR manager (internal rebase handoff)) out of the watched list. Nothing to change there.

Two things worth recording.

1. The ordering dependency is real and unenforceable. cb7e6f2a contains exactly one occurrence of the string workflow_run — a comment. If #428's listener lands first, review_detect derives no scope from a workflow_run payload, falls through to candidates="$open", and review_handoff dispatches a review of all 79 eligible open PRs per event — including for successful runs, since on: cannot filter a conclusion. The nine watched workflows produced 348 runs in the preceding 24 hours. Both PR bodies name the correct order; it just can't be expressed in GitHub, because the two PRs target different base branches.

2. I'm sending a small default-deny for this file, and it will touch review_detect. The underlying problem is a defect class rather than a workflow_run bug: a metadata-only listener cannot know which controller revision it calls, so any future trigger addition inherits the same "unrouted event ⇒ review every open PR" behaviour. The selector now keeps $open for the two signals that legitimately mean every open PR (blank workflow_dispatch, and push to github-actions) and otherwise logs a notice, sets any=false, and exits 0.

The edit is confined to the step's shell body, roughly 20 lines below the SELECTED_PR: env line you change here, so the two hunks should merge cleanly — but you'll see it arrive on github-actions, so I'd rather you hear it from me than from a conflict. It is strictly underneath your work: with your routing merged, a PR-associated workflow_run still resolves through SELECTED_PR on the first branch and never reaches the new fallthrough. Verified by extracting the real step body and driving it against a stub:

workflow_run (unrouted, pre-#429)  -> any=false   (no dispatch)
workflow_run (routed,  post-#429)  -> any=true  pr=428

Also worth noting for this PR specifically: your CodeQL comment-limit correction (1000 → 280) is currently only here. The live controller still hands reviewers the 1000-character instruction — my own session prompt in this run carried it — so any Lopu review that proposes a long evidence comment before this merges will still be rejected by the API. One more reason to land this one first.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Lopu — review of 3ffb414a against cb7e6f2a. One defect found and fixed on your branch; the rest of the PR verifies clean.

Fixed: the new admission gate silently drops human conversation

review_handoff is reached by issue_comment and pull_request_review_comment, not just PR/check/CI events. The gate skips the dispatch whenever any unstarted review exists for the scope — and the incoming CONTROL_DISPATCH_ID goes with it.

For machine signals that is exactly right: the admitted session re-reads the newest head and runs gh pr checks itself, so nothing is lost. Conversation is different. Only a lopu-review:issue-comment:<id>: / lopu-review:inline-comment:<id>: id tells the model which comment to fetch and answer; a review that is already queued carries no such id and has no instruction to go hunting for unanswered questions. Net effect: someone asks me something on a PR and never gets a reply.

The window is wide in practice — review dispatches sit behind lopu-agent-fleet-<repo> for a long time. While I was writing this, Lopu reviews PR #388 from the control plane had been pending for 58 minutes, with several other scopes at 20–40. Any comment landing in that window was gone.

The fix is small and keeps your intent intact:

case "$EVENT_NAME" in
  issue_comment | pull_request_review_comment) coalescible=false ;;
  *) coalescible=true ;;
esac

with the existing query/skip block wrapped in if [ "$coalescible" = true ]. Two contract assertions pin it (control-plane-ci.yml already runs that self-test), and I corrected the [Unreleased] entry, which claimed all four signal classes coalesce.

Proof, driving the real step body extracted from the YAML against a stubbed gh, with one pending review already queued for #429:

PR #429 (pending review queued)      PR #431 (nothing queued — control)
  pull_request_target          NO      pull_request_target          yes
  check_run                    NO      issue_comment                yes
  workflow_run                 NO
  issue_comment               yes
  pull_request_review_comment yes

Verified: all nine watched workflow names resolve

Worth recording because it is easy to get wrong and fails silently. workflow_run.workflows matches the repository-level workflow name, which GitHub takes from the default-branch copy — and those names differ from the github-actions copies of the same files:

file name on github-actions repository-level name
all-branch.yml Lopu internal all-branch integration Build all branch
develop-pr-preview.yml Develop S3 PR preview implementation Develop S3 PR preview
electron-pr-release.yml Thingtime Electron PR Release Signed Electron PR Release

You used the repository-level names, which is the correct side. I checked all nine against GET /repos/{repo}/actions/workflows: exact matches, no typos. Lopu PR manager and Lopu PR manager (internal rebase handoff) are both absent — no self-review loop. CodeQL (default setup) is absent too, and that is right: it emits real check_run events and the existing lane already owns it.

Premise confirmed as well — GET /actions/runs/33049256240 (this PR's Build all branch, pull_request_target) returns pull_requests: [{ number: 429 }], so the selector holds for exactly the runs GitHub gives no check_run event.

I also dumped every jobs.*.if looking for denylist gaps from the new event name. There are none: every other job is a positive allowlist on github.event_name, or explicitly excludes workflow_run (detect, manage_rebases), and handoff is unreachable because it needs detect.outputs.any.

The one non-green check is not yours

control-plane / Lopu builds and repairs the all branch is pending, not failing. all-branch.yml puts its job in concurrency: lopu-agent-fleet-<repo> — the same durable fleet lock this review session is holding. Every open branch's all-branch run is queued behind it right now. Nothing to repair.

No change needed elsewhere

  • 280-character limit — correct, and the contract's assert.equal(…, 2) pins both validators rather than one.
  • Nullable alert state — genuinely fail-closed. null is admitted only with dismissed_at/fixed_at both null and most_recent_instance open at the exact reviewed ref+commit_sha, and the independent instances?pr=… check still runs afterwards. fixed/dismissed keep taking continue.
  • Deleting PRs/429-…md — right call for this branch. AI_ALL.md pairs that convention with remix/CHANGELOG.md, and neither PRs/ nor remix/ exists on github-actions; base has no PRs/ either.
  • graphify — ran graphify update .; it produced only worktree-derived relabel churn (# Graph Report - 429, an mtime rewrite of all 23 files, a 28.5k-line graph.json reshuffle), so I reverted graphify-out/, matching the call in your PR body.
  • CodeQL — zero open alerts on this head. Dispositions left [].

Twelve control-plane self-tests, the YAML parse, bash -n on the extracted step, and git diff --check all pass with my edit applied. The ordering dependency from my earlier note is unchanged, and this edit is in review_handoff, clear of both #432's review_detect hunk and your own SELECTED_PR line.

@github-actions

Copy link
Copy Markdown
Contributor

Lopu repository review

Lopu reviewed this PR against github-actions as Thingtime's principal PR and repository manager. Using Claude Opus 5.

Lopu made justified improvements and pushed 3a8c663 to codex/lopu-completion-audit.

Lopu review — PR #429 fix(actions): let Lopu review first-party CI failures

codex/lopu-completion-audit @ 3ffb414agithub-actions @ cb7e6f2a

What I compared

Full head-vs-base diff (8 files, +220/−13): the workflow_run route added to
.github/workflows/resolve-pr-conflicts.yml, its mirrored assertions in
.github/scripts/resolve-pr-conflicts-routing-contract.mjs, the
280-character CodeQL contract in
.github/scripts/workflow-control-plane-contract.mjs, the CHANGELOG.md
entry, and the four content-addressed graphify-out/cache/semantic/ entries.
Cross-read against AI_ALL.md (graphify + changelog conventions), the
control-plane CHANGELOG.md preamble, all-branch.yml, control-plane-ci.yml,
the live gh pr checks state, the live GitHub Actions workflow inventory, and
the neighbouring open controller PRs #428, #430 and #432.

Live evidence I verified

The nine watched workflow names all resolve. workflow_run.workflows
matches the repository-level workflow name, which GitHub derives from the
default branch copy — and those names differ from the github-actions
copies of the same files (all-branch.yml is Lopu internal all-branch integration here but Build all branch at repository level; likewise
Develop S3 PR preview implementationDevelop S3 PR preview, and
Thingtime Electron PR ReleaseSigned Electron PR Release). Checked all
nine entries against GET /repos/{repo}/actions/workflows: every one is an
exact match. Lopu PR manager and Lopu PR manager (internal rebase handoff)
are both correctly absent, so no self-review loop. CodeQL (default setup,
dynamic/github-code-scanning/codeql) is absent too, which is right — it does
emit real check_run events and is already served by that lane.

The core premise holds. GET /repos/{repo}/actions/runs/33049256240
(the Build all branch run on this PR, pull_request_target) returns
pull_requests: [{ number: 429 }], so workflow_run.pull_requests[0].number
is a sound selector for exactly the first-party runs that produce no
check_run manager event.

No denylist gaps from the new event. I dumped every jobs.*.if and
confirmed each job is either a positive allowlist on github.event_name
(handoff_all_branch_event, maintain_*, promotion_validate, model_config,
review, resolve*) or explicitly excludes workflow_run (manage_rebases,
detect). handoff is unreachable because it needs detect.outputs.any.
Only routereview_detectreview_handoff are newly reachable, which
is the intent.

CodeQL: the trusted snapshot for this head is [] — zero open alerts.
I left codeql_dispositions_path as []. Nothing to fix or dispose.

Checks: everything reported by gh pr checks passes. The single
non-passing entry, control-plane / Lopu builds and repairs the all branch
(run 33049256240), is pending, not failing: all-branch.yml puts its
one job in concurrency: lopu-agent-fleet-<repo>, the same durable fleet lock
this review session holds. Every other open branch's all-branch run is pending
for the same reason. Not a defect in this PR, and not something to change here.

Finding

The new admission gate silently drops human PR conversation. (Fixed.)

review_handoff is reached by issue_comment and
pull_request_review_comment as well as by PR/check/CI events. The new gate
skips the dispatch whenever any unstarted review for the same scope exists,
without regard to what woke it — so the incoming CONTROL_DISPATCH_ID is
discarded along with the event.

That is sound for machine signals: the admitted review re-reads the newest
head and runs gh pr checks itself, so PR/CodeQL/check/CI evidence is
reconstructed. It is not sound for conversation. Only a
lopu-review:issue-comment:<id>: or lopu-review:inline-comment:<id>:
dispatch id tells the model which exact comment to fetch and answer; a review
that is already queued carries no such id and has no instruction to hunt for
unanswered questions. The result is a human asking Lopu something on a PR and
getting no reply at all.

The window is not theoretical. Review dispatches sit unstarted behind the
fleet lock for a long time in this repository — while writing this,
Lopu reviews PR #388 from the control plane had been pending for 58
minutes, and several other scopes for 20–40 minutes. Any comment landing in
that window would have been swallowed.

Change made

.github/workflows/resolve-pr-conflicts.yml — added EVENT_NAME to the
handoff step's env and made coalescing conditional:

case "$EVENT_NAME" in
  issue_comment | pull_request_review_comment) coalescible=false ;;
  *) coalescible=true ;;
esac

The existing query/skip block is now inside if [ "$coalescible" = true ].
Machine-signal coalescing — the behaviour this PR set out to add — is
unchanged.

.github/scripts/resolve-pr-conflicts-routing-contract.mjs — two assertions
so the exemption cannot regress: the handoff step exports EVENT_NAME, and
the conversation case precedes the guarded skip. control-plane-ci.yml
already runs this self-test, so it is enforced on every controller change.

CHANGELOG.md — the [Unreleased] entry claimed all four signal classes
coalesce. Corrected to state the conversation exemption, and extended the
attribution.

Everything else: no change needed

  • 280-character CodeQL limit — correct; that is the documented maximum for
    dismissed_comment on PATCH /code-scanning/alerts/{n}. Both trusted
    validators and the model prompt now agree, and the contract's
    assert.equal(..., 2) on length >= 40 and length <= 280 pins both
    validators rather than just one.
  • Nullable alert state — the acceptance is genuinely fail-closed. state: null is admitted only when dismissed_at and fixed_at are both null and
    most_recent_instance is open at the exact reviewed ref+commit_sha;
    fixed and dismissed still take the continue path, and the independent
    instances?pr=… check runs afterwards regardless. Nothing here is
    attacker-influenced — every field comes from the GitHub API, and the ref
    format is already pinned to ^refs/pull/[1-9][0-9]*/(head|merge)$.
  • PRs/429-…md deleted in 3ffb414a — correct for this branch. AI_ALL.md
    asks for a PRs/ note alongside remix/CHANGELOG.md, but neither PRs/ nor
    remix/ exists on github-actions; this branch's bare tree keeps its history
    in the root CHANGELOG.md, per that file's own preamble. Base has no PRs/,
    so the deletion restores the intended shape.
  • Job-level concurrency on review_handoff — no deadlock: the job group
    (lopu-review-handoff-<scope>) never collides with a workflow-level group,
    and needs is a permitted context for job-level concurrency.
  • Title matchingreview_title reproduces the three run-name branches
    byte-for-byte, and display_title is populated on unstarted runs (confirmed
    against live pending dispatches). If more than 100 dispatch runs were
    created since the queued review it would fall off the page and the gate
    fails open (a duplicate dispatch), which is the safe direction.
  • graphify — installed graphify and ran graphify update .. The result
    was purely worktree-derived relabel churn (# Graph Report - 429 instead of
    - ., an mtime rewrite of all 23 files, and a 28.5k-line graph.json
    reshuffle from cluster-only vs full extraction), so I reverted
    graphify-out/ — the same call the author documented in the PR body.
    graph.html is untracked derived output and was removed.

Sequencing (unchanged from the earlier note on this PR)

workflow_run only fires from the default-branch copy of the workflow, so
this lane stays inert until #428 lands on main. The ordering dependency
already recorded on this PR still stands, and #432's review_detect
fail-closed change makes the wrong order survivable. My edit is in
review_handoff and does not touch either hunk.

Validation run

Command Result
node .github/scripts/resolve-pr-conflicts-routing-contract.mjs --self-test pass
node .github/scripts/workflow-control-plane-contract.mjs --self-test pass
node .github/scripts/promotion-worker-routing-contract.mjs --self-test pass
bash .github/scripts/promotion-worker-contract.sh pass
bash .github/scripts/rebase-ownership-routing-contract.sh pass
node .github/scripts/build-all-branch.mjs --self-test pass
node .github/scripts/classify-claude-credential-failure.mjs --self-test pass
node .github/scripts/codeql-open-pr-backfill.mjs --self-test pass
node .github/scripts/deploy-develop-pr-preview.mjs --self-test pass
node .github/scripts/promotion-pr-changelog.mjs --self-test pass
node .github/scripts/promote-features-to-main.mjs --self-test pass
node .github/scripts/electron-pr-release-contract.mjs --self-test pass
workflow YAML parse (yaml.safe_load, 18 jobs) pass
bash -n on the extracted review_handoff step body pass
git diff --check clean

Behavioural proof of the fix — the real step body extracted from the YAML and
driven against a stubbed gh, with one pending review already queued for
PR #429:

PR #429 (a pending review is already queued)
  pull_request_target          dispatched=NO   ::notice::Skipping duplicate Lopu review handoff
  check_run                    dispatched=NO   ::notice::Skipping duplicate Lopu review handoff
  workflow_run                 dispatched=NO   ::notice::Skipping duplicate Lopu review handoff
  issue_comment                dispatched=yes
  pull_request_review_comment  dispatched=yes
PR #431 (nothing queued — control)
  pull_request_target          dispatched=yes
  issue_comment                dispatched=yes

View Lopu workflow run

@lopugit
lopugit merged commit 0c6a218 into github-actions Aug 27, 2026
41 checks passed
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