From 5895ec68ba29b280580d94ed058c954f709304b2 Mon Sep 17 00:00:00 2001 From: tianyao Date: Wed, 2 Sep 2026 10:14:52 +0000 Subject: [PATCH] ci: split release lifecycle into metadata+exact-tag-build, fix E2E cleanup concurrency race, update stale docs - Replace ci.yml's `publish` job (build+attest+semantic-release, all inline and pre-tag) with `release-metadata` (semantic-release only: tag, changelog, GitHub Release shell entry). The old job attested `github.sha` but semantic-release's `@semantic-release/git` plugin tags a *later* `chore(release):` commit -- the attestation never matched the actual released commit. - Add `release-build.yml`, triggered by `release: published`, calling `firstsun-dev/.github`'s `obsidian-plugin-release-build.yml` (same pattern already used by watermark-bucket-uploader): checkout the exact tag, build once, attest, upload, and sha256-verify the uploaded bytes -- none of which the old `publish` job did. - Drop `@semantic-release/github`'s `assets` config in `.releaserc.json` -- asset upload now happens only in `release-build.yml`, never from semantic-release itself. Also drop the stale `feat/use-shared-workflow` branch entry (confirmed via `gh api` to no longer exist). - Fix `e2e-pr-cleanup.yml`/`e2e-branch-cleanup.yml`'s concurrency group: they used `e2e--`, which no longer matches ci.yml's actual `ci-` group after ci.yml moved off per-provider groups -- so cleanup no longer queued behind a live E2E run as their own comments claimed, letting a PR-close/branch-delete cleanup race a live provider-e2e run and delete its namespace mid-flight. Now both use `ci-` to genuinely match. - Update docs/testing/real-provider-e2e.md, which still described the old per-source/per-provider concurrency scheme and conflated job-level fork-gating (skipped) with step-level workflow_dispatch gating (success with skipped steps). Part of a cross-repo CI architecture review across firstsun-dev/.github, watermark-bucket-uploader, and git-files-sync. --- .github/workflows/ci.yml | 40 +++++++++++------------- .github/workflows/e2e-branch-cleanup.yml | 31 ++++++++++++------ .github/workflows/e2e-pr-cleanup.yml | 18 ++++++----- .github/workflows/release-build.yml | 24 ++++++++++++++ .releaserc.json | 10 ++---- docs/testing/real-provider-e2e.md | 36 ++++++++++++++------- 6 files changed, 103 insertions(+), 56 deletions(-) create mode 100644 .github/workflows/release-build.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7617202..ce63bd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,16 +32,18 @@ on: # └─► provider-e2e ───┤ # lint ───────────────────────┤ # unit-test ──────────────────┤──► required-checks ──► package -# build ──────────────────────┤ └────► publish (main only) +# build ──────────────────────┤ └────► release-metadata (main only) # # All five validation jobs start in parallel right after the push; only the E2E # jobs wait on `changes` for their # path gate. No validation waits on E2E any more -- a lint/unit/build error now # surfaces in <1-2 min instead of after the real-provider matrix. The single # `required-checks` job is the only status branch protection needs to watch. -# Release (package/publish) runs only after that gate passes, so a real -# provider regression still blocks the release instead of shipping and being -# caught after the fact. +# Release (package/release-metadata) runs only after that gate passes, so a +# real provider regression still blocks the release instead of shipping and +# being caught after the fact. release-metadata only tags/publishes a GitHub +# Release shell entry (see its own comment); the actual build/attest/upload +# happens in release-build.yml, triggered by that release being published. # # Whole-run concurrency (workflow level, NOT per job): a `push` to a branch # with an open PR fires both a `push` and a `pull_request` run for the same @@ -430,12 +432,21 @@ jobs: echo "- Artifact: \`${{ env.ZIP_NAME }}\`" } >> "$GITHUB_STEP_SUMMARY" - publish: - name: Release / Publish + # Metadata-only: version bump, CHANGELOG.md, the release commit + tag, and + # the GitHub Release shell entry (no assets — see .releaserc.json's + # `@semantic-release/github` config). It must NOT build/attest/upload + # main.js — that happens in `release-build.yml`, triggered by the + # `release: published` event this step fires, checking out the exact tag + # this step creates. This ordering is required so the attested/uploaded + # bytes are built from the tagged commit (the `chore(release):` commit + # `@semantic-release/git` creates and tags here), not from this job's + # pre-release checkout. + release-metadata: + name: Release / Metadata needs: [required-checks] # semantic-release only releases on main/master (see .releaserc.json's - # `branches`); gating the whole job on those refs skips the build/attest - # work on every PR run. + # `branches`); gating the whole job on those refs skips this on every PR + # run. if: >- needs.required-checks.result == 'success' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master') @@ -443,9 +454,6 @@ jobs: permissions: contents: write id-token: write - attestations: write - env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: @@ -456,16 +464,6 @@ jobs: node-version: '22' cache: npm - run: npm ci - - run: npm run build - - name: Attest main.js - uses: actions/attest-build-provenance@4d101475d8b20a2381f78447822ac1eab6504dd8 # v4.2.2 - with: - subject-path: main.js - - name: Attest styles.css - if: hashFiles('styles.css') != '' - uses: actions/attest-build-provenance@4d101475d8b20a2381f78447822ac1eab6504dd8 # v4.2.2 - with: - subject-path: styles.css - env: GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN || github.token }} run: npx semantic-release diff --git a/.github/workflows/e2e-branch-cleanup.yml b/.github/workflows/e2e-branch-cleanup.yml index 23db039..b48b120 100644 --- a/.github/workflows/e2e-branch-cleanup.yml +++ b/.github/workflows/e2e-branch-cleanup.yml @@ -22,16 +22,29 @@ jobs: # Gitea has no persistent branches to clean up -- its whole # container is torn down per-run by scripts/e2e-harness.sh cleanup. provider: [github, gitlab] - # Same group naming as the provider-e2e job in ci.yml (e2e--) so this cleanup queues behind, rather than races, - # an E2E run still in flight for the same branch/provider. - # cancel-in-progress: false -- must never cancel an active E2E run, only - # run after it. github.event.ref's prefix isn't consistently documented - # across event types, so strip any leading refs/heads/ the same way the - # `run:` step below does, to match ci.yml's github.head_ref || - # github.ref_name (always already short). + # ci.yml's concurrency group is workflow-level (`ci-`, shared by + # push and pull_request runs for that branch -- see ci.yml's header + # comment), not per-job/per-provider. This group must match that exact + # string for the same branch so this cleanup genuinely queues behind, + # rather than races, a live CI/E2E run -- a prior per-provider group name + # here (e2e--) silently stopped matching once ci.yml + # moved to the single whole-workflow group. + # cancel-in-progress: false -- must never cancel an active CI run, only + # run after it. + # CAVEAT (unverified): `group:` is evaluated at trigger time from this + # workflow-level YAML, so it cannot use the `refs/heads/`-stripping the + # `run:` step below does -- it can only reference github.event.ref + # directly. ci.yml's push-triggered group uses github.ref_name, which + # GitHub guarantees is already the short branch name; whether a `delete` + # event's github.event.ref is also already short, or carries a + # `refs/heads/` prefix, is not consistently documented and could not be + # confirmed against a real `delete` event payload in this review. If it + # does carry the prefix, this group will NOT match ci.yml's group for + # the same branch, and branch-delete cleanup will not queue behind a + # live CI run the way e2e-pr-cleanup.yml's (confirmed-matching) group + # does. Verify against a real delete-event run before trusting this. concurrency: - group: e2e-${{ github.event.ref }}-${{ matrix.provider }} + group: ci-${{ github.event.ref }} cancel-in-progress: false steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 diff --git a/.github/workflows/e2e-pr-cleanup.yml b/.github/workflows/e2e-pr-cleanup.yml index 4ee2962..5e8fbbb 100644 --- a/.github/workflows/e2e-pr-cleanup.yml +++ b/.github/workflows/e2e-pr-cleanup.yml @@ -25,14 +25,18 @@ jobs: # Gitea has no persistent branches to clean up -- its whole # container is torn down per-run by scripts/e2e-harness.sh cleanup. provider: [github, gitlab] - # Same group naming as the provider-e2e job in ci.yml (e2e-- - # , keyed by the PR's source branch, not its number -- see - # that job's concurrency comment for why) so this cleanup queues behind, - # rather than races, an E2E run still in flight for the same branch/ - # provider. cancel-in-progress: false -- this must never cancel an - # active E2E run; it only needs to run after it. + # ci.yml's concurrency group is workflow-level (`ci-`, shared by + # push and pull_request runs for that branch -- see ci.yml's header + # comment), not per-job/per-provider. This group must match that exact + # string for the same branch so this cleanup genuinely queues behind, + # rather than races, a live CI/E2E run -- a prior per-provider group name + # here (e2e--) silently stopped matching once ci.yml + # moved to the single whole-workflow group, which let cleanup delete an + # e2e/pr//** namespace while provider-e2e was still using it. + # cancel-in-progress: false -- this must never cancel an active CI run; + # it only needs to run after it. concurrency: - group: e2e-${{ github.event.pull_request.head.ref }}-${{ matrix.provider }} + group: ci-${{ github.event.pull_request.head.ref }} cancel-in-progress: false steps: # No `ref:` -- actions/checkout defaults to the commit that triggered diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml new file mode 100644 index 0000000..6744e7e --- /dev/null +++ b/.github/workflows/release-build.yml @@ -0,0 +1,24 @@ +name: Release Build + +# Triggered once semantic-release (in ci.yml's release-metadata job) has +# finalized the version bump and published the GitHub Release. Builds from +# the exact release tag, attests, and uploads the release assets — see +# firstsun-dev/.github's obsidian-plugin-release-build.yml for why this is a +# separate workflow from ci.yml's release-metadata job. + +on: + release: + types: [published] + +permissions: + contents: write + id-token: write + attestations: write + +jobs: + build: + uses: firstsun-dev/.github/.github/workflows/obsidian-plugin-release-build.yml@main + with: + plugin-id: "git-file-sync" + secrets: + RELEASE_TOKEN: ${{ secrets.RELEASE_TOKEN }} diff --git a/.releaserc.json b/.releaserc.json index d5e5a86..6707596 100644 --- a/.releaserc.json +++ b/.releaserc.json @@ -1,5 +1,5 @@ { - "branches": ["main", "master", "feat/use-shared-workflow"], + "branches": ["main", "master"], "tagFormat": "${version}", "plugins": [ [ @@ -72,13 +72,7 @@ ], [ "@semantic-release/github", - { - "assets": [ - { "path": "main.js" }, - { "path": "manifest.json" }, - { "path": "styles.css" } - ] - } + {} ] ] } diff --git a/docs/testing/real-provider-e2e.md b/docs/testing/real-provider-e2e.md index 1467022..d57a922 100644 --- a/docs/testing/real-provider-e2e.md +++ b/docs/testing/real-provider-e2e.md @@ -124,13 +124,21 @@ killed run's branch is simply never touched by the next one. ### Concurrency and cancellation -`.github/workflows/ci.yml`'s E2E jobs carry per-source/per-provider concurrency groups with -`cancel-in-progress: true`. Push and pull-request runs use the same branch identity, so a push to a -branch with an open PR cancels its duplicate instead of both competing for runner/provider -capacity. Manual dispatches and schedules use `-` instead: they must not cancel a -normal PR's required checks or a push's provider run. The two cleanup workflows share the -branch-based group naming used by push/PR runs, with -`cancel-in-progress: false`, so cleanup queues behind rather than races an active run. +`.github/workflows/ci.yml` carries a single whole-workflow-level concurrency group per branch +(`ci-`, not per-job or per-provider) with `cancel-in-progress: true`. Push and pull-request +runs for the same branch share this one group — deliberately, not just to avoid the two competing +for runner/provider capacity: push and PR-synchronize events for the same branch typically fire for +the identical commit SHA, so a shared group avoids paying for the expensive self-hosted/real-provider +E2E matrix twice for the same commit, and avoids a "split winners" bug an earlier per-provider-group +design had (push could cancel only one provider's leg of a PR run and vice versa, leaving no single +run fully green — see `ci.yml`'s own header comment). Manual dispatches and schedules use +`-` instead: they must not cancel a normal PR's required checks or a push's provider +run. The two cleanup workflows use the same `ci-` group naming as `ci.yml` (not a separate +`e2e-`-prefixed, per-provider naming — an earlier version of both cleanup workflows used a group name +that had silently stopped matching `ci.yml`'s after this refactor, defeating the queuing this +paragraph describes), with `cancel-in-progress: false`, so cleanup queues behind rather than races an +active run. See a caveat in `e2e-branch-cleanup.yml`'s own comment about whether this holds for +`delete` events specifically. The cancelled duplicate's `CI / Required Checks` treats the cancelled leg as a failure, but that run is for the superseded commit — the surviving run (the one GitHub uses for the latest commit) is responsible for the real provider result and release gate, so the cancelled duplicate's red @@ -254,8 +262,8 @@ and its job-level condition rejects fork PRs before runner allocation. Both depe computed by the `CI / Detect Changes` job, since GitHub Actions' own `on.*.paths` would gate the *entire* workflow file, including the always-must-run validation/release jobs). It always runs in full on `workflow_dispatch`, `schedule` (weekly, Monday 06:00 UTC, for API-drift detection), and -pushes to `main`. Each job carries a per-source/provider `concurrency` group (see "Isolation model" -above) and a run/attempt/provider-scoped workdir. +pushes to `main`. The whole workflow shares one `concurrency` group per branch (see "Isolation model" +above), and each E2E job additionally uses a run/attempt/provider-scoped workdir. Two more workflows round out the isolation model's other cleanup layers — see "Isolation model" above for what each does and why: @@ -294,8 +302,14 @@ build ──────────────────────── All five validation jobs start in parallel; a lint/unit/build error now surfaces in <1-2 min instead of after the real-provider matrix. `CI / Required Checks` runs with `if: always()` and -passes only when every validation job reports `success` or `skipped` (a path-filtered-out or -fork-gated-off `provider-e2e` leg reports `success` because its steps are skipped, not failed). +passes only when every validation job reports `success` or `skipped`. A fork-gated-off +`provider-e2e` leg is skipped at the *job* level (the fork-PR check is a job-level `if:`, evaluated +before any step runs), so its result is `skipped`. A path-filtered-out leg is likewise a job-level +skip. Separately, `workflow_dispatch`'s single-provider selection uses a *step*-level gate (the +"Determine whether this provider leg should run" step), so a leg excluded that way still reports +`success` for the job, just with its individual steps skipped. Both outcomes are accepted by the +`success|skipped` check above, so the practical effect (doesn't block the gate) is the same either +way. Any other result — including a `cancelled` matrix leg replaced by a newer run in the same concurrency group — fails the gate; the surviving run is the one whose gate result GitHub uses for the latest commit. `Release / Package` and `Release / Publish` both run only after the gate