Cross-check branch CI run lookup with workflow-runs REST API

[jpascucci-nv]

Description

refs #2975

Mitigate flaky prior-branch artifact lookup failures when gh run list returns a stale bounded slice missing recent successful CI runs.

Branch mode in ci/tools/lookup-run-id now cross-checks gh run list with the direct workflow-runs REST endpoint, normalizes both result sets, unions them by run ID, and then keeps the existing local sorting and artifact validation logic.

Testing

• python3 -m pytest ci/tools/tests/test_lookup_run_id.py
• bash -n ci/tools/lookup-run-id
• git diff --check

Checklist

• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[rwgk]

GPT-6.1-Sol ultra review:

1. [P1] Branch lookups fail on Windows — lookup-run-id:135–137. Bash process substitution produces /proc/.../fd/... paths that native Windows jq cannot open. This already breaks the PR’s Windows build, exiting with code 2. Feed both JSON arrays through stdin instead.

2. [P2] Stale CLI records override successful REST records — lookup-run-id:135–137. unique_by(.databaseId) keeps the first record, from the CLI. If that record reports an unfinished run while REST reports success, the subsequent filter discards the run. I reproduced a “No successful run found” failure despite a successful REST result. Prefer the successful record when merging duplicates.

3. [P2] Other workflow display names stop working — lookup-run-id:112–118. Only CI is translated to a filename. A valid selector such as CI: Coverage now fails the mandatory REST request, which accepts an ID or filename. Resolve selectors generally. Current repository callers use CI, so this affects the documented optional workflow-name interface.

[github-actions]

Doc Preview CI
Preview removed because the pull request was closed or merged.

[leofang]

My bot also took a pass.

[rwgk]

After seeing the findings below I asked:

Would you request changes to 2999 before merging, or would you prefer to merge and work on a follow-on PR?

codex:

I’d merge the reviewed head and address the remaining issues in a follow-on PR.

It fixes the demonstrated CI failures, resolves all three original findings, and passes CI. The remaining findings are valid, but concern bounded discovery and a freshness-policy tradeoff; I don’t consider them merge blockers.

The follow-on should extend discovery when the initial scan finds no qualifying run, clarify the freshness policy, and add regression tests for both cases.

---

codex GPT-6.1-Sol ultra findings:

[P2] The 25-commit limit can hide a valid fallback run — lookup-run-id:146–150. If the qualifying run is on commit 26 and 100 newer runs on other branches fill both broad lists, lookup fails despite unexpired required artifacts. I reproduced this: the updated script fails; the baseline finds the run. Continue discovery when the initial scan finds no qualifying candidate.

[P2] The freshness rule rejects legitimate latest successes — lookup-run-id:214–227. When HEAD has no successful run, a genuine latest success four days old is rejected by the default 72-hour cutoff. I reproduced this even with unexpired artifacts. This measures wall-clock age, whereas Leo suggested comparison with branch HEAD’s commit date. It introduces an availability policy that needs justification; age alone does not demonstrate a stale response.

Leo’s central observation is independently supported: nightly’s log shows it selected the September 22 run although a successful October 2 run already existed. His concerns about incomplete discovery and silent stale selection were valid.

His precise cache explanation remains an inference. Also, the cited community discussion’s “10/10” experiment removed only the status filter while retaining the branch filter. It does not establish independent caches or immunity of head_sha queries.

The current code implements Leo’s proposed defenses, so Jason’s agent’s assessment is substantially correct. “Fully addressed” is too strong given the search bounds and freshness tradeoff above.

Reviewed head e6f3503. All three original findings are fixed.

[leofang]

Thanks, Jason/Ralf! I think this is mergeable, but I am taking another pass and evaluating with my bot if we could get away from needing the stale run check, see #3013.

[leofang]

Not a blocker, but I've been questioning for a long time why we're adding tests for CI tools.

• These tests never caught real-life issues like what we're seeing in the CI
• These tests aren't run anywhere last time I checked (a while ago)
• CI is our best test ground and there should be a boundary below which we stop writing tests, otherwise we would set ourselves in a situation where we test a test that tests another test, it's endless.

[jpascucci-nv]

#3013 is a better fix.

[leofang]

Thanks, Jason/Ralf!