From 835c9229fd3f1d9bb370a0ecb2d8effc4bda376b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= Date: Fri, 24 Jul 2026 16:24:48 +0800 Subject: [PATCH] ci: tier PR quality gates and automate review routing --- .github/CODEOWNERS | 4 - .github/PULL_REQUEST_TEMPLATE.md | 37 +- .github/workflows/ci.yml | 587 ++++++++++++++---- .github/workflows/reviewer-router.yml | 258 ++++++++ CONTRIBUTING.md | 47 +- docs/architecture.md | 27 +- docs/ci-pr-gates.md | 77 ++- docs/releasing.md | 4 +- scripts/ci/changed-test-packages.sh | 136 ++++ test/scripts/changed_test_packages_test.go | 272 ++++++++ test/scripts/changelog_pr_gate_test.go | 43 +- test/scripts/github_script_syntax_test.go | 95 +++ test/scripts/reviewer_router_workflow_test.go | 143 +++++ 13 files changed, 1573 insertions(+), 157 deletions(-) delete mode 100644 .github/CODEOWNERS create mode 100644 .github/workflows/reviewer-router.yml create mode 100755 scripts/ci/changed-test-packages.sh create mode 100644 test/scripts/changed_test_packages_test.go create mode 100644 test/scripts/github_script_syntax_test.go create mode 100644 test/scripts/reviewer_router_workflow_test.go diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS deleted file mode 100644 index d76f5b11..00000000 --- a/.github/CODEOWNERS +++ /dev/null @@ -1,4 +0,0 @@ -# Default code owners for all files -# These users will be automatically requested for review on PRs. - -* @DingTalk-Real-AI/cli-maintainers diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4c024e92..391364b6 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -3,22 +3,41 @@ - What changed? - Why is this change needed? +## Risk tier + +- [ ] Documentation-only: prose/assets only; no executable, generated, workflow, + packaging, or interface behavior changed +- [ ] Standard: ordinary implementation change with a stable package graph +- [ ] High-risk: workflow/policy, package graph, generated Schema/registry, + platform, auth/keychain, installer, packaging, release, transport, recovery, + or another fail-closed infrastructure change + ## Verification -For an exact in-place `CHANGELOG.md`-only pull request, the full-suite checks -may be marked `N/A`, but the targeted CHANGELOG check is required. For every -other pull request, mark the targeted check `N/A` and complete the applicable -full-suite checks. +Record the smallest targeted evidence that proves the changed behavior. Do not +repeat the entire CI suite locally only to fill this checklist: CI expands the +selected tier from documentation checks, through affected-package tests, to +the complete high-risk suite. -- [ ] Exact `CHANGELOG.md`-only check (otherwise `N/A`): +- [ ] Exact in-place `CHANGELOG.md`-only check (otherwise `N/A`): `./scripts/policy/check-changelog-pr.sh --fast-path "$(git merge-base HEAD origin/main)" HEAD` -- [ ] `make build` -- [ ] `make lint` -- [ ] `make test` -- [ ] `make policy` +- [ ] Targeted test/check commands and results: +- [ ] Behavior evidence (test name, CLI output shape, or before/after result): +- [ ] Documentation links/content/rendering checked (documentation-only, otherwise + `N/A`) +- [ ] Full local suite run because the change is high-risk (optional for other + tiers; record command/result or `N/A`) - [ ] `./scripts/policy/check-generated-drift.sh` + (when generator inputs or generated artifacts may change) - [ ] `./scripts/policy/check-command-surface.sh --strict` (if command surface changed) +- [ ] `./scripts/release/verify-package-managers.sh` + (after `make package`, if packaging or installer surfaces changed) ## Notes - Any risks, follow-up work, or intentional scope cuts + +The repository automatically requests one eligible peer reviewer, including +after a new head push when another review is needed. Once the latest push has +peer approval and all nine required checks are current and green, auto-merge +completes the PR; authors do not need to coordinate a separate routine merge. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 14bad1cb..fa443373 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,6 +25,12 @@ jobs: outputs: changelog_only: ${{ steps.classify.outputs.changelog_only }} changelog_changed: ${{ steps.classify.outputs.changelog_changed }} + docs_only: ${{ steps.classify.outputs.docs_only }} + full_suite: ${{ steps.classify.outputs.full_suite }} + release_sensitive: ${{ steps.classify.outputs.release_sensitive }} + interface_sensitive: ${{ steps.classify.outputs.interface_sensitive }} + mcp_sensitive: ${{ steps.classify.outputs.mcp_sensitive }} + edition_sensitive: ${{ steps.classify.outputs.edition_sensitive }} platform_sensitive: ${{ steps.classify.outputs.platform_sensitive }} steps: - name: Classify revision scope @@ -34,10 +40,176 @@ jobs: script: | let changelogOnly = false; let changelogChanged = false; + let docsOnly = false; + let fullSuite = context.eventName === 'push'; + let releaseSensitive = context.eventName === 'push'; + let interfaceSensitive = context.eventName === 'push'; + let mcpSensitive = context.eventName === 'push'; + let editionSensitive = context.eventName === 'push'; let platformSensitive = context.eventName === 'push'; let files = []; let fastPathTrust = 'not evaluated'; + const isDocsOnly = (filename) => + typeof filename === 'string' && + ( + filename === 'CHANGELOG.md' || + filename === 'README.md' || + filename === 'README_zh.md' || + filename === 'CONTRIBUTING.md' || + filename === 'SECURITY.md' || + filename === 'CODE_OF_CONDUCT.md' || + filename === 'LICENSE' || + filename === 'NOTICE' || + filename === '.github/PULL_REQUEST_TEMPLATE.md' || + filename.startsWith('.github/ISSUE_TEMPLATE/') || + filename.startsWith('docs/') + ); + const isReleaseSensitive = (filename) => + typeof filename === 'string' && + ( + filename.startsWith('scripts/release/') || + filename.startsWith('scripts/install') || + filename.startsWith('Formula/') || + filename.startsWith('build/') || + filename === '.goreleaser.yaml' || + filename === '.github/workflows/release.yml' || + filename === '.github/workflows/withdraw-release.yml' + ); + const isPlatformSensitive = (filename) => + typeof filename === 'string' && + ( + filename.startsWith('internal/auth/') || + filename.startsWith('internal/keychain/') || + /_(darwin|windows|linux|unix)(?:_[a-z0-9]+)*\.go$/.test(filename) || + filename === '.github/workflows/ci.yml' || + filename === + 'scripts/policy/run-platform-coverage-gate.sh' + ); + const isNativeGoChange = ({ filename, previous_filename, patch }) => { + const paths = [filename, previous_filename].filter( + (value) => typeof value === 'string' && value !== '' + ); + if (paths.some(isPlatformSensitive)) { + return true; + } + if (!paths.some((value) => value.endsWith('.go'))) { + return false; + } + if (typeof patch !== 'string') { + return true; + } + return ( + /\/\/go:build[^\n]*(darwin|windows|linux|unix)/.test(patch) || + /runtime\.GO(OS|ARCH)/.test(patch) + ); + }; + const isInterfaceSensitive = (filename) => + typeof filename === 'string' && + ( + filename.startsWith('cmd/') || + filename.startsWith('internal/app/') || + filename.startsWith('internal/cli/') || + filename.startsWith('internal/cobracmd/') || + filename.startsWith('internal/helpers/') || + filename.startsWith('internal/i18n/') || + filename.startsWith('internal/interfacesnapshot/') || + filename.startsWith('internal/ir/') || + filename.startsWith('internal/shortcut/') || + filename.startsWith('pkg/cmdutil/') || + filename === 'skills_embed.go' || + filename === 'test/fixtures/cli-interface-baseline.txt' || + filename.startsWith('skills/') + ); + const isMCPSensitive = (filename) => + typeof filename === 'string' && + ( + filename.startsWith('internal/mcp') || + filename.startsWith('internal/transport/') || + filename.startsWith('internal/helpers/') || + filename.startsWith('internal/app/') || + filename.startsWith('test/mock_mcp/') + ); + const isEditionSensitive = (filename) => + typeof filename === 'string' && + ( + filename.startsWith('pkg/edition') || + filename.startsWith('internal/edition') || + filename === 'go.mod' || + filename === 'go.sum' + ); + const isHighRisk = (filename) => + typeof filename === 'string' && + ( + isPlatformSensitive(filename) || + isReleaseSensitive(filename) || + filename.startsWith('.github/workflows/') || + filename === '.github/actionlint.yaml' || + filename.startsWith('scripts/') || + filename.startsWith('verify/') || + filename.startsWith('internal/generator/') || + filename.startsWith('internal/cli/schema') || + filename.startsWith('internal/interfacesnapshot/') || + filename.startsWith('internal/app/upgrade') || + filename.startsWith('internal/transport/') || + filename.startsWith('internal/recovery/') || + filename.startsWith('internal/syncdata/') || + filename.includes('/testdata/') || + filename.startsWith('testdata/') || + filename.startsWith('test/fixtures/') || + filename === 'Makefile' || + filename === 'go.mod' || + filename === 'go.sum' + ); + const classifyFiles = (complete) => { + const paths = files.flatMap(({ filename, previous_filename }) => + [filename, previous_filename].filter( + (value) => typeof value === 'string' && value !== '' + ) + ); + const structuralChange = files.some( + ({ status, previous_filename }) => + status === 'removed' || Boolean(previous_filename) + ); + const goPackageShapeChange = files.some( + ({ filename, previous_filename, status }) => + ['added', 'removed'].includes(status) && + [filename, previous_filename].some( + (value) => + typeof value === 'string' && value.endsWith('.go') + ) + ); + docsOnly = + complete && + files.length > 0 && + paths.length > 0 && + paths.every(isDocsOnly); + releaseSensitive = + !complete || paths.some(isReleaseSensitive); + platformSensitive = + !complete || files.some(isNativeGoChange); + interfaceSensitive = + !complete || + paths.some(isInterfaceSensitive) || + paths.some(isEditionSensitive); + mcpSensitive = + !complete || paths.some(isMCPSensitive); + editionSensitive = + !complete || paths.some(isEditionSensitive); + fullSuite = + !complete || + ( + !docsOnly && + ( + context.eventName === 'push' || + structuralChange || + goPackageShapeChange || + files.some(isNativeGoChange) || + paths.some(isHighRisk) + ) + ); + }; + if (context.eventName === 'pull_request') { const expectedHead = context.payload.pull_request.head.sha; const expectedBase = context.payload.pull_request.base.sha; @@ -85,28 +257,14 @@ jobs: filename === 'CHANGELOG.md' || previous_filename === 'CHANGELOG.md' ); - - const isPlatformSensitive = (filename) => - typeof filename === 'string' && - ( - filename.startsWith('internal/auth/') || - filename.startsWith('internal/keychain/') || - /_(darwin|windows|linux|unix)\.go$/.test(filename) || - filename.startsWith('scripts/release/') || - filename.startsWith('scripts/install') || - filename.startsWith('Formula/') || - filename.startsWith('build/npm/') || - filename === '.goreleaser.yaml' || - filename === '.github/workflows/release.yml' - ); - platformSensitive = files.some( - ({ filename, previous_filename }) => - isPlatformSensitive(filename) || - isPlatformSensitive(previous_filename) - ); + classifyFiles(true); fastPathTrust = changelogOnly ? 'exact pull-request revision and synthetic merge policy' - : 'full admission required'; + : docsOnly + ? 'documentation-only focused admission' + : fullSuite + ? 'high-risk full admission' + : 'changed-package focused admission'; } else if (context.eventName === 'push') { const expectedBefore = context.payload.before; const expectedAfter = context.payload.after; @@ -136,6 +294,7 @@ jobs: per_page: 100, }); files = Array.isArray(comparison.files) ? comparison.files : []; + classifyFiles(files.length < 300); const linearFromValidatedTip = comparison.status === 'ahead' && comparison.merge_base_commit?.sha === expectedBefore && @@ -217,12 +376,24 @@ jobs: core.setOutput('changelog_only', String(changelogOnly)); core.setOutput('changelog_changed', String(changelogChanged)); + core.setOutput('docs_only', String(docsOnly)); + core.setOutput('full_suite', String(fullSuite)); + core.setOutput('release_sensitive', String(releaseSensitive)); + core.setOutput('interface_sensitive', String(interfaceSensitive)); + core.setOutput('mcp_sensitive', String(mcpSensitive)); + core.setOutput('edition_sensitive', String(editionSensitive)); core.setOutput('platform_sensitive', String(platformSensitive)); await core.summary .addHeading('Code Admission scope') .addRaw(`- Event: \`${context.eventName}\`\n`) .addRaw(`- Exact modified CHANGELOG only: \`${changelogOnly}\`\n`) .addRaw(`- CHANGELOG touched: \`${changelogChanged}\`\n`) + .addRaw(`- Documentation-only: \`${docsOnly}\`\n`) + .addRaw(`- Full suite: \`${fullSuite}\`\n`) + .addRaw(`- Release-sensitive: \`${releaseSensitive}\`\n`) + .addRaw(`- Interface-sensitive: \`${interfaceSensitive}\`\n`) + .addRaw(`- MCP-sensitive: \`${mcpSensitive}\`\n`) + .addRaw(`- Edition-sensitive: \`${editionSensitive}\`\n`) .addRaw(`- Native-platform risk paths touched: \`${platformSensitive}\`\n`) .addRaw(`- Changed files: \`${files.length}\`\n`) .addRaw(`- Fast-path trust: ${fastPathTrust}\n`) @@ -232,36 +403,92 @@ jobs: if: steps.classify.outputs.changelog_only == 'true' run: echo "Lint is satisfied by the trusted CHANGELOG-only Policy path." >> "$GITHUB_STEP_SUMMARY" + - name: Record documentation-only fast path + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only == 'true' + run: echo "Lint is satisfied by the non-executable documentation scope." >> "$GITHUB_STEP_SUMMARY" + - name: Check out repository - if: steps.classify.outputs.changelog_only != 'true' + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only != 'true' uses: actions/checkout@v4 - name: Set up Go - if: steps.classify.outputs.changelog_only != 'true' + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only != 'true' uses: actions/setup-go@v5 with: go-version-file: go.mod - name: Verify Test Package Plan - if: steps.classify.outputs.changelog_only != 'true' + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only != 'true' run: make test-plan - name: Format Check - if: steps.classify.outputs.changelog_only != 'true' + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only != 'true' run: make format-check - name: Go Vet - if: steps.classify.outputs.changelog_only != 'true' + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only != 'true' run: go vet ./... - name: Check GitHub Actions workflows - if: steps.classify.outputs.changelog_only != 'true' + if: steps.classify.outputs.changelog_only != 'true' && steps.classify.outputs.docs_only != 'true' run: go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.12 + test-focused: + name: Test (changed packages) + needs: lint + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite != 'true' }} + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Check out repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Verify authoritative synthetic merge + env: + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -eu + test "$(git rev-parse HEAD^1)" = "$PR_BASE_SHA" || { + echo "focused test merge first parent does not match event base" >&2 + exit 1 + } + test "$(git rev-parse HEAD^2)" = "$PR_HEAD_SHA" || { + echo "focused test merge second parent does not match event head" >&2 + exit 1 + } + echo "TEST_BASE_REF=$PR_BASE_SHA" >> "$GITHUB_ENV" + echo "TEST_HEAD_REF=$(git rev-parse HEAD)" >> "$GITHUB_ENV" + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Test changed packages and reverse dependencies + shell: bash + env: + DWS_PACKAGE_VERSION: 0.0.0-test + run: | + set -euo pipefail + package_output="$( + ./scripts/ci/changed-test-packages.sh \ + list "$TEST_BASE_REF" "$TEST_HEAD_REF" + )" + if [ -z "$package_output" ]; then + echo "No buildable Go package is affected by this revision." \ + >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + mapfile -t packages <<< "$package_output" + go test -v -race -count=1 -timeout=8m "${packages[@]}" + test-race: name: "Test (race: ${{ matrix.shard }})" needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' }} runs-on: ubuntu-latest timeout-minutes: 15 strategy: @@ -299,9 +526,9 @@ jobs: go test -v -race -count=1 -timeout=10m "${packages[@]}" test-release-scripts: - name: Test (release scripts) + name: Test (workflow and release contracts) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.release_sensitive == 'true') }} runs-on: ubuntu-latest timeout-minutes: 15 steps: @@ -331,7 +558,7 @@ jobs: test-cross-platform: name: Test (cross-platform compile) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -363,6 +590,7 @@ jobs: name: Test needs: - lint + - test-focused - test-race - test-release-scripts - test-cross-platform @@ -376,7 +604,10 @@ jobs: - name: Verify test shards env: CHANGELOG_ONLY: ${{ needs.lint.outputs.changelog_only }} - PLATFORM_SENSITIVE: ${{ needs.lint.outputs.platform_sensitive }} + DOCS_ONLY: ${{ needs.lint.outputs.docs_only }} + FULL_SUITE: ${{ needs.lint.outputs.full_suite }} + RELEASE_SENSITIVE: ${{ needs.lint.outputs.release_sensitive }} + FOCUSED_RESULT: ${{ needs.test-focused.result }} RACE_RESULT: ${{ needs.test-race.result }} RELEASE_SCRIPTS_RESULT: ${{ needs.test-release-scripts.result }} CROSS_PLATFORM_RESULT: ${{ needs.test-cross-platform.result }} @@ -384,8 +615,9 @@ jobs: WINDOWS_RESULT: ${{ needs.test-windows.result }} run: | failed=0 - if [ "$CHANGELOG_ONLY" = true ]; then + if [ "$CHANGELOG_ONLY" = true ] || [ "$DOCS_ONLY" = true ]; then for shard in \ + "changed packages:$FOCUSED_RESULT" \ "race shards:$RACE_RESULT" \ "release scripts:$RELEASE_SCRIPTS_RESULT" \ "cross-platform compile:$CROSS_PLATFORM_RESULT" \ @@ -403,21 +635,34 @@ jobs: exit fi + focused_expected=success + race_expected=skipped + if [ "$FULL_SUITE" = true ]; then + focused_expected=skipped + race_expected=success + fi + release_expected=skipped + if [ "$FULL_SUITE" = true ] || [ "$RELEASE_SENSITIVE" = true ]; then + release_expected=success + fi for shard in \ - "race shards:$RACE_RESULT" \ - "release scripts:$RELEASE_SCRIPTS_RESULT" \ - "cross-platform compile:$CROSS_PLATFORM_RESULT" + "changed packages:$FOCUSED_RESULT:$focused_expected" \ + "race shards:$RACE_RESULT:$race_expected" \ + "release scripts:$RELEASE_SCRIPTS_RESULT:$release_expected" \ + "cross-platform compile:$CROSS_PLATFORM_RESULT:success" do name="${shard%%:*}" - result="${shard#*:}" - printf '%s: %s\n' "$name" "$result" - if [ "$result" != "success" ]; then + remainder="${shard#*:}" + result="${remainder%%:*}" + expected="${remainder#*:}" + printf '%s: %s (expected %s)\n' "$name" "$result" "$expected" + if [ "$result" != "$expected" ]; then failed=1 fi done native_expected=skipped - if [ "$PLATFORM_SENSITIVE" = true ]; then + if [ "$FULL_SUITE" = true ]; then native_expected=success fi for native in \ @@ -436,7 +681,7 @@ jobs: test-darwin: name: Test (macOS auth/keychain) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.platform_sensitive == 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' }} runs-on: macos-latest timeout-minutes: 15 steps: @@ -454,7 +699,7 @@ jobs: test-windows: name: Test (Windows) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.platform_sensitive == 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' }} runs-on: windows-latest timeout-minutes: 15 steps: @@ -568,13 +813,14 @@ jobs: coverage-current: name: Coverage (current) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' }} runs-on: ubuntu-latest timeout-minutes: 20 steps: - name: Check out repository uses: actions/checkout@v4 with: + fetch-depth: 0 ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} - name: Set up Go @@ -583,15 +829,69 @@ jobs: go-version-file: go.mod - name: Install archive tooling + if: needs.lint.outputs.full_suite == 'true' run: sudo apt-get update && sudo apt-get install -y zip unzip + - name: Resolve authoritative coverage base + env: + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PUSH_BEFORE_SHA: ${{ github.event.before }} + run: | + set -eu + base_ref="$PUSH_BEFORE_SHA" + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then + base_ref="$(git merge-base "$PR_HEAD_SHA" "$PR_BASE_SHA")" + fi + if [ -z "$base_ref" ] || [ "$base_ref" = "0000000000000000000000000000000000000000" ]; then + base_ref="$(git rev-parse HEAD^)" + fi + git rev-parse --verify "${base_ref}^{commit}" >/dev/null + echo "COVERAGE_BASE_REF=$base_ref" >> "$GITHUB_ENV" + echo "COVERAGE_HEAD_REF=$(git rev-parse HEAD)" >> "$GITHUB_ENV" + - name: Build run: make build - name: Run current unit tests with coverage + shell: bash + env: + DWS_PACKAGE_VERSION: 0.0.0-test + FULL_SUITE: ${{ needs.lint.outputs.full_suite }} run: | - go test -count=1 -p 1 -coverprofile=coverage.txt -covermode=atomic ./ ./cmd/... ./internal/... ./skills/... - go tool cover -func=coverage.txt + set -euo pipefail + if [ "$FULL_SUITE" = true ]; then + go test -count=1 -p 1 \ + -coverprofile=coverage.txt \ + -covermode=atomic \ + ./ ./cmd/... ./internal/... ./skills/... + else + changed_output="$( + ./scripts/ci/changed-test-packages.sh \ + changed "$COVERAGE_BASE_REF" "$COVERAGE_HEAD_REF" + )" + impacted_output="$( + ./scripts/ci/changed-test-packages.sh \ + list "$COVERAGE_BASE_REF" "$COVERAGE_HEAD_REF" + )" + if [ -z "$changed_output" ] || [ -z "$impacted_output" ]; then + printf 'mode: atomic\n' > coverage.txt + echo "No buildable Go package needs scoped coverage." \ + >> "$GITHUB_STEP_SUMMARY" + else + mapfile -t changed_packages <<< "$changed_output" + mapfile -t impacted_packages <<< "$impacted_output" + coverpkg="$(IFS=,; echo "${changed_packages[*]}")" + go test -count=1 -p 1 \ + -coverpkg="$coverpkg" \ + -coverprofile=coverage.txt \ + -covermode=atomic \ + "${impacted_packages[@]}" + fi + fi + if [ "$(wc -l < coverage.txt)" -gt 1 ]; then + go tool cover -func=coverage.txt + fi - name: Upload current coverage profile uses: actions/upload-artifact@v4 @@ -603,7 +903,7 @@ jobs: coverage-supporting: name: Coverage (supporting) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -642,7 +942,7 @@ jobs: coverage-baseline: name: Coverage (baseline) needs: lint - if: ${{ needs.lint.outputs.changelog_only != 'true' }} + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' }} runs-on: ubuntu-latest timeout-minutes: 20 steps: @@ -658,6 +958,7 @@ jobs: go-version-file: go.mod - name: Install archive tooling + if: needs.lint.outputs.full_suite == 'true' run: sudo apt-get update && sudo apt-get install -y zip unzip - name: Resolve authoritative coverage base @@ -678,8 +979,24 @@ jobs: echo "COVERAGE_BASE_REF=$base_ref" >> "$GITHUB_ENV" - name: Run baseline unit tests with coverage + shell: bash + env: + DWS_PACKAGE_VERSION: 0.0.0-test + FULL_SUITE: ${{ needs.lint.outputs.full_suite }} run: | - set -eu + set -euo pipefail + changed_output= + impacted_output= + if [ "$FULL_SUITE" != true ]; then + changed_output="$( + ./scripts/ci/changed-test-packages.sh \ + changed "$COVERAGE_BASE_REF" HEAD + )" + impacted_output="$( + ./scripts/ci/changed-test-packages.sh \ + list "$COVERAGE_BASE_REF" HEAD + )" + fi base_worktree="$(mktemp -d "${RUNNER_TEMP}/dws-coverage-base.XXXXXX")" rmdir "$base_worktree" cleanup() { @@ -689,11 +1006,25 @@ jobs: git worktree add --detach "$base_worktree" "$COVERAGE_BASE_REF" ( cd "$base_worktree" - go test -count=1 \ - -p 1 \ - -coverprofile="$GITHUB_WORKSPACE/coverage-base.txt" \ - -covermode=atomic \ - ./ ./cmd/... ./internal/... ./skills/... + if [ "$FULL_SUITE" = true ]; then + go test -count=1 \ + -p 1 \ + -coverprofile="$GITHUB_WORKSPACE/coverage-base.txt" \ + -covermode=atomic \ + ./ ./cmd/... ./internal/... ./skills/... + elif [ -z "$changed_output" ] || [ -z "$impacted_output" ]; then + printf 'mode: atomic\n' > "$GITHUB_WORKSPACE/coverage-base.txt" + else + mapfile -t changed_packages <<< "$changed_output" + mapfile -t impacted_packages <<< "$impacted_output" + coverpkg="$(IFS=,; echo "${changed_packages[*]}")" + go test -count=1 \ + -p 1 \ + -coverpkg="$coverpkg" \ + -coverprofile="$GITHUB_WORKSPACE/coverage-base.txt" \ + -covermode=atomic \ + "${impacted_packages[@]}" + fi ) - name: Upload baseline coverage profile @@ -719,6 +1050,8 @@ jobs: - name: Verify coverage profile jobs env: CHANGELOG_ONLY: ${{ needs.lint.outputs.changelog_only }} + DOCS_ONLY: ${{ needs.lint.outputs.docs_only }} + FULL_SUITE: ${{ needs.lint.outputs.full_suite }} PLATFORM_SENSITIVE: ${{ needs.lint.outputs.platform_sensitive }} CURRENT_RESULT: ${{ needs.coverage-current.result }} SUPPORTING_RESULT: ${{ needs.coverage-supporting.result }} @@ -727,22 +1060,32 @@ jobs: WINDOWS_RESULT: ${{ needs.coverage-windows.result }} run: | failed=0 - expected=success + current_expected=success + supporting_expected=skipped + baseline_expected=success native_expected=skipped - if [ "$CHANGELOG_ONLY" = true ]; then - expected=skipped - elif [ "$PLATFORM_SENSITIVE" = true ]; then + if [ "$CHANGELOG_ONLY" = true ] || [ "$DOCS_ONLY" = true ]; then + current_expected=skipped + baseline_expected=skipped + elif [ "$FULL_SUITE" = true ]; then + supporting_expected=success + fi + if [ "$CHANGELOG_ONLY" != true ] && + [ "$DOCS_ONLY" != true ] && + [ "$PLATFORM_SENSITIVE" = true ]; then native_expected=success fi for profile in \ - "current:$CURRENT_RESULT" \ - "supporting:$SUPPORTING_RESULT" \ - "baseline:$BASELINE_RESULT" + "current:$CURRENT_RESULT:$current_expected" \ + "supporting:$SUPPORTING_RESULT:$supporting_expected" \ + "baseline:$BASELINE_RESULT:$baseline_expected" do name="${profile%%:*}" - result="${profile#*:}" - printf '%s: %s\n' "$name" "$result" + remainder="${profile#*:}" + result="${remainder%%:*}" + expected="${remainder#*:}" + printf '%s: %s (expected %s)\n' "$name" "$result" "$expected" if [ "$result" != "$expected" ]; then failed=1 fi @@ -754,10 +1097,8 @@ jobs: do name="${native%%:*}" result="${native#*:}" - printf '%s: %s\n' "$name" "$result" - if [ "$CHANGELOG_ONLY" = true ]; then - native_expected=skipped - fi + printf '%s: %s (expected %s)\n' \ + "$name" "$result" "$native_expected" if [ "$result" != "$native_expected" ]; then failed=1 fi @@ -765,20 +1106,20 @@ jobs: test "$failed" -eq 0 - name: Check out repository - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' uses: actions/checkout@v4 with: fetch-depth: 0 ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} - name: Set up Go - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' uses: actions/setup-go@v5 with: go-version-file: go.mod - name: Resolve authoritative coverage base - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' env: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} @@ -796,40 +1137,52 @@ jobs: echo "COVERAGE_BASE_REF=$base_ref" >> "$GITHUB_ENV" - name: Download current coverage profile - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' uses: actions/download-artifact@v4 with: name: coverage-current-profile path: . - name: Download supporting coverage profiles - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite == 'true' uses: actions/download-artifact@v4 with: name: coverage-supporting-profiles path: . - name: Download baseline coverage profile - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' uses: actions/download-artifact@v4 with: name: coverage-baseline-profile path: . - name: Enforce coverage gate - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' env: COVERAGE_TARGET: "100" COVERAGE_ENFORCE_OVERALL: "false" COVERAGE_OVERALL_TOLERANCE: "0" - run: COVERAGE_ADDITIONAL_DIFF_PROFILE=coverage-shortcut.txt make coverage-gate BASE_REF="$COVERAGE_BASE_REF" + run: | + additional_profile= + if [ -f coverage-shortcut.txt ]; then + additional_profile=coverage-shortcut.txt + fi + COVERAGE_ADDITIONAL_DIFF_PROFILE="$additional_profile" \ + make coverage-gate BASE_REF="$COVERAGE_BASE_REF" - name: Generate coverage report - if: needs.lint.outputs.changelog_only != 'true' - run: go tool cover -html=coverage.txt -o coverage.html + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' + run: | + if [ "$(wc -l < coverage.txt)" -gt 1 ]; then + go tool cover -html=coverage.txt -o coverage.html + else + echo "No buildable Go package needed a coverage HTML report." \ + >> "$GITHUB_STEP_SUMMARY" + fi - name: Upload coverage artifact - if: needs.lint.outputs.changelog_only != 'true' + if: needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' uses: actions/upload-artifact@v4 with: name: coverage-report @@ -839,6 +1192,7 @@ jobs: coverage-policy.txt coverage-shortcut.txt coverage.html + if-no-files-found: error policy: name: Policy @@ -852,7 +1206,7 @@ jobs: fetch-depth: 0 - name: Set up Go - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} uses: actions/setup-go@v5 with: go-version-file: go.mod @@ -930,12 +1284,29 @@ jobs: echo "Only the trusted base-equivalent CHANGELOG validator ran; executable sources are unchanged." \ >> "$GITHUB_STEP_SUMMARY" + - name: Validate scoped policy + if: ${{ needs.lint.outputs.changelog_only != 'true' && (needs.lint.outputs.docs_only == 'true' || (needs.lint.outputs.full_suite != 'true' && needs.lint.outputs.interface_sensitive != 'true')) }} + env: + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PUSH_BEFORE_SHA: ${{ github.event.before }} + run: | + set -eu + base_ref="$PUSH_BEFORE_SHA" + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then + base_ref="$PR_BASE_SHA" + fi + git rev-parse --verify "${base_ref}^{commit}" >/dev/null + git diff --check "$base_ref" HEAD + ./scripts/policy/check-open-source-assets.sh + echo "Policy used the scoped content/source admission path." \ + >> "$GITHUB_STEP_SUMMARY" + - name: Build - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make build - name: Policy - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make policy interface-integrity: @@ -945,24 +1316,24 @@ jobs: timeout-minutes: 15 steps: - name: Check out repository - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} uses: actions/checkout@v4 with: fetch-depth: 0 ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} - name: Set up Go - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} uses: actions/setup-go@v5 with: go-version-file: go.mod - name: Build - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make build - name: Resolve authoritative compatibility merge-base - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} env: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} @@ -987,7 +1358,7 @@ jobs: echo "COMPATIBILITY_STABLE_REF=$stable_ref" >> "$GITHUB_ENV" - name: Check historical commands and help compatibility - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: | make authoritative-interface-integrity \ BASE_REF="$COMPATIBILITY_BASE_REF" @@ -997,16 +1368,16 @@ jobs: fi - name: Check complete Schema compatibility - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make schema-compatibility BASE_REF="$COMPATIBILITY_BASE_REF" - name: Check skill command references - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make skill-command-integrity - - name: Record CHANGELOG-only fast path - if: needs.lint.outputs.changelog_only == 'true' - run: echo "Interface Integrity is unaffected by an exact CHANGELOG-only diff." >> "$GITHUB_STEP_SUMMARY" + - name: Record scoped fast path + if: ${{ needs.lint.outputs.changelog_only == 'true' || needs.lint.outputs.docs_only == 'true' || (needs.lint.outputs.full_suite != 'true' && needs.lint.outputs.interface_sensitive != 'true') }} + run: echo "Interface Integrity is unaffected by this classified revision." >> "$GITHUB_STEP_SUMMARY" cli-smoke: name: CLI Smoke @@ -1015,26 +1386,26 @@ jobs: timeout-minutes: 10 steps: - name: Check out repository - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} uses: actions/checkout@v4 - name: Set up Go - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} uses: actions/setup-go@v5 with: go-version-file: go.mod - name: Build - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make build - name: Check public top-level commands - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.interface_sensitive == 'true') }} run: make cli-smoke - - name: Record CHANGELOG-only fast path - if: needs.lint.outputs.changelog_only == 'true' - run: echo "CLI Smoke is unaffected by an exact CHANGELOG-only diff." >> "$GITHUB_STEP_SUMMARY" + - name: Record scoped fast path + if: ${{ needs.lint.outputs.changelog_only == 'true' || needs.lint.outputs.docs_only == 'true' || (needs.lint.outputs.full_suite != 'true' && needs.lint.outputs.interface_sensitive != 'true') }} + run: echo "CLI Smoke is unaffected by this classified revision." >> "$GITHUB_STEP_SUMMARY" mock-mcp-smoke: name: Mock MCP @@ -1043,22 +1414,22 @@ jobs: timeout-minutes: 10 steps: - name: Check out repository - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.mcp_sensitive == 'true') }} uses: actions/checkout@v4 - name: Set up Go - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.mcp_sensitive == 'true') }} uses: actions/setup-go@v5 with: go-version-file: go.mod - name: Check HTTP and stdio MCP transport - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.mcp_sensitive == 'true') }} run: make mock-mcp-smoke - - name: Record CHANGELOG-only fast path - if: needs.lint.outputs.changelog_only == 'true' - run: echo "Mock MCP is unaffected by an exact CHANGELOG-only diff." >> "$GITHUB_STEP_SUMMARY" + - name: Record scoped fast path + if: ${{ needs.lint.outputs.changelog_only == 'true' || needs.lint.outputs.docs_only == 'true' || (needs.lint.outputs.full_suite != 'true' && needs.lint.outputs.mcp_sensitive != 'true') }} + run: echo "Mock MCP is unaffected by this classified revision." >> "$GITHUB_STEP_SUMMARY" edition-tests: name: Edition @@ -1067,19 +1438,19 @@ jobs: timeout-minutes: 10 steps: - name: Check out repository - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.edition_sensitive == 'true') }} uses: actions/checkout@v4 - name: Set up Go - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.edition_sensitive == 'true') }} uses: actions/setup-go@v5 with: go-version-file: go.mod - name: Run edition contract tests - if: needs.lint.outputs.changelog_only != 'true' + if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && (needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.edition_sensitive == 'true') }} run: go test -v -count=1 ./pkg/editiontest/... - - name: Record CHANGELOG-only fast path - if: needs.lint.outputs.changelog_only == 'true' - run: echo "Edition is unaffected by an exact CHANGELOG-only diff." >> "$GITHUB_STEP_SUMMARY" + - name: Record scoped fast path + if: ${{ needs.lint.outputs.changelog_only == 'true' || needs.lint.outputs.docs_only == 'true' || (needs.lint.outputs.full_suite != 'true' && needs.lint.outputs.edition_sensitive != 'true') }} + run: echo "Edition is unaffected by this classified revision." >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/reviewer-router.yml b/.github/workflows/reviewer-router.yml new file mode 100644 index 00000000..14cba345 --- /dev/null +++ b/.github/workflows/reviewer-router.yml @@ -0,0 +1,258 @@ +name: Reviewer routing + +on: + pull_request_target: + branches: [main] + types: [opened, synchronize, reopened, ready_for_review] + +# pull_request_target deliberately runs only this workflow from the protected +# base branch. Never check out or execute pull-request code here. +permissions: + contents: write + pull-requests: write + +concurrency: + group: reviewer-router-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + route: + if: github.event.pull_request.draft == false + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Route review and enable auto-merge + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const pullNumber = context.payload.pull_request.number; + const eventHeadSha = context.payload.pull_request.head.sha; + const reviewerPool = [ + 'sczheng189', + 'shangguanxuan633-lab', + 'audanye-sudo', + 'wxianfeng', + ]; + + async function getReadyEventPull(phase) { + const {data: currentPull} = await github.rest.pulls.get({ + owner, + repo, + pull_number: pullNumber, + }); + if ( + currentPull.head.sha !== eventHeadSha || + currentPull.state !== 'open' || + currentPull.draft || + currentPull.base.ref !== 'main' + ) { + core.info( + `PR #${pullNumber} state or revision no longer matches this ready-main event during ${phase}; routing stopped.`, + ); + return null; + } + return currentPull; + } + const pullRequest = await getReadyEventPull('initial read'); + if (!pullRequest) { + return; + } + const author = pullRequest.user.login.toLowerCase(); + const headSha = pullRequest.head.sha; + const latestPusher = + context.payload.action === 'synchronize' + ? context.payload.sender?.login?.toLowerCase() + : author; + + async function routeReview() { + const eligible = reviewerPool.filter( + reviewer => + reviewer.toLowerCase() !== author && + reviewer.toLowerCase() !== latestPusher, + ); + if (eligible.length === 0) { + core.warning(`No eligible reviewer remains for PR #${pullNumber}.`); + return; + } + + const alreadyRequested = + (pullRequest.requested_reviewers || []).length > 0 || + (pullRequest.requested_teams || []).length > 0; + if (alreadyRequested) { + core.info(`PR #${pullNumber} already has a requested reviewer; leaving it unchanged.`); + return; + } + + let reviews; + try { + reviews = await github.paginate(github.rest.pulls.listReviews, { + owner, + repo, + pull_number: pullNumber, + per_page: 100, + }); + } catch (error) { + core.warning( + `Could not inspect existing reviews for PR #${pullNumber}; skipping reviewer routing to avoid a duplicate request (${error.status || 'unknown status'}).`, + ); + return; + } + + const latestDecisionByLogin = new Map(); + for (const review of reviews) { + const login = review.user?.login?.toLowerCase(); + if ( + !login || + !['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes( + review.state, + ) + ) { + continue; + } + const previous = latestDecisionByLogin.get(login); + if (!previous || review.id > previous.id) { + latestDecisionByLogin.set(login, review); + } + } + const currentHeadDecision = [...latestDecisionByLogin.values()].find( + review => + review.commit_id === headSha && + eligible.some( + reviewer => + reviewer.toLowerCase() === + review.user.login.toLowerCase(), + ) && + ['APPROVED', 'CHANGES_REQUESTED'].includes(review.state), + ); + if (currentHeadDecision) { + core.info( + `PR #${pullNumber} already has a ${currentHeadDecision.state} review on its current head; leaving review ownership unchanged.`, + ); + return; + } + + const loads = new Map(eligible.map(reviewer => [reviewer, 0])); + try { + const openPullRequests = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + per_page: 100, + }); + for (const openPullRequest of openPullRequests) { + for (const reviewer of openPullRequest.requested_reviewers || []) { + const candidate = eligible.find( + login => login.toLowerCase() === reviewer.login.toLowerCase(), + ); + if (candidate) { + loads.set(candidate, loads.get(candidate) + 1); + } + } + } + } catch (error) { + core.warning( + `Could not read current reviewer load; using deterministic rotation (${error.status || 'unknown status'}).`, + ); + } + + const offset = pullNumber % eligible.length; + const rotated = eligible.slice(offset).concat(eligible.slice(0, offset)); + const tieOrder = new Map(rotated.map((reviewer, index) => [reviewer, index])); + const staleChangeRequester = [...latestDecisionByLogin.values()] + .filter(review => review.state === 'CHANGES_REQUESTED') + .sort((left, right) => right.id - left.id) + .map(review => + eligible.find( + reviewer => + reviewer.toLowerCase() === review.user.login.toLowerCase(), + ), + ) + .find(Boolean); + const ranked = [...eligible].sort( + (left, right) => + Number(right === staleChangeRequester) - + Number(left === staleChangeRequester) || + loads.get(left) - loads.get(right) || + tieOrder.get(left) - tieOrder.get(right), + ); + + for (const reviewer of ranked) { + try { + const currentPull = await getReadyEventPull('review request'); + if (!currentPull) { + return; + } + if ( + (currentPull.requested_reviewers || []).length > 0 || + (currentPull.requested_teams || []).length > 0 + ) { + core.info( + `PR #${pullNumber} received a reviewer while routing; leaving it unchanged.`, + ); + return; + } + await github.rest.pulls.requestReviewers({ + owner, + repo, + pull_number: pullNumber, + reviewers: [reviewer], + }); + core.info( + `Requested @${reviewer} for PR #${pullNumber} (open request load: ${loads.get(reviewer)}).`, + ); + return; + } catch (error) { + core.warning( + `Could not request @${reviewer} for PR #${pullNumber}; trying the next candidate (${error.status || 'unknown status'}).`, + ); + } + } + + core.warning(`No reviewer request could be created for PR #${pullNumber}.`); + } + + async function enableAutoMerge() { + try { + const currentPull = await getReadyEventPull('auto-merge enable'); + if (!currentPull) { + return; + } + if (currentPull.auto_merge) { + core.info(`Auto-merge is already enabled for PR #${pullNumber}.`); + return; + } + await github.graphql( + `mutation EnableAutoMerge($pullRequestId: ID!) { + enablePullRequestAutoMerge( + input: { + pullRequestId: $pullRequestId + mergeMethod: MERGE + } + ) { + pullRequest { + autoMergeRequest { + enabledAt + } + } + } + }`, + {pullRequestId: currentPull.node_id}, + ); + core.info(`Enabled native auto-merge for PR #${pullNumber}.`); + } catch (error) { + core.warning( + `Could not enable auto-merge for PR #${pullNumber}; checks and review can continue normally (${error.message}).`, + ); + } + } + + try { + await routeReview(); + } catch (error) { + core.warning( + `Reviewer routing hit an unexpected error for PR #${pullNumber}; review can still proceed manually (${error.message}).`, + ); + } + await enableAutoMerge(); diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 38b9e684..b9af498e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,9 @@ notes that are intentionally kept out of the repository root. ## Local Checks -Run the verification commands that match the surface you changed before you hand work back. +Run the verification commands that match the surface you changed before you +hand work back. The goal is useful, change-specific evidence, not a second +local execution of every CI job. Common repository checks already used here include: @@ -44,19 +46,48 @@ make lint git diff --check ``` +Select the PR risk tier before choosing checks: + +| Tier | Typical scope | Developer evidence | CI expansion | +|---|---|---|---| +| Documentation-only | Prose and documentation assets with no executable, generated, workflow, packaging, or interface change | Links/content/rendering plus repository asset checks | Lightweight documentation validation; all nine named contexts still report | +| Standard | Ordinary implementation work with a stable package graph | Focused unit/integration tests and observable behavior for the changed path | Race tests for changed packages and their reverse dependencies, scope-matched HEAD/base coverage, and representative Darwin/Windows compilation | +| High-risk | Workflow/policy, package graph, generated Schema/registry, platform, auth/keychain, installer, packaging, release, transport, recovery, or an unprovable infrastructure change | Relevant full or domain suite plus focused behavior evidence | Complete race suite, native platform tests, and all affected domain gates; protected `main` uses this tier | + +Classification fails closed: an incomplete diff, package add/remove/rename, or +uncertain dependency graph selects the high-risk suite. Native changed-code +coverage is additionally selected for platform-sensitive code. + ## Pull Request Checklist 1. Keep implementation and tests in sync. -2. Run `./scripts/dev/ci-local.sh`. -3. Run `./scripts/policy/check-command-surface.sh --strict` when command paths/flags change. CI also runs `./scripts/policy/check-command-compatibility.sh --base-ref --stable-ref ` against both the target branch and latest stable release. -4. Run `./scripts/policy/check-generated-drift.sh` when generated artifacts may change. -5. Run `./scripts/release/verify-package-managers.sh` when packaging or installer surfaces change (run `make package` first). -6. Update docs and `CHANGELOG.md` for behavior/interface changes. -7. Include verification evidence in your PR description. +2. Select the documentation-only, standard, or high-risk tier and run the + smallest checks that prove the change. Use `./scripts/dev/ci-local.sh` when + a complete local pass is warranted; it is not required for every ordinary + PR. +3. Include both the commands/results and user-visible or contract-level + behavior evidence in the PR description. +4. Run `./scripts/policy/check-command-surface.sh --strict` when command + paths/flags change. CI also runs + `./scripts/policy/check-command-compatibility.sh --base-ref --stable-ref ` + against both the target branch and latest stable release. +5. Run `./scripts/policy/check-generated-drift.sh` when generated artifacts may + change. +6. Run `./scripts/release/verify-package-managers.sh` when packaging or + installer surfaces change (run `make package` first). +7. Update docs and `CHANGELOG.md` for behavior/interface changes. ## Submission Flow 1. Make the smallest atomic change that satisfies the task. 2. Keep doc edits factual and limited to implemented behavior. 3. Run the relevant verification commands. -4. Report the validation results with the handoff. +4. Report the validation results and risk tier with the handoff. +5. Open a ready PR against `main`. Base-owned automation assigns one eligible + peer reviewer, balancing the current open-review load and excluding the + author. A new head push re-enters the same routing flow when the latest + revision still needs review. +6. After the latest push has one peer approval and the exact nine required + contexts are current and green, auto-merge completes the PR. If `main` + advances first, strict status checks revalidate the branch; no separate + routine merge request is needed. diff --git a/docs/architecture.md b/docs/architecture.md index d2e40eb5..9138ee73 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -52,7 +52,13 @@ run. ```mermaid flowchart TB - PR["Pull request"] --> CA["CI"] + PR["Pull request"] --> CLASSIFY["Fail-closed risk classification"] + CLASSIFY --> DOCS["Documentation-only
asset/content validation"] + CLASSIFY --> STANDARD["Standard
affected + reverse-dependent race
scope-matched HEAD/base coverage"] + CLASSIFY --> HIGH["High-risk / main
full race + native tests"] + DOCS --> CA["CI"] + STANDARD --> CA + HIGH --> CA subgraph CA_CHECKS["Nine required contexts"] L["Lint"] T["Test"] @@ -72,9 +78,16 @@ flowchart TB PLATFORM --> RELEASE ``` -Complete Multi-profile E2E and the ordinary full native-platform matrix are -downstream of PR admission. PRs still run primary-environment assurance and -fast cross-platform compilation; auth, keychain, OS-specific, installer, and -release changes additionally select native platform tests before merge. See -[`docs/ci-pr-gates.md`](ci-pr-gates.md) for the exact context and ruleset -contract. +All nine named contexts are produced for every tier. Domain-specific helpers +run when their owned surface is affected; otherwise the corresponding context +records an explicit unaffected success. Standard code changes still receive +representative Darwin/Windows compilation. High-risk PRs and protected `main` +run the complete race and native test suites, while platform-sensitive diffs +also receive native changed-code coverage. + +Review orchestration is also base-owned: it requests one eligible peer without +executing PR code, re-routes an updated head when needed, and auto-merge +completes only after the latest push has peer approval plus the current +revision's nine strict contexts. Complete Multi-profile E2E remains downstream +of PR admission. See [`docs/ci-pr-gates.md`](ci-pr-gates.md) for the exact +classification, context, reviewer, and ruleset contract. diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 25a1fffb..69f9a6e3 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -4,9 +4,9 @@ The pull-request admission layer has exactly nine required external contexts: | Required context | Contract | |---|---| -| `Lint` | Stable PR revision classification, formatting, `go vet`, and Actionlint | -| `Test` | Race/unit/release-script tests plus fast cross-platform compilation | -| `Coverage` | Overall non-regression and 100% changed-code coverage | +| `Lint` | Stable PR revision/risk classification plus applicable formatting, `go vet`, and Actionlint | +| `Test` | Tier-selected race/unit/release-script tests plus representative cross-platform compilation | +| `Coverage` | Scope-matched overall non-regression and 100% changed-code coverage | | `Policy` | Repository policy and the fail-closed CHANGELOG contract | | `Edition` | Edition contract tests | | `Interface Integrity` | CLI, Schema, Skill, and stable-release compatibility | @@ -84,13 +84,28 @@ second file but still rejects invalid dates or versions, missing bullets, placeholder `TODO`/`TBD`, unmanaged-section changes, and unsafe tree modes. Adding a second file therefore cannot bypass CHANGELOG validation. -## Platform and downstream boundaries +## Risk tiers and downstream boundaries -Ordinary PRs run the primary Linux assurance plus fast Darwin/Windows compile -checks. Full native macOS/Windows tests and platform coverage run on a PR only -when its diff touches auth, keychain, OS-specific Go files, installers, -packaging, Formulae, or release automation. Protected `main` pushes run the -complete native matrix. +`Lint` resolves the complete base/head diff before any helper is skipped. +Unknown or truncated input fails closed into the high-risk tier. + +| Tier | Selection | Admission work | +|---|---|---| +| Documentation-only | Only prose/documentation assets; no executable, generated, workflow, packaging, or interface surface | Documentation and repository-asset validation; expensive code helpers skip while every required context still succeeds | +| Standard | Ordinary code change with a stable package graph | Race tests for changed Go packages and their reverse dependencies; candidate and merge-base coverage over the same impacted scope and `coverpkg`; representative Darwin/Windows compilation | +| High-risk / protected `main` | Workflow/policy, package add/remove/rename, generated Schema/registry, platform, auth/keychain, installer, packaging, release, transport, recovery, or an unprovable infrastructure classification | Complete race suite and full native macOS/Windows tests, plus every affected domain gate | + +Domain helpers (`Edition`, `Interface Integrity`, `CLI Smoke`, and `Mock MCP`, +for example) execute their substantive suites when the diff can affect that +contract or when the high-risk tier is selected. Otherwise their stable named +contexts still report a successful, explicit unaffected result. Release-script +tests follow the same impact rule. This preserves the ruleset contract without +charging every developer for unrelated work. + +Platform-sensitive changes additionally run native changed-code coverage. +Protected `main` always runs native tests; generic portable changes are held to +the Linux changed-code gate rather than being forced to manufacture +platform-only coverage. Complete `Multi-profile E2E` is not a PR admission context. It belongs to the `Main Integration — 主干集成` workflow and runs only after a push to `main` (or @@ -116,9 +131,28 @@ flowchart TB MAIN --> RELEASE["Release delivery"] ``` +## Review ownership and auto-merge + +A base-owned `pull_request_target` workflow routes newly opened, updated, +reopened, or newly ready PRs targeting `main` to one eligible peer reviewer. It +does not check out or execute PR code, excludes both the author and the known +latest pusher, and balances the open requested-review load across the reviewed +maintainer pool. A current-head approval or change request is preserved; after +a new push, stale activity does not suppress a fresh request, and an +outstanding change requester is preferred for continuity. + +The branch ruleset keeps one human approval and all nine strict required +contexts, and requires someone other than the latest pusher to approve after +the most recent head update. Repository auto-merge is enabled for ready PRs, +so a PR merges after that approval and the current revision's nine checks are +green. If `main` advances, strict checks rerun before merge. The reviewer +router is orchestration, not a quality context, and must not be added to the +ruleset. + ## Running focused gates locally -Run the contracts relevant to the change: +Run the contracts relevant to the change. Ordinary contributors are not +expected to repeat every CI job locally: ```sh make build @@ -139,15 +173,18 @@ base_ref=$(git merge-base HEAD origin/main) ./scripts/policy/check-changelog-pr.sh --fast-path "$base_ref" HEAD ``` -`make coverage-gate` is an enforcement step, not a profile generator. CI -generates the candidate, supporting, merge-base, and (when risk-selected) -native profiles before the aggregate `Coverage` context evaluates them. The +`make coverage-gate` is an enforcement step, not a profile generator. For a +standard PR, CI derives changed packages and their reverse-dependency test +closure, then generates candidate and merge-base profiles with the same test +scope and `coverpkg`. High-risk and protected-main runs use the complete +profiles. Supporting and (when platform-selected) native profiles are +generated before the aggregate `Coverage` context evaluates them. The aggregate and native gates require 100% coverage for changed executable Go -statements. Overall coverage remains an unrounded, zero-tolerance merge-base -non-regression check. Candidate and baseline profiles are evaluated by the -same block-deduplicating checker; supporting policy and shortcut profiles -contribute to changed-code coverage only. The checked-in badge is presentation -only and is never read as a gate input. +statements. Overall coverage remains an unrounded, zero-tolerance, +scope-matched merge-base non-regression check. Candidate and baseline profiles +are evaluated by the same block-deduplicating checker; supporting policy and +shortcut profiles contribute to changed-code coverage only. The checked-in +badge is presentation only and is never read as a gate input. Compatibility checks derive authoritative Interface snapshots from the PR merge-base and the latest reachable stable release. The candidate cannot bless @@ -175,3 +212,7 @@ Do not require helper jobs, `Multi-profile E2E`, or an aggregate admission alias. Update ruleset contexts only after the new names have appeared on the protected branch, so a rename cannot silently remove enforcement or leave an unproducible required context. + +The branch ruleset also requires one approval after the latest push. Enable +repository auto-merge and automatic head-branch deletion; keep the base-owned +reviewer router outside the required-context list. diff --git a/docs/releasing.md b/docs/releasing.md index 2f888a8f..e710f0f5 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -176,7 +176,7 @@ seal job 写入 tag 后如果只因 GitHub API 瞬时 404/429/5xx 或后续 job OSS 的 `latest.txt` / `beta.txt` 是镜像频道元数据；当前仓库安装器仍主要从 GitHub/Gitee 解析版本。启用 OSS 后，发布和撤回把它作为受控分发渠道处理，保证一旦外部消费者接入该 pointer，也不会继续解析到已撤回版本；未启用时两条流程都明确跳过不存在的 OSS 渠道。 -Release workflow 会生成 Darwin/Linux 双架构 Formula，并在不可变资产逐个校验后，只提交对应 stable 或 beta Formula 文件到 `main`；并发 `main` 更新会以全新 clone 最多重试三次，绝不 force push。由于 `GITHUB_TOKEN` 的 push 不会启动另一轮 CI，workflow 只在确认该 commit 单父、唯一改动为目标 Formula、内容与本次已验证产物逐字节一致，且父 commit 九项 Code Admission 全绿后，直接为 Formula-only commit 封存同名九项成功 checks，避免下一次发布因缺失 contexts 被卡住。撤回 workflow 暂时仍使用相同模板和回退版本 checksums 打开反向 PR；问题 GitHub Release 会先被移除以阻止新安装，永久墓碑和 workflow 日志承担审计/续跑依据。 +Release workflow 会生成 Darwin/Linux 双架构 Formula，并在不可变资产逐个校验后，由 `HOMEBREW_PR_TOKEN` 所属的受控发布身份只提交对应 stable 或 beta Formula 文件到 `main`，不再创建二次发布 PR；并发 `main` 更新会以全新 clone 最多重试三次，绝不 force push。该身份的提交不会依赖另一轮 CI 来补齐证明：workflow 只在确认该 commit 单父、唯一改动为目标 Formula、内容与本次已验证产物逐字节一致，且父 commit 九项 Code Admission 全绿后，直接为 Formula-only commit 封存同名九项成功 checks，避免下一次发布因缺失 contexts 被卡住。撤回 workflow 暂时仍使用相同模板和回退版本 checksums 打开反向 PR；问题 GitHub Release 会先被移除以阻止新安装，永久墓碑和 workflow 日志承担审计/续跑依据。 ## 平台治理前置 @@ -192,6 +192,6 @@ Release workflow 会生成 Darwin/Linux 双架构 Formula，并在不可变资 - 正常 Homebrew 发布使用现有的 `HOMEBREW_PR_TOKEN` 直接提交 Formula-only commit，不再创建 Homebrew PR，也不跑权限 canary。GitHub 不允许内置 Actions App 作为当前仓库 ruleset 的 bypass actor，因此两个默认分支 ruleset 都只给该 token 所属的指定发布管理员用户 `always` bypass；仓库脚本仍会限制提交路径、校验 Ruby、禁止 force push，并在并发更新时重新基于最新 `main`。 - `HOMEBREW_PR_TOKEN` 应保持仓库范围的 `Contents: write` 与 `Pull requests: write` 权限；后者仅供撤回流程创建回退 PR。不要与 `RELEASE_GOVERNANCE_TOKEN` 复用，并定期审计 token owner 与 ruleset bypass actor 一致。 - 创建 `release-withdrawal` environment，只允许受保护分支，设置至少一名 required reviewer、禁止申请人自审并关闭管理员绕过。撤回 workflow 会通过 API 复核这些规则；任何一项缺失都会在触碰 npm、OSS、Gitee、Homebrew 或 GitHub Release 前失败。 -- 仓库或组织的 Actions 策略必须允许 `Release` 与 `Withdraw release` workflow 的 `GITHUB_TOKEN` 获得各 job 声明的 `contents: write`；Release 的 GitHub Actions identity 还必须能够直接更新两个受控 Formula 路径。若撤回凭证采用 environment secret，确认 `release-withdrawal` 审批完成后能够读取撤回所需的 npm、OSS、Gitee 和 Homebrew 凭证。 +- 仓库或组织的 Actions 策略必须允许 `Release` 与 `Withdraw release` workflow 的 `GITHUB_TOKEN` 获得各 job 声明的权限；正常发布由 `HOMEBREW_PR_TOKEN` 更新两个受控 Formula 路径，内置 token 只承担 workflow 自身声明的封板与校验写入。若撤回凭证采用 environment secret，确认 `release-withdrawal` 审批完成后能够读取撤回所需的 npm、OSS、Gitee 和 Homebrew 凭证。 immutable releases，或任一 Code Admission context 缺失、未成功时，发布脚本会自动拒绝封 tag。tag ruleset 可能来自组织层，脚本不自动推断其最终作用范围；管理员确认不能省略，脚本约定也不能替代平台强制。 diff --git a/scripts/ci/changed-test-packages.sh b/scripts/ci/changed-test-packages.sh new file mode 100755 index 00000000..3558b689 --- /dev/null +++ b/scripts/ci/changed-test-packages.sh @@ -0,0 +1,136 @@ +#!/bin/sh +# Copyright 2026 Alibaba Group +# Licensed under the Apache License, Version 2.0 (the "License"); + +set -eu + +ROOT="$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd)" + +usage() { + printf '%s\n' "usage: $0 BASE_REF HEAD_REF" >&2 + exit 2 +} + +[ "${1:-}" = changed ] || [ "${1:-}" = list ] || usage +[ "$#" -eq 3 ] || usage + +MODE="$1" +BASE_REF="$2" +HEAD_REF="$3" + +cd "$ROOT" +git rev-parse --verify --quiet "${BASE_REF}^{commit}" >/dev/null || { + printf 'changed test package base is not a commit: %s\n' "$BASE_REF" >&2 + exit 2 +} +git rev-parse --verify --quiet "${HEAD_REF}^{commit}" >/dev/null || { + printf 'changed test package head is not a commit: %s\n' "$HEAD_REF" >&2 + exit 2 +} +resolved_head="$(git rev-parse "${HEAD_REF}^{commit}")" +current_head="$(git rev-parse HEAD)" +[ "$current_head" = "$resolved_head" ] || { + printf 'changed test package head does not match the checked-out revision: expected %s, got %s\n' \ + "$resolved_head" "$current_head" >&2 + exit 2 +} +git diff --quiet HEAD -- && git diff --cached --quiet HEAD -- || { + printf '%s\n' 'changed test package planning requires a clean tracked worktree' >&2 + exit 2 +} + +workdir="$(mktemp -d "${TMPDIR:-/tmp}/dws-changed-packages.XXXXXX")" +trap 'rm -rf "$workdir"' EXIT HUP INT TERM +files="$workdir/files" +changed_packages="$workdir/changed-packages" +impacted_packages="$workdir/impacted-packages" +all_packages="$workdir/all-packages" +package_dependencies="$workdir/package-dependencies" +embed_owners="$workdir/embed-owners" +: > "$changed_packages" +: > "$impacted_packages" + +git diff --no-ext-diff --find-renames --name-only \ + --diff-filter=ACMRD "$BASE_REF" "$HEAD_REF" > "$files" + +if ! go list ./... > "$all_packages"; then + printf '%s\n' 'failed to resolve the module package graph' >&2 + exit 1 +fi +if ! go list -f \ + '{{- $import := .ImportPath -}}{{- $dir := .Dir -}}{{- range .EmbedFiles }}{{$import}}|{{$dir}}|{{.}}{{"\n"}}{{- end -}}{{- range .TestEmbedFiles }}{{$import}}|{{$dir}}|{{.}}{{"\n"}}{{- end -}}{{- range .XTestEmbedFiles }}{{$import}}|{{$dir}}|{{.}}{{"\n"}}{{- end -}}' \ + ./... > "$embed_owners"; then + printf '%s\n' 'failed to resolve embedded file ownership' >&2 + exit 1 +fi + +while IFS= read -r file; do + [ -n "$file" ] || continue + case "$file" in + CHANGELOG.md|README.md|README_zh.md|CONTRIBUTING.md|SECURITY.md|\ + CODE_OF_CONDUCT.md|LICENSE|NOTICE|docs/*|\ + .github/PULL_REQUEST_TEMPLATE.md|.github/ISSUE_TEMPLATE/*) + continue + ;; + esac + + directory="$(dirname "$file")" + if [ "$directory" = . ]; then + pattern=. + else + pattern="./$directory" + fi + if [ -d "$directory" ]; then + has_go_files=false + for go_file in "$directory"/*.go; do + if [ -f "$go_file" ]; then + has_go_files=true + break + fi + done + if [ "$has_go_files" = true ]; then + if ! package="$(go list -f '{{.ImportPath}}' "$pattern")"; then + printf 'failed to resolve changed Go package: %s\n' "$directory" >&2 + exit 1 + fi + printf '%s\n' "$package" >> "$changed_packages" + continue + fi + fi + + embedded_owner_found=false + while IFS='|' read -r package package_dir embedded_file; do + [ -n "$package" ] || continue + case "$package_dir" in + "$ROOT") embedded_path="$embedded_file" ;; + "$ROOT"/*) + embedded_path="${package_dir#"$ROOT"/}/$embedded_file" + ;; + *) continue ;; + esac + if [ "$embedded_path" = "$file" ]; then + printf '%s\n' "$package" >> "$changed_packages" + embedded_owner_found=true + fi + done < "$embed_owners" + [ "$embedded_owner_found" = true ] || continue +done < "$files" + +LC_ALL=C sort -u "$changed_packages" -o "$changed_packages" +if [ "$MODE" = changed ] || [ ! -s "$changed_packages" ]; then + cat "$changed_packages" + exit 0 +fi + +while IFS= read -r package; do + if ! go list -deps -test "$package" > "$package_dependencies"; then + printf 'failed to resolve dependencies for Go package: %s\n' "$package" >&2 + exit 1 + fi + if grep -Fxf "$changed_packages" "$package_dependencies" >/dev/null; then + printf '%s\n' "$package" + fi +done < "$all_packages" > "$impacted_packages" + +cat "$changed_packages" >> "$impacted_packages" +LC_ALL=C sort -u "$impacted_packages" diff --git a/test/scripts/changed_test_packages_test.go b/test/scripts/changed_test_packages_test.go new file mode 100644 index 00000000..3f228c0a --- /dev/null +++ b/test/scripts/changed_test_packages_test.go @@ -0,0 +1,272 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); + +package scripts_test + +import ( + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" +) + +func TestChangedTestPackagesPlansProductionReverseDependencies(t *testing.T) { + repository := newChangedTestPackagesRepository(t) + baseRef := commitChangedTestPackagesFixture(t, repository, "initial fixture") + + writeChangedTestPackagesFixture(t, repository, "base/base.go", `package base + +func Value() string { + return "changed" +} +`) + headRef := commitChangedTestPackagesFixture(t, repository, "change base") + + changed := runChangedTestPackages(t, repository, "changed", baseRef, headRef) + if want := []string{"example.com/fixture/base"}; !slices.Equal(changed, want) { + t.Fatalf("changed packages = %q, want %q", changed, want) + } + + impacted := runChangedTestPackages(t, repository, "list", baseRef, headRef) + want := []string{ + "example.com/fixture/base", + "example.com/fixture/dependent", + "example.com/fixture/testconsumer", + } + if !slices.Equal(impacted, want) { + t.Fatalf("impacted packages = %q, want %q", impacted, want) + } + if slices.Contains(impacted, "example.com/fixture/unrelated") { + t.Fatalf("impacted packages unexpectedly contain unrelated package: %q", impacted) + } +} + +func TestChangedTestPackagesMapsEmbeddedAssetsToTheirOwner(t *testing.T) { + repository := newChangedTestPackagesRepository(t) + baseRef := commitChangedTestPackagesFixture(t, repository, "initial fixture") + + writeChangedTestPackagesFixture( + t, + repository, + "assetowner/assets/message.json", + "{\"message\":\"updated\"}\n", + ) + headRef := commitChangedTestPackagesFixture(t, repository, "update embedded asset") + + want := []string{"example.com/fixture/assetowner"} + if changed := runChangedTestPackages(t, repository, "changed", baseRef, headRef); !slices.Equal(changed, want) { + t.Fatalf("changed packages = %q, want embedded owner %q", changed, want) + } + if impacted := runChangedTestPackages(t, repository, "list", baseRef, headRef); !slices.Equal(impacted, want) { + t.Fatalf("impacted packages = %q, want embedded owner %q", impacted, want) + } +} + +func TestChangedTestPackagesIgnoresDocumentationOnlyDiff(t *testing.T) { + repository := newChangedTestPackagesRepository(t) + baseRef := commitChangedTestPackagesFixture(t, repository, "initial fixture") + + writeChangedTestPackagesFixture(t, repository, "docs/guide.md", "# Updated guide\n") + headRef := commitChangedTestPackagesFixture(t, repository, "update documentation") + + for _, mode := range []string{"changed", "list"} { + if packages := runChangedTestPackages(t, repository, mode, baseRef, headRef); len(packages) != 0 { + t.Errorf("%s packages = %q, want no packages", mode, packages) + } + } +} + +func TestChangedTestPackagesRejectsMismatchedOrDirtyCheckout(t *testing.T) { + t.Run("mismatched head", func(t *testing.T) { + repository := newChangedTestPackagesRepository(t) + baseRef := commitChangedTestPackagesFixture(t, repository, "initial fixture") + writeChangedTestPackagesFixture(t, repository, "base/base.go", "package base\n\nconst Value = \"changed\"\n") + commitChangedTestPackagesFixture(t, repository, "change base") + + output, err := runChangedTestPackagesFailure(repository, "changed", baseRef, baseRef) + if err == nil || !strings.Contains(output, "head does not match the checked-out revision") { + t.Fatalf("mismatched HEAD failure = %v, output = %q", err, output) + } + }) + + t.Run("dirty tracked file", func(t *testing.T) { + repository := newChangedTestPackagesRepository(t) + headRef := commitChangedTestPackagesFixture(t, repository, "initial fixture") + writeChangedTestPackagesFixture(t, repository, "base/base.go", "package base\n\nconst Value = \"dirty\"\n") + + output, err := runChangedTestPackagesFailure(repository, "changed", headRef, headRef) + if err == nil || !strings.Contains(output, "requires a clean tracked worktree") { + t.Fatalf("dirty worktree failure = %v, output = %q", err, output) + } + }) +} + +func TestChangedTestPackagesFailsClosedWhenPackageGraphIsInvalid(t *testing.T) { + repository := newChangedTestPackagesRepository(t) + baseRef := commitChangedTestPackagesFixture(t, repository, "initial fixture") + + writeChangedTestPackagesFixture(t, repository, "dependent/dependent.go", `package dependent + +import "example.com/fixture/missing" + +func Value() string { + return missing.Value() +} +`) + headRef := commitChangedTestPackagesFixture(t, repository, "break package graph") + + script := filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh") + command := exec.Command(script, "list", baseRef, headRef) + command.Dir = repository + output, err := command.CombinedOutput() + if err == nil { + t.Fatalf("invalid package graph unexpectedly succeeded:\n%s", output) + } + if !strings.Contains(string(output), "failed to resolve the module package graph") { + t.Fatalf("invalid package graph failure = %q, want fail-closed diagnostic", output) + } +} + +func newChangedTestPackagesRepository(t *testing.T) string { + t.Helper() + + repository := t.TempDir() + for path, contents := range map[string]string{ + "go.mod": `module example.com/fixture + +go 1.22 +`, + "base/base.go": `package base + +func Value() string { + return "base" +} +`, + "dependent/dependent.go": `package dependent + +import "example.com/fixture/base" + +func Value() string { + return base.Value() +} +`, + "testconsumer/consumer.go": `package testconsumer + +func Value() string { + return "consumer" +} +`, + "testconsumer/consumer_test.go": `package testconsumer + +import ( + "testing" + + "example.com/fixture/base" +) + +func TestBaseValue(t *testing.T) { + if base.Value() == "" { + t.Fatal("base value is empty") + } +} +`, + "assetowner/asset.go": `package assetowner + +import _ "embed" + +//go:embed assets/message.json +var message []byte + +func Message() []byte { + return message +} +`, + "assetowner/assets/message.json": "{\"message\":\"initial\"}\n", + "unrelated/unrelated.go": `package unrelated + +func Value() string { + return "unrelated" +} +`, + "docs/guide.md": "# Guide\n", + } { + writeChangedTestPackagesFixture(t, repository, path, contents) + } + + projectRoot, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve project root: %v", err) + } + script, err := os.ReadFile(filepath.Join(projectRoot, "scripts", "ci", "changed-test-packages.sh")) + if err != nil { + t.Fatalf("read changed package planner: %v", err) + } + scriptPath := filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh") + if err := os.MkdirAll(filepath.Dir(scriptPath), 0o755); err != nil { + t.Fatalf("create script directory: %v", err) + } + if err := os.WriteFile(scriptPath, script, 0o755); err != nil { + t.Fatalf("copy changed package planner: %v", err) + } + + runChangedTestPackagesCommand(t, repository, "git", "init", "-q", "-b", "main") + runChangedTestPackagesCommand(t, repository, "git", "config", "user.name", "DWS CI") + runChangedTestPackagesCommand(t, repository, "git", "config", "user.email", "dws-ci@example.invalid") + return repository +} + +func writeChangedTestPackagesFixture(t *testing.T, repository, path, contents string) { + t.Helper() + + fullPath := filepath.Join(repository, filepath.FromSlash(path)) + if err := os.MkdirAll(filepath.Dir(fullPath), 0o755); err != nil { + t.Fatalf("create directory for %s: %v", path, err) + } + if err := os.WriteFile(fullPath, []byte(contents), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +func commitChangedTestPackagesFixture(t *testing.T, repository, message string) string { + t.Helper() + + runChangedTestPackagesCommand(t, repository, "git", "add", ".") + runChangedTestPackagesCommand(t, repository, "git", "commit", "-q", "-m", message) + return strings.TrimSpace(runChangedTestPackagesCommand(t, repository, "git", "rev-parse", "HEAD")) +} + +func runChangedTestPackages(t *testing.T, repository, mode, baseRef, headRef string) []string { + t.Helper() + + output := runChangedTestPackagesCommand( + t, + repository, + filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh"), + mode, + baseRef, + headRef, + ) + return strings.Fields(output) +} + +func runChangedTestPackagesFailure(repository, mode, baseRef, headRef string) (string, error) { + script := filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh") + command := exec.Command(script, mode, baseRef, headRef) + command.Dir = repository + output, err := command.CombinedOutput() + return string(output), err +} + +func runChangedTestPackagesCommand(t *testing.T, directory, name string, args ...string) string { + t.Helper() + + command := exec.Command(name, args...) + command.Dir = directory + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("%s %s failed: %v\n%s", name, strings.Join(args, " "), err, output) + } + return string(output) +} diff --git a/test/scripts/changelog_pr_gate_test.go b/test/scripts/changelog_pr_gate_test.go index 8ae839f1..537c05d7 100644 --- a/test/scripts/changelog_pr_gate_test.go +++ b/test/scripts/changelog_pr_gate_test.go @@ -434,11 +434,52 @@ func TestChangelogPRFastPathWorkflowContract(t *testing.T) { `COVERAGE_TARGET: "100"`, `COVERAGE_ENFORCE_OVERALL: "false"`, `COVERAGE_OVERALL_TOLERANCE: "0"`, - `COVERAGE_ADDITIONAL_DIFF_PROFILE=coverage-shortcut.txt`, + `additional_profile=coverage-shortcut.txt`, "run: make test-plan", "run: make format-check", "./scripts/ci/test-packages.sh list", `package_output="$(./scripts/ci/test-packages.sh list`, + "docs_only: ${{ steps.classify.outputs.docs_only }}", + "full_suite: ${{ steps.classify.outputs.full_suite }}", + "classifyFiles(files.length < 300)", + "platformSensitive =", + "files.some(isNativeGoChange)", + "paths.some(isEditionSensitive)", + `/_(darwin|windows|linux|unix)(?:_[a-z0-9]+)*\.go$/`, + "filename.startsWith('build/')", + "filename === '.github/actionlint.yaml'", + "filename.startsWith('scripts/')", + "filename.startsWith('verify/')", + "filename === 'test/fixtures/cli-interface-baseline.txt'", + "filename.startsWith('internal/interfacesnapshot/')", + "filename.startsWith('internal/cobracmd/')", + "filename.startsWith('internal/i18n/')", + "filename.startsWith('pkg/cmdutil/')", + "filename === 'skills_embed.go'", + "filename.startsWith('test/mock_mcp/')", + "name: Test (changed packages)", + "changed-test-packages.sh", + "Verify authoritative synthetic merge", + `test "$(git rev-parse HEAD^1)" = "$PR_BASE_SHA"`, + `test "$(git rev-parse HEAD^2)" = "$PR_HEAD_SHA"`, + `echo "TEST_HEAD_REF=$(git rev-parse HEAD)"`, + "list \"$TEST_BASE_REF\" \"$TEST_HEAD_REF\"", + "needs.lint.outputs.full_suite != 'true'", + `name: "Test (race: ${{ matrix.shard }})"`, + "name: Test (workflow and release contracts)", + "needs.lint.outputs.full_suite == 'true' || needs.lint.outputs.release_sensitive == 'true'", + "needs.lint.outputs.full_suite == 'true'", + "-coverpkg=\"$coverpkg\"", + "changed \"$COVERAGE_BASE_REF\" \"$COVERAGE_HEAD_REF\"", + "list \"$COVERAGE_BASE_REF\" \"$COVERAGE_HEAD_REF\"", + "current_expected=success", + "supporting_expected=skipped", + "baseline_expected=success", + "Validate scoped policy", + "Interface Integrity is unaffected by this classified revision.", + "CLI Smoke is unaffected by this classified revision.", + "Mock MCP is unaffected by this classified revision.", + "Edition is unaffected by this classified revision.", } { if !strings.Contains(admission, want) { t.Errorf("Code Admission workflow missing contract %q", want) diff --git a/test/scripts/github_script_syntax_test.go b/test/scripts/github_script_syntax_test.go new file mode 100644 index 00000000..6c18873b --- /dev/null +++ b/test/scripts/github_script_syntax_test.go @@ -0,0 +1,95 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); + +package scripts_test + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestGitHubScriptWorkflowJavaScriptParses(t *testing.T) { + t.Parallel() + + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is required to syntax-check actions/github-script steps") + } + root, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatalf("resolve repository root: %v", err) + } + paths, err := filepath.Glob(filepath.Join(root, ".github", "workflows", "*.yml")) + if err != nil { + t.Fatalf("glob workflows: %v", err) + } + + checked := 0 + for _, path := range paths { + data, readErr := os.ReadFile(path) + if readErr != nil { + t.Fatalf("read %s: %v", path, readErr) + } + var workflow any + if unmarshalErr := yaml.Unmarshal(data, &workflow); unmarshalErr != nil { + t.Fatalf("parse %s: %v", path, unmarshalErr) + } + var scripts []string + collectGitHubScripts(workflow, &scripts) + for index, script := range scripts { + checked++ + source := fmt.Sprintf( + "async function workflowScript%d() {\n%s\n}\n", + index, + script, + ) + scriptPath := filepath.Join( + t.TempDir(), + fmt.Sprintf("%s-%d.js", filepath.Base(path), index), + ) + if writeErr := os.WriteFile(scriptPath, []byte(source), 0o600); writeErr != nil { + t.Fatalf("write syntax fixture for %s: %v", path, writeErr) + } + command := exec.Command(node, "--check", scriptPath) + if output, runErr := command.CombinedOutput(); runErr != nil { + t.Errorf( + "github-script syntax failed for %s step %d: %v\n%s", + path, + index, + runErr, + output, + ) + } + } + } + if checked == 0 { + t.Fatal("no actions/github-script steps were found") + } +} + +func collectGitHubScripts(value any, scripts *[]string) { + switch typed := value.(type) { + case map[string]any: + uses, _ := typed["uses"].(string) + if strings.HasPrefix(uses, "actions/github-script@") { + if with, ok := typed["with"].(map[string]any); ok { + if script, ok := with["script"].(string); ok { + *scripts = append(*scripts, script) + } + } + } + for _, child := range typed { + collectGitHubScripts(child, scripts) + } + case []any: + for _, child := range typed { + collectGitHubScripts(child, scripts) + } + } +} diff --git a/test/scripts/reviewer_router_workflow_test.go b/test/scripts/reviewer_router_workflow_test.go new file mode 100644 index 00000000..e99070b6 --- /dev/null +++ b/test/scripts/reviewer_router_workflow_test.go @@ -0,0 +1,143 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); + +package scripts_test + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +type reviewerRouterWorkflow struct { + On map[string]reviewerRouterTrigger `yaml:"on"` + Permissions map[string]string `yaml:"permissions"` + Jobs map[string]reviewerRouterJob `yaml:"jobs"` +} + +type reviewerRouterTrigger struct { + Branches []string `yaml:"branches"` + Types []string `yaml:"types"` +} + +type reviewerRouterJob struct { + If string `yaml:"if"` + Steps []reviewerRouterStep `yaml:"steps"` +} + +type reviewerRouterStep struct { + Uses string `yaml:"uses"` + With map[string]string `yaml:"with"` +} + +func TestReviewerRouterWorkflowContract(t *testing.T) { + t.Parallel() + + path, err := filepath.Abs(filepath.Join("..", "..", ".github", "workflows", "reviewer-router.yml")) + if err != nil { + t.Fatalf("Abs(reviewer-router.yml) error = %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile(%s) error = %v", path, err) + } + + var workflow reviewerRouterWorkflow + if err := yaml.Unmarshal(data, &workflow); err != nil { + t.Fatalf("yaml.Unmarshal(%s) error = %v", path, err) + } + + if len(workflow.On) != 1 { + t.Fatalf("workflow triggers = %v, want pull_request_target only", workflow.On) + } + trigger, ok := workflow.On["pull_request_target"] + if !ok { + t.Fatalf("workflow triggers = %v, want pull_request_target", workflow.On) + } + if wantBranches := []string{"main"}; !reflect.DeepEqual(trigger.Branches, wantBranches) { + t.Fatalf("pull_request_target branches = %v, want %v", trigger.Branches, wantBranches) + } + wantTypes := []string{"opened", "synchronize", "reopened", "ready_for_review"} + if !reflect.DeepEqual(trigger.Types, wantTypes) { + t.Fatalf("pull_request_target types = %v, want %v", trigger.Types, wantTypes) + } + + wantPermissions := map[string]string{ + "contents": "write", + "pull-requests": "write", + } + if !reflect.DeepEqual(workflow.Permissions, wantPermissions) { + t.Fatalf("workflow permissions = %v, want exactly %v", workflow.Permissions, wantPermissions) + } + + if len(workflow.Jobs) != 1 { + t.Fatalf("workflow jobs = %v, want one isolated routing job", workflow.Jobs) + } + job, ok := workflow.Jobs["route"] + if !ok { + t.Fatalf("workflow jobs = %v, want route", workflow.Jobs) + } + if job.If != "github.event.pull_request.draft == false" { + t.Fatalf("route.if = %q, want non-draft guard", job.If) + } + const githubScriptSHA = "actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b" + if len(job.Steps) != 1 || job.Steps[0].Uses != githubScriptSHA { + t.Fatalf("route steps = %#v, want one pinned base-owned github-script step", job.Steps) + } + + script := job.Steps[0].With["script"] + for _, want := range []string{ + "'sczheng189'", + "'shangguanxuan633-lab'", + "'audanye-sudo'", + "'wxianfeng'", + "github.rest.pulls.get", + "currentPull.head.sha !== eventHeadSha", + "currentPull.state !== 'open'", + "currentPull.draft", + "currentPull.base.ref !== 'main'", + "getReadyEventPull('review request')", + "getReadyEventPull('auto-merge enable')", + "context.payload.action === 'synchronize'", + "context.payload.sender?.login?.toLowerCase()", + "reviewer.toLowerCase() !== author", + "reviewer.toLowerCase() !== latestPusher", + "pullRequest.requested_reviewers", + "pullRequest.requested_teams", + "github.rest.pulls.listReviews", + "['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes", + "review.commit_id === headSha", + "['APPROVED', 'CHANGES_REQUESTED'].includes(review.state)", + "review.state === 'CHANGES_REQUESTED'", + "staleChangeRequester", + "state: 'open'", + "loads.set(candidate, loads.get(candidate) + 1)", + "loads.get(left) - loads.get(right)", + "for (const reviewer of ranked)", + "github.rest.pulls.requestReviewers", + "trying the next candidate", + "Reviewer routing hit an unexpected error", + "enablePullRequestAutoMerge", + "mergeMethod: MERGE", + "core.warning", + } { + if !strings.Contains(script, want) { + t.Errorf("reviewer router script is missing contract marker %q", want) + } + } + + for _, forbidden := range []string{ + "actions/checkout", + "['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED']", + "PeterGuy326", + "core.setFailed", + } { + if strings.Contains(string(data), forbidden) { + t.Errorf("reviewer router must not contain %q", forbidden) + } + } +}