From df469ea6ff693990c5f768d1e2e85c8f7867bceb Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:39:57 +0800 Subject: [PATCH] fix(ci): keep the pre-merge check green in report mode (#7311) * fix(ci): keep the pre-merge check off the report-mode failures * fix(ci): fail the pre-merge check in block mode without a base commit * fix(ci): report every pre-merge failure the way its mode asks The merge step now separates a base commit it cannot fetch from a merge it cannot do, both failures print an annotation, and a spec pins the mode wiring of the three Policy workflows. * test(ci): type the Policy workflow assertions --------- Co-authored-by: imccyu --- .github/AGENTS.md | 2 +- .github/workflows/pr-check-on-head-branch.yml | 33 +++++++++++-- scripts/ci-workflow.spec.ts | 46 +++++++++++++++++++ 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/.github/AGENTS.md b/.github/AGENTS.md index 965145639f..9b891d3a6e 100644 --- a/.github/AGENTS.md +++ b/.github/AGENTS.md @@ -4,6 +4,6 @@ Run jobs on Windows runners (`windows-*` labels) under native `pwsh`. Native Win The manual [Windows packaging workflow](workflows/windows-package.yml) uses the dedicated self-hosted Windows x64 label `dsh-win-package-trial` under native `pwsh`; preserve this routing separately from the automatic CI lanes. -The merge checks run from three workflows named `Policy`, which run on `pull_request_target` so their definitions come from master, and call the `deepseek-harness-check` action directly: `pr-check-on-head-branch.yml` on pull-request updates gives `Policy / Master Change` and `Policy / Pre-Merge Check`, `pr-check-after-approve.yml` on pull-request updates and submitted or dismissed reviews gives `Policy / Approve Check` and `Policy / Reviewer Evaluate`, and `pr-check-on-base-branch.yml` on master pushes re-runs the head-branch checks as `Policy / Recheck`. Branch rules require those check names, so keep the job names and the workflow name. Each check reads its mode from a repository variable: `MERGE_POLICY_MASTER_DIFF`, `MERGE_POLICY_PRE_MERGE`, `MERGE_POLICY_HEAD_DIFF`, and `MERGE_POLICY_REVIEW`, each `off` (skip), `report` (run without failing the job), or `block` (fail the job); a missing value or an unknown one counts as `report`. `Policy / Pre-Merge Check` merges the commit `Policy / Master Change` reports, so it runs only after that job and stays off while `MERGE_POLICY_MASTER_DIFF` is `off`. +The merge checks run from three workflows named `Policy`, which run on `pull_request_target` so their definitions come from master, and call the `deepseek-harness-check` action directly: `pr-check-on-head-branch.yml` on pull-request updates gives `Policy / Master Change` and `Policy / Pre-Merge Check`, `pr-check-after-approve.yml` on pull-request updates and submitted or dismissed reviews gives `Policy / Approve Check` and `Policy / Reviewer Evaluate`, and `pr-check-on-base-branch.yml` on master pushes re-runs the head-branch checks as `Policy / Recheck`. Branch rules require those check names, so keep the job names and the workflow name. Each check reads its mode from a repository variable: `MERGE_POLICY_MASTER_DIFF`, `MERGE_POLICY_PRE_MERGE`, `MERGE_POLICY_HEAD_DIFF`, and `MERGE_POLICY_REVIEW`, each `off` (skip), `report` (run without failing the job), or `block` (fail the job); a missing value or an unknown one counts as `report`. `Policy / Pre-Merge Check` merges the commit `Policy / Master Change` reports, so it runs only after that job and stays off while `MERGE_POLICY_MASTER_DIFF` is `off`; a merge it cannot do and a build that fails both follow its own mode, so `report` warns and skips the build while `block` fails the job. The manual Desktop publication workflow selects a successful package run and attempt, then routes each retained build through `DESKTOP_PUBLISH_RUNNERS` to its original machine's unique label. Keep Windows run steps on native PowerShell and Mac run steps on the default POSIX shell; shared entrypoints are Node scripts. Test and production jobs use separate `desktop-test` and `desktop-production` approval Environments after a credential-free preparation job. diff --git a/.github/workflows/pr-check-on-head-branch.yml b/.github/workflows/pr-check-on-head-branch.yml index cb72293c53..5ab02a6a94 100644 --- a/.github/workflows/pr-check-on-head-branch.yml +++ b/.github/workflows/pr-check-on-head-branch.yml @@ -57,28 +57,51 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 1 - - name: Merge the checked base-branch commit + # Merging is the precondition of the build below, so a failure here warns and skips it in `report` mode and fails + # in `block` mode. + - id: merge + name: Merge the checked base-branch commit env: + CHECK_MODE: ${{ vars.MERGE_POLICY_PRE_MERGE }} BASE_SHA: ${{ needs.master.outputs.base_sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} SINCE: ${{ needs.master.outputs.since }} run: | - [[ "$BASE_SHA" =~ ^[0-9a-f]{40}$ && "$SINCE" =~ ^[0-9]+$ ]] || { echo "::error::the master diff check reported no base commit"; exit 1; } - git fetch --no-tags --no-write-fetch-head --shallow-since="$SINCE" origin "$HEAD_SHA" "$BASE_SHA" - git -c user.name=pr-check -c user.email=pr-check@users.noreply.github.com merge --no-ff --no-edit -m "Merge $BASE_SHA into $HEAD_SHA" "$BASE_SHA" + stop() { + if [ "$CHECK_MODE" = block ]; then + echo "::error title=Pre-Merge Check::$1" + exit 1 + fi + echo "::warning title=Pre-Merge Check::$1" + exit 0 + } + if [[ ! "$BASE_SHA" =~ ^[0-9a-f]{40}$ || ! "$SINCE" =~ ^[0-9]+$ ]]; then + stop "the master diff check reported no base commit, so nothing was merged or compiled" + fi + git fetch --no-tags --no-write-fetch-head --shallow-since="$SINCE" origin "$HEAD_SHA" "$BASE_SHA" || + stop "the base commit $BASE_SHA could not be fetched, so nothing was merged or compiled" + git -c user.name=pr-check -c user.email=pr-check@users.noreply.github.com merge --no-ff --no-edit -m "Merge $BASE_SHA into $HEAD_SHA" "$BASE_SHA" || + stop "the pull request cannot merge $BASE_SHA; merge master into it and push, then this check runs again" + echo "merged=true" >> "$GITHUB_OUTPUT" - uses: pnpm/action-setup@v5.0.0 + if: steps.merge.outputs.merged == 'true' - uses: actions/setup-node@v7.0.0 + if: steps.merge.outputs.merged == 'true' with: node-version: '24' package-manager-cache: false - name: Run the pre-merge command + if: steps.merge.outputs.merged == 'true' env: CHECK_MODE: ${{ vars.MERGE_POLICY_PRE_MERGE }} run: | if ! bash -euxo pipefail -c 'pnpm install --frozen-lockfile && pnpm exec tsx scripts/compile-referenced-projects.ts host-contracts && pnpm exec tsx scripts/compile-referenced-projects.ts client'; then - if [ "$CHECK_MODE" = block ]; then exit 1; fi + if [ "$CHECK_MODE" = block ]; then + echo "::error title=Pre-Merge Check::the merged tree does not install or compile" + exit 1 + fi echo "::warning title=Pre-Merge Check::the merged tree does not install or compile; this check does not block" fi diff --git a/scripts/ci-workflow.spec.ts b/scripts/ci-workflow.spec.ts index 29d4dc3354..877b45c610 100644 --- a/scripts/ci-workflow.spec.ts +++ b/scripts/ci-workflow.spec.ts @@ -1686,3 +1686,49 @@ function workflowJob(workflow: Record, job: string): Record { return typeof value === 'object' && value !== null && !Array.isArray(value) } + +describe('Policy workflows', () => { + const workflows = [ + ['pr-check-on-head-branch.yml', ['master']], + ['pr-check-after-approve.yml', ['approve', 'review']], + ] as const + + function stepsOf(job: Record): Record[] { + const steps = job.steps as unknown[] + if (!Array.isArray(steps)) throw new TypeError('job must define steps') + return steps.filter(isRecord) + } + + it.each(workflows)('%s runs each check from master and reads its mode from a repository variable', (file, jobs) => { + const workflow = loadWorkflow('.github/workflows/' + file) + expect(Object.keys(workflow).filter(key => key !== 'name')).toEqual(['on', 'permissions', 'concurrency', 'jobs']) + const events = workflow.on + if (!isRecord(events)) throw new TypeError(`${file} must declare events`) + expect(events).not.toHaveProperty('pull_request') + expect(events).toHaveProperty('pull_request_target') + for (const job of jobs) { + const check = stepsOf(workflowJob(workflow, job)).find(step => typeof step.uses === 'string' && step.uses.startsWith('deepseek-harness/deepseek-harness-check@')) + if (check === undefined || !isRecord(check.env)) throw new TypeError(`${file} ${job} must run the check action with env`) + expect(String(check.env.CHECK_MODE)).toContain('vars.MERGE_POLICY_') + } + }) + + it('keeps the pre-merge build behind the merge step, which honours the mode on its failures', () => { + const steps = stepsOf(workflowJob(loadWorkflow('.github/workflows/pr-check-on-head-branch.yml'), 'pre-merge')) + const merge = steps.findIndex(step => step.id === 'merge') + const build = steps.findIndex(step => step.name === 'Run the pre-merge command') + expect(merge).toBeGreaterThanOrEqual(0) + expect(build).toBeGreaterThan(merge) + const merging = steps[merge] + if (merging === undefined || !isRecord(merging.env)) throw new TypeError('the merge step must set env') + expect(String(merging.env.CHECK_MODE)).toContain('MERGE_POLICY_PRE_MERGE') + for (const step of steps.slice(merge)) { + if (typeof step.run !== 'string') continue + expect(step.run).toContain('"$CHECK_MODE" = block') + } + for (const step of steps.slice(merge + 1)) { + if (step.name === 'Run the pre-merge command') continue + expect(step.if).toBe("steps.merge.outputs.merged == 'true'") + } + }) +})