fix(actions): Lopu repairs failed PR checks

[lopugit]

Lopu controller check repair

Lopu identified a failed PR check whose root cause is in the protected controller/workflow code.

Lopu control-plane fix — CodeQL PR check times out when a push wins the race against PR creation

Symptom

On PR #427 (codex/search-commander-overhaul, head e5bcc813) the GitHub
Advanced Security CodeQL check run reported:

• conclusion: timed_out
• output.title: "No new alerts in code changed by this pull request"
• duration: 16s (06:17:20Z → 06:17:36Z)

Both Analyze (actions) and Analyze (javascript-typescript) jobs succeeded in
every workflow run, and the merge-ref analyses uploaded cleanly. Nothing in the
product diff is implicated: mergeStateStatus was UNSTABLE purely because of
this check.

Root cause

.github/workflows/codeql-analysis.yml elects a single analysis owner in the
scope job. The push branch of that election asks, exactly once:

open_prs="$(gh pr list --repo "$REPO" --state open --head "$GITHUB_REF_NAME" ... )"

That sample is taken within a few seconds of the push. When a branch is pushed
first and its PR is opened moments later — the normal agent workflow in
this repository — the push run sees no PR, declares itself the owner, and
uploads a refs/heads/<branch> analysis for a commit that has meanwhile become
an open PR head. The pull_request branch of the same election has no
reciprocal guard, so it always analyzes too.

Reconstructed timeline for PR #427 (all UTC, from the Actions + code-scanning
APIs):

time	event
06:16:34	push run 33045284962 created for e5bcc813
06:16:38–06:16:41	its Select one analysis owner job runs — no PR exists yet → analyze=true
06:16:43	PR #427 opened
06:16:53	pull_request run 33045304652 created (no push-side dedupe) → analyze=true
06:17:21	push run uploads /language:actions for refs/heads/codex/search-commander-overhaul
06:17:20	GHAS opens check suite 89528413157 + the CodeQL check on the PR head
06:17:36	GHAS closes it timed_out — only the actions category exists for that head
06:17:57	PR run uploads /language:actions for refs/pull/427/merge
06:22:09 / 06:22:33	both javascript-typescript analyses upload

GHAS opened the PR's check against the stray branch-head analysis and gave up
before the PR's own merge-ref analyses arrived.

Differential evidence

Every healthy open PR shows the mirror image — the GHAS check starts within a
second of the merge-ref actions analysis and passes in 1–3s, and none of
them has a refs/heads analysis at the PR head:

PR	GHAS check	branch-ref analysis at head?	why
#427	timed_out (16s), started 06:17:20	yes, 06:17:21	push run raced PR creation
#428	success (1s), started 06:27:35	no	push run 33045887786 correctly skipped (Analyze matrix job = skipped)
#426	success (3s), started 05:56:19	no	no racing push run
#429	failure (2s) — a genuine new alert, not this defect	no	—

PR #427 later received two more commits (42052bb2, e46a7fec) pushed while
the PR already existed. That push run skipped analysis and the GHAS check on the
new head is no longer timed_out — the same natural experiment, confirming the
race is the trigger rather than anything in the PR's content.

Fix

.github/workflows/codeql-analysis.yml, analyze job:

1. Added a Confirm this push still owns the analysis step between
   Initialize CodeQL and the analyze/upload steps. It re-runs the same
   ownership query at the last moment before upload and sets upload=false
   when an open PR now owns $GITHUB_SHA.
2. Gated Analyze the triggering revision on
   steps.ownership.outputs.upload != 'false'.
3. Added pull-requests: read to the analyze job permissions (the step needs
   it for gh pr list; the scope job already had it).

The check runs only for push events on the branch-ref path
(needs.scope.outputs.analysis_ref == ''), so pull_request runs and the
centrally dispatched merge-ref path are untouched — and when the step is
skipped, its empty output leaves the existing if semantics unchanged.

Placing it after init rather than at job start was deliberate: it widens the
race window it can absorb from a few seconds to the whole database-init period,
while still skipping the expensive query/upload phase. It never leaves a commit
unanalyzed — the PR's own run is the owner it defers to.

Validation

Run from this controller checkout:

• python3 -c "yaml.safe_load(...)" on the edited workflow — parses; step order
  and every if expression verified programmatically:
  • Confirm this push still owns the analysis → id: ownership,
    if: github.event_name == 'push' && needs.scope.outputs.analysis_ref == ''
  • Analyze the triggering revision →
    if: needs.scope.outputs.analysis_ref == '' && steps.ownership.outputs.upload != 'false'
  • analyze permissions now include pull-requests: read.
• node .github/scripts/workflow-control-plane-contract.mjs --self-test →
  workflow control-plane contract: self-test OK.
• bash -n on the extracted step script → syntax OK.
• The extracted step script executed against live repository data:
  • branch codex/search-commander-overhaul @ e46a7fec… (the current PR Overhaul web search and Commander results #427
    head) → emits the notice and writes upload=false — correctly cedes to the
    PR run.
  • branch codex/search-commander-overhaul @ e5bcc813… (a superseded SHA no
    longer any PR's head) → upload=true — still analyzes.
  • branch github-actions @ HEAD (no open PR) → upload=true — unaffected.

Not changed

The stray branch-ref analysis already recorded for e5bcc813 is immutable
history and needs no cleanup; PR #427's current head is analyzed correctly.

Source Lopu workflow run

[github-actions]

Lopu review — the diagnosis holds up; one defect in the new step, fixed in place

I confirmed the root cause independently, then found one real problem in the fix itself and two convention gaps. Changes are in the worktree and will land on this branch.

The diagnosis is right

The code-scanning API for refs/heads/codex/search-commander-overhaul @ e5bcc813 returns exactly two analyses:

uploaded	category
06:17:21Z	/language:actions
06:22:09Z	/language:javascript-typescript

GHAS opened PR #427's check at 06:17:20Z and closed it timed_out at 06:17:36Z — while only one of the two language configurations existed at that head. That is the same phenomenon as this repo's recurring 1 configuration not found refresh commits (13f7f5fb, 481cbcfe), so this targets the real mechanism rather than a symptom. Approach approved.

Two other things I checked and can confirm safe:

• pull-requests: read on analyze is within every caller's grant — the main and develop listeners both grant it at workflow level, alpha/beta carry no listener, and scope already required the same scope. No new ceiling on the reusable workflow.
• The skipped-step gate is correct. A skipped step yields an empty output and '' != 'false', so pull_request, schedule, and the centrally dispatched merge-ref path are untouched. Verified by parsing the YAML back.

🐛 The new step was fail-closed on a transient API error

set -euo pipefail with an unguarded gh pr list means one rate-limited or transient response aborts the step, which fails the analyze job. That is strictly worse than either outcome the step chooses between: it produces exactly the red CodeQL check this PR exists to eliminate, and throws away a CodeQL database that was already built. It runs on every push to every branch (push: branches: ["**"] on the product listener), so the exposure is broad.

Reproduced by extracting the step's own run: body from the YAML and running it with a failing gh on PATH:

version	exit	$GITHUB_OUTPUT
as pushed (6629d577)	1 — fails the job	(empty)
after this review	0	upload=true

Fixed with the idiom this same file already uses twice for optional lookups (lines 118–123, 139–148) — if candidate="$(gh …)"; then … else … fi, preserving gh's exit status rather than || true, which the contract explicitly forbids here. The fallback is the pre-change behaviour: analyze anyway, and emit a ::warning:: so the blip stays diagnosable.

🔒 Added a regression lock

workflow-control-plane-contract.mjs asserts essentially every CodeQL control-plane invariant, including previously fixed regressions of exactly this shape. The new ownership re-check had none, so a later edit could silently drop it and bring the timed_out bug back. Added five assertions: step ordering (after init, before upload — the deliberate design point that widens the absorbed window), the step's if, the analyze gate, the fail-open fallback, and the exact analyze permission block.

Each one is proven non-vacuous. Applied to a throwaway copy of .github, every mutation fails with its own message:

mutation	assertion that fires
drop the ownership step	ordering
move it before Initialize CodeQL	ordering
remove the upload != 'false' gate	analyze gate
remove pull-requests: read	permissions
restore the fail-closed query	fail-open fallback

📝 Added the missing CHANGELOG entry

CHANGELOG.md states outright that control-plane CI changes are recorded there because they never reach the product changelog. #432 added one; this PR had none. Appended an ### Fixed entry at the end of the section, matching 47b2fb20 ("avoid parallel changelog conflict") — verified conflict-free, since base only touched the top of ### Changed.

⚠️ One residual worth knowing about — not changed

The re-check is per-matrix-job. If a PR is created in the gap between the actions job's re-check and the javascript-typescript job's re-check, one branch-ref analysis can still land at the PR head and the symptom can recur. That window is now bounded by the skew between the two jobs' init durations (tens of seconds) rather than the whole run, and it is never worse than today's behaviour — so closing it would need cross-matrix coordination that the observed failure rate does not justify. Flagging it rather than building machinery for it.

Validation

Everything below run in the PR worktree:

• Full control-plane-ci.yml verify lane — node --check over every .github/scripts/*.mjs, bash -n over every .github/scripts/*.sh, git diff --check: clean.
• workflow-control-plane-contract.mjs --self-test → OK, plus build-all-branch, classify-claude-credential-failure, electron-pr-release-contract, resolve-pr-conflicts-routing-contract.
• Step body executed against live repository data: a branch at its live open-PR head → upload=false (cedes, emits the notice); the same branch at a superseded SHA → upload=true; github-actions @ HEAD with no open PR → upload=true.
• git merge-tree --write-tree against the current base tip 0c6a2189 → clean merge.

Check state

No failing checks on 6629d577 — CodeQL, both Analyze matrix jobs, contract advisories and both Web CI lanes all pass; every skipping entry is an intentionally gated control-plane lane. The trusted CodeQL snapshot for this head is empty, so no dispositions were written.

— Lopu, Thingtime repository steward

[github-actions]

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 9047a3c to lopu/workflow-check-fix-33045330366.

Lopu review — PR #430 · fix(actions): Lopu repairs failed PR checks

lopu/workflow-check-fix-33045330366 @ 6629d577 → github-actions @ 0c6a2189

What I compared

• Full head vs. target base. Merge base is cb7e6f2a; the three-dot diff is
  exactly one file — .github/workflows/codeql-analysis.yml, +30/−1. (A plain
  two-dot diff also lists resolve-pr-conflicts*, CHANGELOG.md and Graphify
  index files; those are base-side advances from fix(actions): Lopu repairs failed PR checks #432 and fix(actions): let Lopu review first-party CI failures #429, not this PR.)
• The reusable-workflow contract in .github/scripts/workflow-control-plane-contract.mjs,
  the control-plane-ci.yml verify + advisory lanes, and CHANGELOG.md's stated
  "CI changes are recorded here from now on" convention.
• Every caller of this reusable workflow: the main and develop listeners
  (alpha/beta carry none).
• Live check state (gh pr checks 430) and the live code-scanning API for the
  branch the PR body diagnoses.
• Adjacent history: fix(actions): Lopu repairs failed PR checks #432 (merged, resolve-pr-conflicts.yml — no overlap),
  Prioritize the main-to-develop sync resolver #426–fix(actions): let Lopu review first-party CI failures #429, and the recurring timed_out refresh commits 13f7f5fb / 481cbcfe.

Check state

No failing checks. All required contexts pass on 6629d577, including
CodeQL (3s), Analyze (actions), Analyze (javascript-typescript),
Contract advisories (non-blocking), and both Web CI lanes. Every skipping
entry is an intentionally gated control-plane lane. Nothing to repair here.

CodeQL: the trusted snapshot for this head is empty ([]). No alerts, so
no dispositions were written — 430.json stays [].

Findings

1. The diagnosis is correct — independently confirmed

The PR claims a stray refs/heads analysis at a live PR head makes GHAS open
that PR's check against the branch snapshot and close it timed_out. The
code-scanning API for refs/heads/codex/search-commander-overhaul @ e5bcc813
returns exactly two analyses:

uploaded	category
06:17:21Z	/language:actions
06:22:09Z	/language:javascript-typescript

GHAS opened PR #427's check at 06:17:20Z and closed it timed_out at 06:17:36Z
— i.e. while only one of the two language configurations existed at that
head. That is the same phenomenon as this repo's recurring
"1 configuration not found" refresh commits (13f7f5fb, 481cbcfe), so the
fix targets the real mechanism rather than a symptom.

2. Permission addition is safe

pull-requests: read on analyze is within every caller's grant — both the
main and develop listeners grant it at workflow level, and alpha/beta
have no listener at all. The scope job already required the same scope, so
this adds no new ceiling to the reusable workflow.

3. Event gating is correct

Parsed from the YAML: the new step is id: ownership,
if: github.event_name == 'push' && needs.scope.outputs.analysis_ref == '', and
Analyze the triggering revision is gated on
… && steps.ownership.outputs.upload != 'false'. A skipped step yields an empty
output, and '' != 'false', so pull_request, schedule, and the centrally
dispatched merge-ref path keep their existing semantics unchanged.

4. Defect fixed — the new step was fail-closed on a transient API error

set -euo pipefail with an unguarded gh pr list means one rate-limited or
transient response aborts the step, which fails the analyze job. That is
strictly worse than either outcome the step chooses between: it produces exactly
the red CodeQL check this PR exists to eliminate, and discards a CodeQL
database that was already built. It runs on every push to every branch
(push: branches: ["**"] on the product listener), so the exposure is broad.

Reproduced by running the step's own run: body extracted from the YAML with a
failing gh on PATH:

version	exit	$GITHUB_OUTPUT
as pushed (6629d577)	1 — step fails the job	(empty)
after this review	0	upload=true

Fixed using the idiom this same file already uses twice for optional lookups
(lines 118–123, 139–148) — if candidate="$(gh …)"; then … else … fi, which
preserves gh's exit status rather than || true (the contract explicitly
forbids || true here). The fallback is the pre-change behaviour: analyze, and
emit a ::warning:: so the blip is diagnosable.

5. Added — regression lock in the control-plane contract

workflow-control-plane-contract.mjs asserts essentially every CodeQL
control-plane invariant, including previously fixed regressions of exactly this
shape (the merge-ref 404-body one, the || true one). The new ownership
re-check had none, so a later edit could silently drop it and reintroduce the
timed_out bug. Added five assertions covering: step ordering (after init,
before upload — the deliberate design point that widens the absorbed window),
the step's if, the analyze step's gate, the fail-open fallback, and the exact
analyze permission block.

6. Added — missing CHANGELOG entry

CHANGELOG.md states outright that control-plane CI changes are recorded there
because they never reach the product changelog. The sibling Lopu fix #432 added
one; this PR had none. Appended an ### Fixed entry. Placed at the end of the
section, matching 47b2fb20 ("avoid parallel changelog conflict") — verified
conflict-free: base only touched the top of ### Changed.

7. Residual, documented rather than engineered around

The re-check is per-matrix-job. If a PR is created in the narrow gap between the
actions job's re-check and the javascript-typescript job's re-check, one
branch-ref analysis can still land at the PR head and the symptom can recur.
That window is bounded by the skew between the two jobs' init durations
(tens of seconds) instead of the whole run, and it is never worse than the
current behaviour, so closing it would need cross-matrix coordination that is
not justified by the observed failure rate. Called out on the PR, not changed.

Changes made

file	change
.github/workflows/codeql-analysis.yml	ownership lookup made fail-open, falling back to the prior analyze behaviour with a ::warning::
.github/scripts/workflow-control-plane-contract.mjs	five assertions locking the new invariant
CHANGELOG.md	### Fixed entry for the control-plane behaviour change

+58 / −6 across three files. No change to the PR's core approach, which is sound.

Validation

Run in the PR worktree:

• Full control-plane-ci.yml verify lane: node --check over every
  .github/scripts/*.mjs, bash -n over every .github/scripts/*.sh,
  git diff --check — all clean.
• node .github/scripts/workflow-control-plane-contract.mjs --self-test → OK.
  Also green: build-all-branch, classify-claude-credential-failure,
  electron-pr-release-contract, resolve-pr-conflicts-routing-contract.
• Negative tests proving the new assertions are not vacuous — each mutation
  applied to a throwaway copy of .github, each failing with its own message:
  drop the ownership step → ordering assertion fails; move it before
  Initialize CodeQL → ordering assertion fails; remove the upload != 'false'
  gate → gate assertion fails; remove pull-requests: read → permission
  assertion fails; restore the fail-closed query → fallback assertion fails.
  Unmodified worktree → self-test OK.
• YAML parsed with yaml.safe_load; analyze step order, ids and every if
  expression read back programmatically and confirmed.
• bash -n on the step body extracted from the YAML → OK.
• Step body executed against live repository data:
  claude/custom-audience-groups @ its live open-PR head → upload=false
  (cedes, emits the notice); same branch at a superseded SHA → upload=true;
  github-actions @ HEAD, no open PR → upload=true.
• Failing-gh simulation, table in finding 4.
• git merge-tree --write-tree against the current base tip 0c6a2189 →
  clean merge, no conflicts.

Disposition

Approve the approach. Changes are left uncommitted in the worktree for the
trusted publisher to push to lopu/workflow-check-fix-33045330366. Because this
PR is the dedicated controller-fix PR already targeting github-actions, the
repair belongs on its own head branch; $GITHUB_WORKSPACE/trusted was not
touched and no lopu-workflow-fix.md was written.

View Lopu workflow run