From 95488c3be3b8b2805ee0c55bb556aa7e27f78a75 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 17:36:49 +0900 Subject: [PATCH] fix(opencode): stop two echo-only jobs from serializing the review critical path opencode-review.yml chained five jobs in series: required-workflow-bootstrap -> admit-current-head -> coverage-source-tree -> coverage-evidence -> opencode-review-target. The middle two exist solely to hold branch-protection contexts; each one's entire body is a single `echo`, and neither declares `outputs:`, so both `needs:` edges through them ordered work without carrying any data. Ordering is not free. A job is not created until its `needs:` complete, so under a saturated queue every link waits out the whole queue again. Measured on naruon#1528 (run 33581213805), where each job's created_at equals the previous job's completed_at: required-workflow-bootstrap waited 7h57m, ran 4s coverage-source-tree waited 9h40m, ran 4s coverage-evidence waited 13h01m, ran 5s opencode-review waited 12h13m That is ~22h41m of queue time spent to print two sentences, with the actual review held behind it. Both edges are removed and the two context holders now depend on admit-current-head directly, dropping serial depth from 5 to 3 and the number of queue waits from 4 to 2. Safety, verified rather than assumed: - coverage-evidence had no `if:` and relied entirely on transitive skipping through coverage-source-tree. Its admission gate is now stated explicitly, so an unadmitted head still skips it. Dropping the edge without this would have run a required context on unadmitted heads. - opencode-review-target never reads coverage-evidence at runtime; the only reference was the `needs:` line itself. The consumer of that context is opencode-review-dispatch.yml via scripts/ci/opencode_coverage_identity.py, which resolves it against the check-runs API on its own schedule. - No test asserts this ordering. scripts/ci/test_strix_quick_gate.sh:1203 names both jobs but as set membership, not sequence. - Branch protection evaluates required contexts independently; all of required-workflow-bootstrap, coverage-evidence and opencode-review still report. Scope is deliberately limited to opencode-review.yml. opencode-review-dispatch.yml has jobs of the same two names whose edge is a real data dependency -- its coverage-source-tree uploads the materialized PR merge tree and its coverage-evidence downloads it -- and must not be parallelized. Credit to peer session review for catching that name collision, and to a Codex audit for finding the echo-only jobs in the first place. Co-Authored-By: Claude Opus 5 --- .github/workflows/opencode-review.yml | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 4494e74090..19ea58003f 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -288,7 +288,18 @@ jobs: coverage-evidence: name: coverage-evidence - needs: [coverage-source-tree] + # Deliberately NOT `needs: [coverage-source-tree]`. Neither job declares + # `outputs:`, so that edge only ordered two single-`echo` context holders -- + # and a job is not created until its `needs:` complete, so under a saturated + # queue each link waits out the whole queue again. Measured on + # naruon#1528 (run 33581213805): coverage-source-tree waited 9h40m to run for + # 4s, then coverage-evidence waited a further 13h01m to run for 5s, holding + # the actual review behind ~22h41m of pure queueing. Depending on + # `admit-current-head` directly lets the two run in parallel. The `if:` below + # restates the admission gate this job previously inherited transitively + # through coverage-source-tree, so an unadmitted head still skips it. + needs: [required-workflow-bootstrap, admit-current-head] + if: needs.admit-current-head.outputs.admitted == 'true' runs-on: ubuntu-24.04 steps: - run: >- @@ -297,7 +308,16 @@ jobs: opencode-review-target: name: opencode-review - needs: [admit-current-head, coverage-evidence] + # `coverage-evidence` is deliberately absent here. This job never reads it + # at runtime -- the only consumer of that context is + # `opencode-review-dispatch.yml`, which resolves it through + # `scripts/ci/opencode_coverage_identity.py` against the check-runs API on + # its own schedule, so it does not care when this job ran relative to it. + # The edge was pure ordering, and ordering is expensive: a job is not + # created until its `needs:` finish, so this link cost a further 12h13m of + # queue wait on naruon#1528 (run 33581213805). Admission is still enforced + # directly by this job's own `if:` below, not inherited through that edge. + needs: [admit-current-head] if: needs.admit-current-head.outputs.admitted == 'true' runs-on: ubuntu-24.04 permissions: