From 8f3a2c936e4a89a63349994b06faad74443a5fa3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 17:12:34 +0900 Subject: [PATCH 1/3] docs(claude): record the two CI-failure diagnosis traps this session cost time on Both are first-hand findings, assigned to this session under the peer work split (peer3 takes verification discipline, peer1 queue operations): 1. GITHUB_ACTIONS-gated production code executes inside the test suite in CI but never locally, so "the suite passes on my machine" is not evidence. This blocked agent-review-runtime-quality across ~20 unrelated PRs (#1896). 2. gh pr checks surfaces stale cancelled runs indefinitely, including on already-merged PRs whose residual closure-event runs nothing supersedes. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 550ad5ce80..4504da65d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,6 +143,22 @@ repeatable compile command. base branch catches up; a same-head manual `workflow_dispatch` Strix run may supply review evidence but does not replace required PR checks. Do not widen a `pull_request_target` job token to repository-write permission. +- **A green local test run is not evidence the suite passes in CI.** Production code in + `scripts/ci/` gates real behaviour on `os.environ.get("GITHUB_ACTIONS") == "true"` (e.g. + `inspect_pr`'s call to `recover_current_head_startup_failures`). GitHub sets that variable for + the *entire job*, including the pytest process running the suite, so such a branch executes + during tests in CI and never locally — tests that never anticipated the side effect then fail + only on the runner. This blocked the `agent-review-runtime-quality` required check across ~20 + unrelated PRs before it was found (#1896). When a check fails but the suite passes locally, + re-run it as `GITHUB_ACTIONS=true PYTHONPATH=. python -m pytest tests` before concluding the + failure is infrastructure noise; if that reproduces it, stub the environment-gated call in the + affected tests rather than changing the production guard. +- **`gh pr checks` can report failures that are neither current nor real.** It aggregates by check + *name* and will keep surfacing an old cancelled run — including, indefinitely, the residual + closure-event runs of a PR that already merged, since nothing will ever supersede them. Check + `gh pr view --json state,mergedAt` first, then confirm a suspicious entry with + `gh api repos///actions/jobs/` (a `"conclusion": "cancelled"` with empty + `steps` never ran) before spending any effort diagnosing it or reporting it to another session. - **Review output must go through the Python normalizer** (`scripts/ci/opencode_review_normalize_output.py`) — it escapes `<`, `>`, `&` when embedding JSON in HTML comments to prevent Markdown-comment breakout. Do not reintroduce bash fast-path extraction. From fe592bebd19c0f1317ead321a0f347067f159f03 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 20:30:13 +0900 Subject: [PATCH 2/3] docs(claude): record queue-depth, merge-gating, and duplicate-job-name traps Three measurements from the 2026-09-05 saturation work that are specific to this organization and cost real time to establish. Queue depth read from an unfiltered actions/runs call undercounts silently, because that call returns only the ~30 most recent runs regardless of status. Merge-gating read from classic branch protection alone misclassifies ruleset-gated repositories, and reading either one without distinguishing a central review context from a repository-local one (wardnet requires only "rust", newsdom-api only "pytest") overstates how much of the queue can block a merge. Enumerating all 76 repositories on both paths puts that figure at five repositories, holding 39% of the queued runs. The duplicate-job-name entry records a near miss: coverage-source-tree is an echo-only sentinel in opencode-review.yml and a load-bearing job with dependents in opencode-review-dispatch.yml. A truncated reference scan hid the second one. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 4504da65d8..a5096a9440 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,6 +159,23 @@ repeatable compile command. `gh pr view --json state,mergedAt` first, then confirm a suspicious entry with `gh api repos///actions/jobs/` (a `"conclusion": "cancelled"` with empty `steps` never ran) before spending any effort diagnosing it or reporting it to another session. +- **Queue depth and merge-gating each need a specific query, and the obvious one is wrong for both.** + For depth, `?status=queued&per_page=1` and read `total_count`; an unfiltered `actions/runs` returns + only ~30 recent runs and silently undercounts. For gating, read **both** + `branches/main/protection/required_status_checks` (classic) and `rules/branches/main` (ruleset + effective) — checking only the first misclassifies ruleset-gated repos such as `aFIPC` and + `newsdom-api` as ungated. Distinguish *gated* from *centrally* gated too: `wardnet` requires only + `rust` and `newsdom-api` only `pytest`, so no central review check blocks a merge in either. + Measured 2026-09-05 across all 76 repos, only `.github`, `contextual-orchestrator`, `fast-mlsirm`, + `pg-erd-cloud`, and `bandscope` have a central review context gating merge, and 61% of the org's + 2093 queued runs originate outside them (#1928). +- **A job name is not unique across workflows — check the pair, not the name.** `coverage-source-tree` + exists twice: in `opencode-review.yml` it is an echo-only sentinel with no dependents after #1910 + and is a required context in no repository, while in `opencode-review-dispatch.yml` it materializes + the coverage source tree and is depended on by `needs: [validate-pr-metadata, coverage-source-tree]`. + Deleting the first on a whole-repo string search that happened to be truncated would have broken the + second. Relatedly: a `grep | head -N` that returns exactly N lines is a truncation warning, not a + result — re-run it uncapped before concluding anything from its absence of hits. - **Review output must go through the Python normalizer** (`scripts/ci/opencode_review_normalize_output.py`) — it escapes `<`, `>`, `&` when embedding JSON in HTML comments to prevent Markdown-comment breakout. Do not reintroduce bash fast-path extraction. From 96dc8958afc03efb0c0956b5b02e0060d98d1534 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 20:39:15 +0900 Subject: [PATCH 3/3] docs(claude): warn against hard-coding main when querying branches Correction to the previous commit, which asserted a queue-composition figure derived from a defective query. Querying branches/main/protection across the organization is wrong: 35 of the 76 repositories have a different default branch. naruon has no main at all, so the API answers "Branch not found", and reading that 404 as "unprotected" misclassified the second-largest queue contributor. bandscope shows the other half of the trap, having a protected main that is not its default, so a hard-coded query can return a plausible answer for the wrong branch rather than an error. Replaces the numeric claim with the query discipline, which is what transfers. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c944d1c89b..c936750426 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -166,16 +166,21 @@ repeatable compile command. `gh pr view --json state,mergedAt` first, then confirm a suspicious entry with `gh api repos///actions/jobs/` (a `"conclusion": "cancelled"` with empty `steps` never ran) before spending any effort diagnosing it or reporting it to another session. -- **Queue depth and merge-gating each need a specific query, and the obvious one is wrong for both.** - For depth, `?status=queued&per_page=1` and read `total_count`; an unfiltered `actions/runs` returns - only ~30 recent runs and silently undercounts. For gating, read **both** - `branches/main/protection/required_status_checks` (classic) and `rules/branches/main` (ruleset - effective) — checking only the first misclassifies ruleset-gated repos such as `aFIPC` and - `newsdom-api` as ungated. Distinguish *gated* from *centrally* gated too: `wardnet` requires only - `rust` and `newsdom-api` only `pytest`, so no central review check blocks a merge in either. - Measured 2026-09-05 across all 76 repos, only `.github`, `contextual-orchestrator`, `fast-mlsirm`, - `pg-erd-cloud`, and `bandscope` have a central review context gating merge, and 61% of the org's - 2093 queued runs originate outside them (#1928). +- **Never hard-code `main` when querying a branch across this organization — 35 of the 76 repos + have a different default branch.** `naruon`'s is `develop` and it has no `main` at all, so + `branches/main/protection` answers `"Branch not found"`, which means the branch is absent, *not* + that the branch is unprotected. Reading that 404 as "no protection" misclassified the org's + second-largest queue contributor (231 queued runs, 5 central review contexts gating merge) and + inverted a majority claim in #1928. Resolve `.default_branch` per repo first. Note that a repo can + also have a protected `main` that is not its default (`bandscope`), so a hard-coded query can + return a real-looking answer for the wrong branch instead of an error. +- **Merge-gating needs both protection paths, and "gated" is not "centrally gated".** Read + `branches//protection/required_status_checks` (classic) *and* `rules/branches/` + (ruleset effective); checking only the first misclassifies ruleset-gated repos such as `aFIPC` and + `newsdom-api`. Then check whether the required contexts are actually the central review checks — + `wardnet` requires only `rust` and `newsdom-api` only `pytest`, so no central review check blocks a + merge in either. For queue depth, use `?status=queued&per_page=1` and read `total_count`; an + unfiltered `actions/runs` returns only ~30 recent runs and silently undercounts. - **A job name is not unique across workflows — check the pair, not the name.** `coverage-source-tree` exists twice: in `opencode-review.yml` it is an echo-only sentinel with no dependents after #1910 and is a required context in no repository, while in `opencode-review-dispatch.yml` it materializes