From 05abe9c6dc0f8913f3b0fdab1835308921b98b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hakan=20G=C3=9CLEN?= <5780826+hkngln@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:20:54 +0300 Subject: [PATCH 1/2] ci: bound the five jobs #812 left unbounded, including the fifth required context (#844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tan-cli#812 bounded parity.yml, which produces four of the five required contexts. The fifth -- `zizmor · workflow security`, the `name:` of ci.yml's `workflow-security` job -- still inherited GitHub's 360-minute job default, as did ci.yml's `python`, `shim` and `wheel-floor` and version-identity.yml's `not-a-released-version`. Measured `grep -c timeout-minutes` on both files at the branch point: 0. ci.yml: python 60, shim 10, wheel-floor 10, workflow-security 10. version-identity.yml: not-a-released-version 10. The `python` cap is set by the RELEASE path, not the PR path, and that is the whole subtlety. Same job, 6-8 min on pull_request/merge_group across six consecutive runs (32049093308, 32047576495, 32046563750, 32044765485, 32039025818, 32030135439) but 23 min on a release tag (run 31834718564, v0.6.0-rc1) -- release.yml:235 calls this workflow with `sdk_parity: true`, which switches on the planner byte-parity and planner-binding suites that skip without an alp-sdk checkout. A cap picked from the PR measurement passes every PR and then fails the tag. 30 would leave that run 30% headroom; 60 keeps it bounded without being a tripwire. The other four measure under a minute on both paths, so 10 is a hang catcher rather than a budget. After this, zero jobs across all seven pull_request-triggered workflows are unbounded -- verified by parsing every file in .github/workflows/ and printing each job's value, not by eye. (parity.yml's own six arrive with #841.) tests/gates: 470 passed, 9 skipped, exit 0. zizmor 1.29.0 --min-severity medium --no-online-audits: exit 0, no findings. --- .github/workflows/ci.yml | 21 +++++++++++++++++++++ .github/workflows/version-identity.yml | 4 ++++ changelog.d/844.fixed.md | 20 ++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 changelog.d/844.fixed.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6effb8fa..6b9f1adc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,6 +124,16 @@ jobs: # widening of this job. python: runs-on: ubuntu-latest + # 60, and the number is set by the RELEASE path, not the PR one. This job + # is 6-8 minutes on a `pull_request`/`merge_group` run but **23 minutes** + # on a release tag (run 31834718564, `v0.6.0-rc1`), because release.yml + # calls this workflow with `sdk_parity: true` (release.yml:235) and that + # switches on the planner byte-parity and planner-binding suites which + # SKIP without an alp-sdk checkout. A cap chosen from the PR measurement + # would pass every PR and then fail the tag -- the most expensive place in + # this repo to be wrong. 30 would leave that 23-minute run 30% headroom; + # 60 leaves it bounded without being a tripwire. + timeout-minutes: 60 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -364,6 +374,9 @@ jobs: # Node's own test runner, so there is nothing to install. shim: runs-on: ubuntu-latest + # Under a minute on every run sampled, PR and release tag alike. 10 is a + # hang catcher for the pip install, not a budget. + timeout-minutes: 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -392,6 +405,8 @@ jobs: # measures. wheel-floor: runs-on: ubuntu-latest + # Under a minute on every run sampled. Same reasoning as `shim` above. + timeout-minutes: 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -465,6 +480,12 @@ jobs: workflow-security: name: zizmor · workflow security runs-on: ubuntu-latest + # This job's `name:` IS the fifth required context, and it was the one + # tan-cli#812 left unbounded while closing the other four (all of which + # come from parity.yml). A wedged run here holds the PR for GitHub's + # 360-minute default. Measured under a minute: pip install plus a + # hermetic `--no-online-audits` pass over .github/workflows/. + timeout-minutes: 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/.github/workflows/version-identity.yml b/.github/workflows/version-identity.yml index 6059b53a..ae79128c 100644 --- a/.github/workflows/version-identity.yml +++ b/.github/workflows/version-identity.yml @@ -41,6 +41,10 @@ jobs: not-a-released-version: name: TAN_VERSION is not an existing tag's runs-on: ubuntu-latest + # A `fetch-depth: 0` checkout and one tag comparison -- measured under a + # minute (run 32046563751). The only unbounded job left in a + # PR-triggered workflow after ci.yml's four above (tan-cli#844). + timeout-minutes: 10 steps: # fetch-depth: 0 — the whole point. `fetch-tags: true` with a shallow # fetch would also do, but 0 cannot be silently defeated by a future diff --git a/changelog.d/844.fixed.md b/changelog.d/844.fixed.md new file mode 100644 index 00000000..008d9d4f --- /dev/null +++ b/changelog.d/844.fixed.md @@ -0,0 +1,20 @@ +- **No job in a PR-triggered workflow runs unbounded any more.** tan-cli#812 + bounded `parity.yml`, which produces four of the five required contexts; the + fifth — `zizmor · workflow security`, the `name:` of `ci.yml`'s + `workflow-security` job — was still inheriting GitHub's 360-minute job + default, along with `ci.yml`'s `python`, `shim` and `wheel-floor` and + `version-identity.yml`'s `not-a-released-version`. A wedged leg on that fifth + context held the PR for six hours instead of failing in bounded time. + + `ci.yml`: `python` 60, `shim` 10, `wheel-floor` 10, `workflow-security` 10. + `version-identity.yml`: `not-a-released-version` 10. + +- **`ci.yml`'s `python` cap is set by the release path, not the PR path, and + that is the whole subtlety.** The same job measures 6-8 minutes on a + `pull_request`/`merge_group` run and **23 minutes** on a release tag (run + `31834718564`, `v0.6.0-rc1`), because `release.yml:235` calls this workflow + with `sdk_parity: true` and that switches on the planner byte-parity and + planner-binding suites which SKIP without an alp-sdk checkout. A cap chosen + from the PR measurement would pass every PR and then fail the tag — the most + expensive place in this repo to be wrong. 30 would have left that run 30% + headroom; 60 keeps it bounded without being a tripwire. From d9b271ede63d1d519ae082814ac0f53a7b68df5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hakan=20G=C3=9CLEN?= <5780826+hkngln@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:50:13 +0300 Subject: [PATCH 2/2] ci: split the python job's cap by workload instead of capping both at 60 Review on #845, minor 1. A flat 60 was chosen because the release path measures 23m against 6-8m on PRs, but `timeout-minutes` takes an expression and the one input that causes the whole spread is already declared on this workflow. timeout-minutes: ${{ inputs.sdk_parity && 60 || 30 }} `inputs` is null on `pull_request`/`push`/`merge_group`, so the expression reads 30 there; `release.yml:235`'s `workflow_call` passes `sdk_parity: true` and it reads 60. 30 on the PR leg, not the 20 the review suggested, because re-measuring moved the number. Across the last 25 successful ci.yml runs the `python` job spans 6m..11m, p100 11m on run 32174278683 -- not the 9m the review measured over 20 runs, and not the "6-8 minutes" this file claimed. 20 would be 1.8x headroom on a figure that is still moving; the release leg shows the same trend (633s -> 1422s). Both legs stay far under GitHub's 360-minute default, which is the number a hang catcher actually has to beat. Review nit 2 folded in rather than spending a CI cycle on it alone: the "6-8 minutes" range and the "30% headroom" arithmetic (1422s is 23.7m, so 30 would be 26.5%, not 30%) are corrected in both the workflow comment and changelog.d/844.fixed.md. Recorded at the value and in the fragment, because it is a live trap for tan-cli#854: this is a STRING expression, and #854 generalises parity.yml's timeout gate, whose `isinstance(timeout, int)` assertion would red on this line. Gates on the git merge-tree result against origin/dev (rc=0, clean): tests/gates 485 passed, 9 skipped test_parity_workflow_concurrency_and_timeouts.py 15 passed actionlint .github/workflows/ci.yml version-identity.yml rc=0, no findings --- .github/workflows/ci.yml | 41 ++++++++++++++++++++++++++++++---------- changelog.d/844.fixed.md | 33 +++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b9f1adc..48345a07 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,16 +124,37 @@ jobs: # widening of this job. python: runs-on: ubuntu-latest - # 60, and the number is set by the RELEASE path, not the PR one. This job - # is 6-8 minutes on a `pull_request`/`merge_group` run but **23 minutes** - # on a release tag (run 31834718564, `v0.6.0-rc1`), because release.yml - # calls this workflow with `sdk_parity: true` (release.yml:235) and that - # switches on the planner byte-parity and planner-binding suites which - # SKIP without an alp-sdk checkout. A cap chosen from the PR measurement - # would pass every PR and then fail the tag -- the most expensive place in - # this repo to be wrong. 30 would leave that 23-minute run 30% headroom; - # 60 leaves it bounded without being a tripwire. - timeout-minutes: 60 + # Two numbers, because this job has two workloads and one flat cap has to + # be wrong for one of them. + # + # `sdk_parity` (declared above, `type: boolean, default: false`) is the + # whole difference: it switches on the planner byte-parity and + # planner-binding suites, which SKIP without an alp-sdk checkout -- 282 of + # 1163 tests. `release.yml:235` passes true; nothing else does. + # + # PR / merge_group 6-11 min measured across the last 25 successful + # runs (min 6m; p100 11m, run 32174278683) + # release tag 23 min run 31834718564, `v0.6.0-rc1` + # (1422s = 23.7m) + # + # A flat cap taken from the PR measurement passes every PR and then fails + # the tag, which is the most expensive place in this repo to be wrong. A + # flat 60 avoids that but leaves the >99% of runs that are PRs held for an + # hour against an 11-minute p100. The expression gets both: `inputs` is + # null on `pull_request`/`push`/`merge_group`, so `inputs.sdk_parity` is + # falsy there and this reads 30; on the `workflow_call` from release.yml it + # reads 60. + # + # 30 rather than 20 on the PR leg: p100 is 11m and trending up (the same + # trend visible on the release leg, 633s -> 1422s), so 20 is 1.8x headroom + # on a number that has moved. Both legs stay far under GitHub's own + # 360-minute default, which is what a hang catcher has to beat. + # + # NOTE for tan-cli#854, which generalises `parity.yml`'s timeout gate to + # every PR-triggered workflow: this value is a STRING expression, not an + # int. That gate's `isinstance(timeout, int)` assertion has to learn about + # expressions or it will red on this line. + timeout-minutes: ${{ inputs.sdk_parity && 60 || 30 }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/changelog.d/844.fixed.md b/changelog.d/844.fixed.md index 008d9d4f..b615081f 100644 --- a/changelog.d/844.fixed.md +++ b/changelog.d/844.fixed.md @@ -6,15 +6,26 @@ `version-identity.yml`'s `not-a-released-version`. A wedged leg on that fifth context held the PR for six hours instead of failing in bounded time. - `ci.yml`: `python` 60, `shim` 10, `wheel-floor` 10, `workflow-security` 10. - `version-identity.yml`: `not-a-released-version` 10. + `ci.yml`: `python` 30 on a PR and 60 on the release call, `shim` 10, + `wheel-floor` 10, `workflow-security` 10. `version-identity.yml`: + `not-a-released-version` 10. -- **`ci.yml`'s `python` cap is set by the release path, not the PR path, and - that is the whole subtlety.** The same job measures 6-8 minutes on a - `pull_request`/`merge_group` run and **23 minutes** on a release tag (run - `31834718564`, `v0.6.0-rc1`), because `release.yml:235` calls this workflow - with `sdk_parity: true` and that switches on the planner byte-parity and - planner-binding suites which SKIP without an alp-sdk checkout. A cap chosen - from the PR measurement would pass every PR and then fail the tag — the most - expensive place in this repo to be wrong. 30 would have left that run 30% - headroom; 60 keeps it bounded without being a tripwire. +- **`ci.yml`'s `python` job carries two caps, because it has two workloads.** + The same job measures 6-11 minutes on a `pull_request`/`merge_group` run + (last 25 successful runs; p100 11m, run `32174278683`) and **23.7 minutes** + on a release tag (run `31834718564`, `v0.6.0-rc1`, 1422s), because + `release.yml:235` calls this workflow with `sdk_parity: true` and that + switches on the planner byte-parity and planner-binding suites which SKIP + without an alp-sdk checkout — 282 of 1163 tests. + + A flat cap taken from the PR measurement passes every PR and then fails the + tag, the most expensive place in this repo to be wrong; a flat 60 avoids that + but holds the >99% of runs that are PRs for an hour against an 11-minute + p100. `timeout-minutes: ${{ inputs.sdk_parity && 60 || 30 }}` gets both: + `inputs` is null on `pull_request`/`push`/`merge_group`, so the expression + reads 30 there and 60 on the `workflow_call` from `release.yml`. 30 rather + than 20 on the PR leg because the p100 is 11m and rising, the same trend the + release leg shows (633s → 1422s). + + Note for whoever generalises `parity.yml`'s timeout gate (tan-cli#854): this + value is a string expression, not an int.