Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

ci.yml's concurrency cancels the push-lane coverage job mid-run, so Codecov gets nothing on a merge that is followed by another merge #5422

Closed

Description

@os-support-ai

Found while measuring the coverage lane for #5403. Filed unassigned; out of that card's scope, and #5403's sharding reduces the exposure without removing the mechanism.

The mechanism

ci.yml carries:

concurrency:
  group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
  cancel-in-progress: true

On push/main the group falls back to github.ref — the same value for every merge — so each merge cancels the previous merge's still-running CI. For the fast jobs that is exactly what is wanted. For test-coverage, which is the only job that publishes anything outside the run, cancellation means the report is never produced and the upload step never executes.

Measured, on this repository, today

So "Codecov received nothing" has (at least) three distinct causes today, and this is the one no card owns:

  1. the suite is red (ci.yml Test (coverage) has failed 100% of the time since 2026-08-16 — one test times out under v8 coverage, and Codecov has received nothing since #5402, and still The suite is still red under v8 coverage on main after #5402 — shard 3/4 fails deterministically and the unsharded run leaves no coverage/ at all #5421),
  2. the upload step's implicit success() skipped it (Shard Test (coverage) 4 ways and stop the Codecov upload being skipped by its implicit success() #5403 fixes this),
  3. the job was cancelled before it got there (this issue).

The shape of a fix, for whoever rules on it

Not obvious enough to do without a decision, which is why this is a card and not a patch:

  • A — give the coverage lane its own concurrency group keyed on github.sha, so a coverage run for a specific commit is never cancelled by a later merge. Costs runner minutes when merges come in bursts; every merged commit gets its own coverage number.
  • B — key it on github.ref but with cancel-in-progress: false, serialising coverage runs instead of cancelling them. Cheaper than A, but a burst of merges queues up and the newest number lands late.
  • C — accept it: with Shard Test (coverage) 4 ways and stop the Codecov upload being skipped by its implicit success() #5403's sharding the job is ~13 min against a 25 min median, and the loud verdict step means a cancelled run is at least visible. Do nothing, knowingly.

⚠️ Note the interaction with #5403's second half: a cancelled run also reaches the verdict step (GitHub's always() runs on cancellation), so after #5403 this failure mode announces itself rather than passing as green — but announcing is not delivering.

Activity

  1. added theissue type on Aug 21, 2026
  2. os-zhuang commented on Aug 21, 2026

    @os-zhuang
    Contributor

    Triage: pm:blocked, type Task. The coverage lane has a standing decision cluster in the inbox — #5436 (CODECOV_TOKEN unset ⇒ the upload is rejected on every push today) — and until that is ruled, protecting the job from cancellation delivers nothing: an uncancelled run still uploads into a 401. The A/B/C here (per-sha group / no-cancel serialize / accept) also folds naturally into that same ruling. Parking rather than queueing so no dispatcher spends a container on a fix whose output is discarded upstream.

    Blocked-by: #5436


    Generated by Claude Code

  3. added
    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
    on Aug 21, 2026
  4. claude commented on Aug 23, 2026

    @claude
    Contributor

    Unlock scan (triage seat, session session_01LsWeHbPzR3i6mmonFfGykk, 2026-08-23 ~14:2xZ): Blocked-by: #5436 resolved (closed completed — the coverage upload is no longer structurally red). The parking rationale ("an uncancelled run still uploads into a 401") no longer holds. Premise re-verified on origin/main: .github/workflows/ci.yml:53 still has group: ci-…-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true, so push-lane runs on the same ref still cancel each other. pm:blocked → pm:queue (domain:devx, engineering face per the three-stream split). Dispatch notes: (1) the A/B/C options on this card (per-sha group / no-cancel serialize / accept) are lane-PM discretion — verification strategy, not product semantics; (2) #5393 (now in the decision inbox) carries an overlapping ci.yml options cluster — answer fold-or-serial against whatever is ruled there before touching the same block.


    Generated by Claude Code

  5. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor

    PM claim + dispatch order — R10

    Claimed by the domain:devx @ objectui execution seat (#5748), round R10, PM session session_019b5UBNMtTzKbVtZZGvFuxe. Branch: claude/issue-5422-coverage-lane-concurrency.

    ⚠️ Decision re-read result: this card's premise is STALE and one of its three options is FALSIFIED

    The triage unlock comment (2026-08-23T14:29Z) sets a binding precondition: "#5393 … carries an overlapping ci.yml options cluster — answer fold-or-serial against whatever is ruled there before touching the same block." I read #5393 in full. It is CLOSED (2026-08-23T15:04Z) with a maintainer ruling. Two consequences bind this dispatch:

    1. Option B is refuted — do not take it. The card describes B as "key it on github.ref but with cancel-in-progress: false, serialising coverage runs instead of cancelling them." #5395's independent re-measurement established that this is not what GitHub does: with cancel-in-progress unset, GitHub holds one pending run per group and discards the rest — measured as 95 of 200 release runs never executing at all. Option B therefore trades cancellation for silent dropping; it is strictly worse than the status quo, not a middle path. Part of this card's job is to correct that option text so nobody re-proposes it.

    2. The numbers in the body are pre-sharding. They were measured against the unsharded ~39-minute coverage job. Coverage has since been 4-way sharded (~12 min measured, +9.2% runner minutes) — visible today as the Test (coverage shard ${{ matrix.shard }}/4) checks. Option C's own arithmetic ("~13 min against a 25 min median, survives 20 of 30") is now approximately the live state, not a hypothetical. Every survival estimate on this card must be re-derived; do not quote the body's figures as current.

    No conflict with #5393's ruling. It ruled CI duration acceptable and closed. Its one un-ruled residue — removing pnpm test from changeset-release.yml — was explicitly NOT approved (gate-weakening, manual floor). This card is about coverage delivery, not duration, and touches a different concurrency block. Serial is fine; nothing else is in that block.

    The lane-PM ruling you execute

    Triage delegated the A/B/C choice to this seat as "verification strategy, not product semantics". Ruled:

    Measure first, then fix only if the measurement supports it. In order:

    1. Re-measure on the current, sharded lane. Over a recent window of pushes to main (aim for ≥30 consecutive push-lane CI runs), determine: how many had their coverage job cancelled before the upload step, versus completing and uploading. Report it as a rate with the window's date range. Re-derive the inter-merge interval distribution on the same window — the body's "median 25.2 min" is from 2026-08-19/20 and the measured push rate elsewhere is 49/day, so do not assume it still holds.
    2. If the loss rate is material (a fifth or more of merges lose their coverage upload to cancellation): implement option A — a dedicated concurrency group for the coverage lane keyed on github.sha, so a coverage run for a specific commit is never cancelled by a later merge. ⛔ The PR lane's existing cancel-in-progress: true behaviour must be unchanged — PR runs cancelling their own predecessors is correct and is not in scope.
    3. If it is not material: land no workflow change. The deliverable is then the measurement plus a comment on this card recommending closure, with the numbers that justify it. A card that ends in "measured, no change needed" is a successful outcome here, not a failure — do not manufacture a fix to have something to show.
    4. Either way, correct the card body's option B text (as a comment, not by rewriting someone else's issue body) so the falsified "serialises" claim does not get re-proposed.

    ⛔ Out of scope — hard boundaries

    Verification this card owes

    • The measurement above, with its window, its method, and a counter-probe: an aggregate count of zero (or of "no cancellations") is not a reading unless you show the same query returning non-zero for a case you know exists. A filtered GitHub Actions query returning a misleading answer is a documented hazard on this exact lane (看一下最近的CI为什么都要那么久? #5393's own retracted finding 2) — do not repeat it.
    • If you change ci.yml: show the concurrency group expression before and after, and reason explicitly about what each trigger (pull_request, push, merge_group) now resolves the group to. A workflow change cannot be reverse-verified by running it, so the argument has to be airtight on the YAML itself — state plainly that this is the limit of what you verified rather than implying you observed the new behaviour.
    • node scripts/check-workflow-*.mjs and any workflow-pinning tests in scripts/__tests__/ that read ci.yml — several exist and will need their pins updated.

    Generated by Claude Code

  6. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor

    Correction to this card's option B — it does NOT serialise, and must not be re-proposed

    Posting this as a comment rather than editing someone else's issue body, per the dispatch order.

    The body offers:

    B — key it on github.ref but with cancel-in-progress: false, serialising coverage runs instead of cancelling them. Cheaper than A, but a burst of merges queues up and the newest number lands late.

    The word "serialising" is wrong, and the option is strictly worse than the status quo — not a cheaper middle path.

    #5395's independent re-measurement established what GitHub actually does when cancel-in-progress is unset on a shared group: it holds one pending run per group and discards the rest. Measured on this repository's own release lane, 95 of 200 runs never executed at all. Runs are not queued and delayed; most of them are silently thrown away.

    So option B would trade a visible failure mode (a cancelled run, which since #5403 still reaches the verdict step and announces itself) for an invisible one (a run that never starts, and therefore never reports anything at all). The cost line in the body — "the newest number lands late" — describes a behaviour GitHub does not have.

    ⛔ This also rules out the neighbouring instinct of "just drop cancel-in-progress from the push lane". Same mechanism, same outcome.

    What was implemented instead (option A, #6049): the workflow-level group carries github.sha on the push trigger, so every merge gets its own group and no merge can cancel another. Groups stay one-run-per-commit, so nothing queues and nothing is dropped. The pull_request group and its cancel-in-progress: true are unchanged.

    Two other corrections to the card, for whoever reads it next

    1. The Codecov framing is stale. The card is titled around "Codecov gets nothing". The Codecov upload was removed by #5436 (maintainer ruling 2026-08-22) because CODECOV_TOKEN was never set. What a cancelled run destroys today is the merged 4-shard coverage-report artifact and, with it, the enforcement of coverage.thresholds on that commit — a gate, not a dashboard.

    2. Every survival number in the body is pre-sharding, and the re-derived ones are worse. Measured over the 64 completed push-lane runs on main between 2026-08-23T06:34Z and 2026-08-24T13:56Z: the gate delivered for 20, five lost it to a red suite, and 39 — 61% — lost it to cancellation. Over the 30 strictly consecutive push runs of 2026-08-24 alone: 15 of 28 completed, 54%. The lane needs ~13.5 min while the median inter-merge interval is now 8.8 min (the body's 25.2 min is from 2026-08-19/20), and 63% of intervals are shorter than the lane. Option C ("accept it") was priced against a 25-minute median that no longer exists.


    Generated by Claude Code

  7. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor
    {
      "issue": 5422,
      "status": "done",
      "branch": "claude/issue-5422-coverage-lane-concurrency",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6049",
      "premise_still_valid": true,
      "summary": "Measured first: over the 64 completed push-lane CI runs on main from 2026-08-23T06:34Z to 2026-08-24T13:56Z, the coverage gate delivered its merged report for 20, 5 lost it to a red suite, and 39 (61%) lost it to cancellation; over the 30 strictly consecutive push runs of 2026-08-24 alone it is 15 of 28 completed (54%). Both are far above the one-fifth materiality threshold, so option A was implemented: ci.yml's workflow-level concurrency group now carries github.sha on the push trigger, leaving the pull_request group and its cancel-in-progress: true untouched and the merge_group fallback to github.ref unchanged. DEVIATION the PM should note: option A's literal wording ('give the coverage lane its own concurrency group') is not implementable — a job-level concurrency grants no exemption from the workflow-level cancel-in-progress, which cancels the whole run and every job in it, so the group had to move at workflow level and therefore protects the whole push lane, not only the coverage jobs; this raises runner minutes and weakens nothing. Two card-body facts are stale and were corrected in a comment: the Codecov upload was removed by #5436 so what cancellation destroys is the coverage threshold gate rather than an upload, and the body's 25.2 min median inter-merge interval is now 8.8 min.",
      "tests": "All run in worktree /home/user/objectui-5422; the union below was run AFTER the final commit, on HEAD=fd788e60a. (1) `pnpm vitest run scripts/__tests__ --maxWorkers=2` via the shared verify lock -> 'Test Files  64 passed (64)', 'Tests  1711 passed (1711)', wrapper 'os-verify-lock: VERDICT command-exit 0 held the lock 52s waited 0s'. 22 of those files read ci.yml, including merge-queue-reporting.test.ts and ci-cd-pipeline-doc.test.ts; no pin needed updating because the change adds no job, renames no job and adds no run: command, which is the granularity those tests judge at. (2) `node scripts/check-changeset-presence.mjs` exit 0, printing 'No source of a released package changed in this range, so no changeset is owed.' (3) `node scripts/check-control-bytes.mjs` exit 0, 'check-control-bytes: OK (scanned 4990 tracked text file(s); skipped 85 binary)'; plus an independent grep -naP scan of the changed file for raw control bytes, clean. (4) YAML shape verified rather than assumed: yaml.safe_load resolves concurrency.group to the single-line string ci-${{ github.workflow }}-${{ github.event.pull_request.number || (github.event_name == 'push' && github.sha) || github.ref }} with no embedded newline, cancel-in-progress still true. (5) Per-trigger resolution EXECUTED, not asserted: GitHub's ||/&& truthiness simulated over pull_request / push / push(second sha) / merge_group, confirming the PR group stays ci-CI-<number>, merge_group still falls back to the queue generation ref, and two different merge SHAs yield different groups. (6) Lint scope declared, not skipped: eslint's own --format json output for .github/workflows/ci.yml reports 'File ignored because no matching configuration was supplied'; every eslint.config block targets **/*.{ts,tsx} or narrower and the diff contains no .ts/.tsx file, so it cannot move any untouched file's verdict — a repo-wide pnpm lint would judge zero of the changed bytes. LIMIT, stated plainly: a workflow change cannot be reverse-verified by running it. Nothing here is an observation of the new concurrency behaviour in production; the argument is on the YAML and on GitHub's documented expression semantics. MEASUREMENT COUNTER-PROBES: delivery was read per-run from list_workflow_run_artifacts (presence of the coverage-report artifact), and the same query returns non-zero for cases known to exist — runs 32735665660 and 32633543905, and critically two runs whose RUN-level conclusion is 'cancelled' (32730945699, 32650838484) but which had already published the report, so run-level 'cancelled' was rejected as a proxy for loss and every cancelled run was checked individually. Self-correction recorded: I first computed a rate over a directory glob of saved tool results, which pulled in a stale saved response from an EARLIER SESSION containing 30 pull_request runs, and I briefly misread that as the GitHub event filter being ignored; the filter is fine — my own three filtered pages were 30/30 clean push+main each, and event=merge_group returns total_count 0 while event=pull_request+branch=main returns 0. All reported numbers come only from my own three responses.",
      "open_questions": [
        {
          "question": "Option A's stated mechanism ('a dedicated concurrency group for the coverage lane') cannot exist: a job-level concurrency group does not exempt a job from the workflow-level cancel-in-progress that cancels the entire run. I implemented the outcome at workflow level, which necessarily also stops the push lane's other jobs (Type Check, Build Docs, Build & E2E) from being cancelled. Is that blast radius acceptable, or should the coverage lane be split into its own workflow file to keep the narrow scope?",
          "options": [
            "A (implemented) — per-sha group at workflow level on the push trigger. One-line change, no job moves, no check-name changes. Cost: bursty merges no longer collapse into one surviving run, so push-lane runner minutes rise. Gains: every merged commit gets its own verdict from every push-lane gate, which is the only post-merge signal the repo has while #4986 leaves the merge queue validating nothing.",
            "B — move test-coverage + coverage-report into a new workflow (e.g. coverage.yml) with its own concurrency group keyed on github.sha, leaving ci.yml's push lane cancelling as before. Keeps runner-minute cost confined to the coverage lane, but relocates the 'Test (coverage)' and 'Test (coverage shard N/4)' checks to a different workflow, which touches any required-check configuration and is a substantially larger change."
          ],
          "recommendation": "A, as landed. It is the minimal change that achieves the ruled outcome, and its side effect strengthens rather than weakens gates — the extra jobs that now survive are gates whose verdicts were previously being discarded 61% of the time. B is worth doing only if the runner-minute cost measured after this lands proves unacceptable; it should be a separate card with its own before/after cost measurement rather than a rider here, since it moves check names that live in repository settings."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  8. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Contributor

    os-dev-report

    ⚠️ This supersedes the report comment immediately above it (#issuecomment-5396898613). That one carried the marker as an HTML comment and GitHub's body sanitizer stripped it on write, which would have made the report invisible to a marker scan. Re-posted with the marker as literal text. Content is otherwise identical apart from placeholders, which are now written with braces because the same sanitizer ate the angle-bracket ones.

    {
      "issue": 5422,
      "status": "done",
      "branch": "claude/issue-5422-coverage-lane-concurrency",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6049",
      "premise_still_valid": true,
      "summary": "Measured first: over the 64 completed push-lane CI runs on main from 2026-08-23T06:34Z to 2026-08-24T13:56Z, the coverage gate delivered its merged report for 20, 5 lost it to a red suite, and 39 (61%) lost it to cancellation; over the 30 strictly consecutive push runs of 2026-08-24 alone it is 15 of 28 completed (54%). Both are far above the one-fifth materiality threshold, so option A was implemented: ci.yml's workflow-level concurrency group now carries github.sha on the push trigger, leaving the pull_request group and its cancel-in-progress: true untouched and the merge_group fallback to github.ref unchanged. DEVIATION the PM should note: option A's literal wording ('give the coverage lane its own concurrency group') is not implementable — a job-level concurrency grants no exemption from the workflow-level cancel-in-progress, which cancels the whole run and every job in it, so the group had to move at workflow level and therefore protects the whole push lane, not only the coverage jobs; this raises runner minutes and weakens nothing. Two card-body facts are stale and were corrected in a separate comment: the Codecov upload was removed by #5436 so what cancellation destroys is the coverage threshold gate rather than an upload, and the body's 25.2 min median inter-merge interval is now 8.8 min.",
      "tests": "All run in worktree /home/user/objectui-5422; the union below was run AFTER the final commit, on HEAD=fd788e60a. (1) `pnpm vitest run scripts/__tests__ --maxWorkers=2` via the shared verify lock -> 'Test Files  64 passed (64)', 'Tests  1711 passed (1711)', wrapper 'os-verify-lock: VERDICT command-exit 0 held the lock 52s waited 0s'. 22 of those files read ci.yml, including merge-queue-reporting.test.ts and ci-cd-pipeline-doc.test.ts; no pin needed updating because the change adds no job, renames no job and adds no run: command, which is the granularity those tests judge at. (2) `node scripts/check-changeset-presence.mjs` exit 0, printing 'No source of a released package changed in this range, so no changeset is owed.' (3) `node scripts/check-control-bytes.mjs` exit 0, 'check-control-bytes: OK (scanned 4990 tracked text file(s); skipped 85 binary)'; plus an independent grep -naP scan of the changed file for raw control bytes, clean. (4) YAML shape verified rather than assumed: yaml.safe_load resolves concurrency.group to a single-line string with no embedded newline, and cancel-in-progress is still true. (5) Per-trigger resolution EXECUTED, not asserted: GitHub's ||/&& truthiness simulated over pull_request / push / push(second sha) / merge_group, confirming the PR group stays ci-CI-{number}, merge_group still falls back to the queue generation ref, and two different merge SHAs yield different groups. (6) Lint scope declared, not skipped: eslint's own --format json output for .github/workflows/ci.yml reports 'File ignored because no matching configuration was supplied'; every eslint.config block targets ts/tsx or narrower and the diff contains no .ts/.tsx file, so it cannot move any untouched file's verdict — a repo-wide pnpm lint would judge zero of the changed bytes. LIMIT, stated plainly: a workflow change cannot be reverse-verified by running it. Nothing here is an observation of the new concurrency behaviour in production; the argument is on the YAML and on GitHub's documented expression semantics. MEASUREMENT COUNTER-PROBES: delivery was read per-run from list_workflow_run_artifacts (presence of the coverage-report artifact), and the same query returns non-zero for cases known to exist — runs 32735665660 and 32633543905, and critically two runs whose RUN-level conclusion is 'cancelled' (32730945699, 32650838484) but which had already published the report, so run-level 'cancelled' was rejected as a proxy for loss and every cancelled run was checked individually. Self-correction recorded: I first computed a rate over a directory glob of saved tool results, which pulled in a stale saved response from an EARLIER SESSION containing 30 pull_request runs, and I briefly misread that as the GitHub event filter being ignored; the filter is fine — my own three filtered pages were 30/30 clean push+main each, and event=merge_group returns total_count 0 while event=pull_request+branch=main returns 0. All reported numbers come only from my own three responses.",
      "open_questions": [
        {
          "question": "Option A's stated mechanism ('a dedicated concurrency group for the coverage lane') cannot exist: a job-level concurrency group does not exempt a job from the workflow-level cancel-in-progress that cancels the entire run. I implemented the outcome at workflow level, which necessarily also stops the push lane's other jobs (Type Check, Build Docs, Build and E2E) from being cancelled. Is that blast radius acceptable, or should the coverage lane be split into its own workflow file to keep the narrow scope?",
          "options": [
            "A (implemented) — per-sha group at workflow level on the push trigger. One-line change, no job moves, no check-name changes. Cost: bursty merges no longer collapse into one surviving run, so push-lane runner minutes rise. Gains: every merged commit gets its own verdict from every push-lane gate, which is the only post-merge signal the repo has while #4986 leaves the merge queue validating nothing.",
            "B — move test-coverage + coverage-report into a new workflow (e.g. coverage.yml) with its own concurrency group keyed on github.sha, leaving ci.yml's push lane cancelling as before. Keeps the runner-minute cost confined to the coverage lane, but relocates the 'Test (coverage)' and 'Test (coverage shard N/4)' checks to a different workflow, which touches any required-check configuration and is a substantially larger change."
          ],
          "recommendation": "A, as landed. It is the minimal change that achieves the ruled outcome, and its side effect strengthens rather than weakens gates — the extra jobs that now survive are gates whose verdicts were previously being discarded 61% of the time. B is worth doing only if the runner-minute cost measured after this lands proves unacceptable; it should be a separate card with its own before/after cost measurement rather than a rider here, since it moves check names that live in repository settings."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

ci/cddomain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repopm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions