diff --git a/.github/scripts/ug-review/review.py b/.github/scripts/ug-review/review.py index bc86ec729..14906ad75 100644 --- a/.github/scripts/ug-review/review.py +++ b/.github/scripts/ug-review/review.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 -"""Post a label-triggered, LLM-generated Unity Gateway pull-request review. +"""Post a comment-triggered, LLM-generated Unity Gateway pull-request review. -The script runs from the trusted default branch under ``pull_request_target``. +The script runs from the trusted default branch, triggered by a ``/ug-review`` comment. It retrieves PR-controlled metadata and patches as untrusted text through the GitHub API; it never checks out or executes code from the PR head. """ diff --git a/.github/scripts/user-journey-required/check.py b/.github/scripts/user-journey-required/check.py index aacb9bcc3..9cf71d06b 100644 --- a/.github/scripts/user-journey-required/check.py +++ b/.github/scripts/user-journey-required/check.py @@ -1,9 +1,10 @@ #!/usr/bin/env python3 """Require integration coverage when a PR changes a ug user journey. -This script is run only from the trusted default branch by a -``pull_request_target`` workflow. PR-controlled patches are untrusted text: they -are retrieved through the GitHub API and sent to the judge, never executed. +The workflow checks this script out from the default branch and runs it automatically +for in-repo PRs, or for a fork PR when an org member comments ``/user-journey-check``. +PR-controlled patches are untrusted text: they are retrieved through the +GitHub API and sent to the judge, never executed. """ from __future__ import annotations @@ -296,6 +297,22 @@ def _sync_waiver_label() -> int: return _pass("waiver label already matches the authorized comment state.") +def _record_evaluated_head(pr_path: str, pr: dict[str, Any]) -> None: + """Pin the verdict to the head the diff was read at, for the fork-PR status report.""" + sha = str(pr.get("head", {}).get("sha", "")) + current = _gh_json(pr_path) + current_sha = str(current.get("head", {}).get("sha", "")) if isinstance(current, dict) else "" + if not re.fullmatch(r"[0-9a-f]{40}", sha) or current_sha != sha: + raise GateError( + f"The PR head changed while its diff was read ({sha[:7] or 'unknown'} -> " + f"{current_sha[:7] or 'unknown'}). Re-run this check." + ) + output = os.environ.get("GITHUB_OUTPUT") + if output: + with open(output, "a") as handle: + handle.write(f"sha={sha}\n") + + def _pass(message: str) -> int: print(f"PASS: {message}") return 0 @@ -315,6 +332,7 @@ def main() -> int: files = _gh_json(f"{pr_path}/files", paginate=True) if not isinstance(pr, dict) or not isinstance(files, list): raise GateError("GitHub returned an unexpected pull-request response.") + _record_evaluated_head(pr_path, pr) if not any(_is_product_path(str(file.get("filename", ""))) for file in files): return _pass("no ug product or packaging files changed; no journey test is required.") diff --git a/.github/workflows/fork-integration.yml b/.github/workflows/fork-integration.yml new file mode 100644 index 000000000..e7600f58d --- /dev/null +++ b/.github/workflows/fork-integration.yml @@ -0,0 +1,127 @@ +name: Fork integration + +# Fork PRs get no secrets on `pull_request`. After reviewing a PR, an org member +# comments `/integration-test` to run the Integration workflow with secrets on the PR +# head, or `/integration-test ` to run only if the head is still the reviewed +# commit. Workflow definitions always come from the default branch. +on: + issue_comment: + types: [created] + +permissions: + contents: read + +jobs: + authorize: + name: Authorize fork integration + if: >- + ${{ + github.event.issue.pull_request && + (github.event.comment.body == '/integration-test' || + startsWith(github.event.comment.body, '/integration-test ')) && + contains(fromJSON('["MEMBER", "OWNER"]'), github.event.comment.author_association) + }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + pull-requests: write + statuses: write + outputs: + sha: ${{ steps.pr.outputs.sha }} + steps: + - name: Pin the reviewed commit + id: pr + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + COMMENT_ID: ${{ github.event.comment.id }} + COMMENT_BODY: ${{ github.event.comment.body }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + refuse() { + gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --body "Fork integration not started: $1" + echo "::error::$1" + exit 1 + } + [[ "$COMMENT_BODY" =~ ^/integration-test([[:space:]]+([0-9a-f]{7,40}))?[[:space:]]*$ ]] \ + || refuse "use \`/integration-test\`, or \`/integration-test \` with the head commit you reviewed." + requested="${BASH_REMATCH[2]}" + pr=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER") + sha=$(jq -r '.head.sha' <<<"$pr") + head_repo=$(jq -r '.head.repo.full_name // empty' <<<"$pr") + [[ "$sha" =~ ^[0-9a-f]{40}$ ]] || refuse "could not read the PR head commit." + [ -n "$head_repo" ] || refuse "the fork repository no longer exists." + # Naming the commit binds the run to what was reviewed, not to a later push. + [[ -z "$requested" || "$sha" == "$requested"* ]] \ + || refuse "the PR head is now ${sha:0:7}, not $requested. Review the new commits and comment again." + gh api -X POST "repos/$GITHUB_REPOSITORY/issues/comments/$COMMENT_ID/reactions" \ + -f content=eyes >/dev/null + gh api -X POST "repos/$GITHUB_REPOSITORY/statuses/$sha" -f state=pending \ + -f context="Fork integration" -f description="Running on ${sha:0:7}" \ + -f target_url="$RUN_URL" >/dev/null + echo "sha=$sha" >> "$GITHUB_OUTPUT" + + reject: + name: Reject non-member request + if: >- + ${{ + github.event.issue.pull_request && + (github.event.comment.body == '/integration-test' || + startsWith(github.event.comment.body, '/integration-test ')) && + !contains(fromJSON('["MEMBER", "OWNER"]'), github.event.comment.author_association) + }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + pull-requests: write + steps: + - name: Reply with the member-only notice + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + COMMENTER: ${{ github.event.comment.user.login }} + run: | + gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --body \ + "Sorry @$COMMENTER, only Databricks org members can run integration tests. A maintainer will run \`/integration-test\` after reviewing the changes." + + integration: + name: Integration + needs: authorize + # Job-level so ordinary PR comments, which skip this job, never cancel a run. + concurrency: + group: fork-integration-${{ github.event.issue.number }} + cancel-in-progress: true + permissions: + contents: read + id-token: write + uses: ./.github/workflows/integration.yml + with: + ref: ${{ needs.authorize.outputs.sha }} + secrets: inherit # zizmor: ignore[secrets-inherit] same-repo workflow that needs nearly every secret + + report: + name: Report fork integration + needs: [authorize, integration] + if: ${{ always() && needs.authorize.result == 'success' }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + statuses: write + pull-requests: write + steps: + - name: Post the result on the PR + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + SHA: ${{ needs.authorize.outputs.sha }} + RESULT: ${{ needs.integration.result }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + state=$([ "$RESULT" = success ] && echo success || echo failure) + gh api -X POST "repos/$GITHUB_REPOSITORY/statuses/$SHA" \ + -f state="$state" -f context="Fork integration" \ + -f description="Integration $RESULT on ${SHA:0:7}" -f target_url="$RUN_URL" >/dev/null + gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \ + --body "Fork integration $RESULT on ${SHA:0:7}: $RUN_URL" diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index f6730bb16..df7c308ae 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -2,6 +2,11 @@ name: Integration on: workflow_call: + inputs: + ref: + description: Commit to test; fork-integration.yml passes the PR head pinned by /integration-test + type: string + default: '' workflow_dispatch: inputs: ug_version: @@ -38,7 +43,7 @@ permissions: contents: read concurrency: - group: integration-${{ github.event.pull_request.number || github.ref }}-${{ github.event_name }} + group: integration-${{ github.event.pull_request.number || github.event.issue.number || github.ref }}-${{ github.event_name }} cancel-in-progress: true env: @@ -61,14 +66,16 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - fetch-depth: 0 + ref: ${{ inputs.ref || github.sha }} persist-credentials: false + fetch-depth: 0 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22.19.0 - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - name: Test a fresh installed package without credentials run: | uv run --no-project --python 3.12 python scripts/run_integration.py \ @@ -103,14 +110,16 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - fetch-depth: 0 + ref: ${{ inputs.ref || github.sha }} persist-credentials: false + fetch-depth: 0 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22.19.0 - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - name: Authenticate to JFrog with GitHub OIDC id: jfrog uses: jfrog/setup-jfrog-cli@279b1f629f43dd5bc658d8361ac4802a7ef8d2d5 # v4.9.1 @@ -204,14 +213,16 @@ jobs: steps: &live-steps - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - fetch-depth: 0 + ref: ${{ inputs.ref || github.sha }} persist-credentials: false + fetch-depth: 0 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22.19.0 - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - uses: databricks/setup-cli@bdb89f81c11a5bd647fd55b585b7c396ec68a25a # v1.0.0 with: version: 1.17.0 @@ -281,14 +292,16 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - fetch-depth: 0 + ref: ${{ inputs.ref || github.sha }} persist-credentials: false + fetch-depth: 0 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22.19.0 - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - uses: databricks/setup-cli@bdb89f81c11a5bd647fd55b585b7c396ec68a25a # v1.0.0 with: version: 1.17.0 @@ -424,14 +437,16 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - fetch-depth: 0 + ref: ${{ inputs.ref || github.sha }} persist-credentials: false + fetch-depth: 0 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22.19.0 - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - uses: databricks/setup-cli@bdb89f81c11a5bd647fd55b585b7c396ec68a25a # v1.0.0 with: version: 1.17.0 @@ -481,14 +496,16 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - fetch-depth: 0 + ref: ${{ inputs.ref || github.sha }} persist-credentials: false + fetch-depth: 0 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22.19.0 - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - uses: databricks/setup-cli@bdb89f81c11a5bd647fd55b585b7c396ec68a25a # v1.0.0 with: version: 1.17.0 @@ -537,6 +554,7 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: + ref: ${{ inputs.ref || github.sha }} persist-credentials: false - name: Discover every E2E CUJ file id: discover @@ -574,6 +592,7 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: + ref: ${{ inputs.ref || github.sha }} persist-credentials: false - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: @@ -581,6 +600,7 @@ jobs: - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 with: version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} - uses: databricks/setup-cli@bdb89f81c11a5bd647fd55b585b7c396ec68a25a # v1.0.0 with: version: 1.17.0 diff --git a/.github/workflows/ug-review.yml b/.github/workflows/ug-review.yml index d23145e10..8a6eca127 100644 --- a/.github/workflows/ug-review.yml +++ b/.github/workflows/ug-review.yml @@ -1,27 +1,28 @@ name: UG Review -# The reviewer always runs trusted code from the default branch. PR-controlled -# metadata and patches are fetched as untrusted text and are never executed. +# An org member comments `/ug-review` on a PR. The reviewer always runs trusted code from +# the default branch. PR-controlled metadata and patches are fetched as untrusted text and +# are never executed. on: - pull_request_target: # zizmor: ignore[dangerous-triggers] - types: [opened, synchronize, reopened, ready_for_review, labeled] + issue_comment: + types: [created] permissions: contents: read -concurrency: - group: ug-review-${{ github.event.pull_request.number }} - cancel-in-progress: true - jobs: review: name: Review with the UG rubric if: >- ${{ - !github.event.pull_request.draft && - contains(github.event.pull_request.labels.*.name, 'ug-review') && - (github.event.action != 'labeled' || github.event.label.name == 'ug-review') + github.event.issue.pull_request && + github.event.comment.body == '/ug-review' && + contains(fromJSON('["MEMBER", "OWNER"]'), github.event.comment.author_association) }} + # Job-level so ordinary PR comments, which skip this job, never cancel a review. + concurrency: + group: ug-review-${{ github.event.issue.number }} + cancel-in-progress: true permissions: contents: read issues: write @@ -46,7 +47,7 @@ jobs: env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ github.event.issue.number }} DATABRICKS_HOST: https://ai-oss-ecosystem-integration-testing.cloud.databricks.com DATABRICKS_CLIENT_ID: 6476a5e7-7f32-4073-abc5-ee30bb7d0060 DATABRICKS_CLIENT_SECRET: ${{ secrets.UG_CLI_REVIEW_BOT_SP_CLIENT_SECRET }} diff --git a/.github/workflows/user-journey-test-required.yml b/.github/workflows/user-journey-test-required.yml index 7c3243317..8979618d0 100644 --- a/.github/workflows/user-journey-test-required.yml +++ b/.github/workflows/user-journey-test-required.yml @@ -1,27 +1,40 @@ name: User Journey Test Required -# This gate runs from the trusted base branch and reads PR patches through the -# GitHub API. It never checks out or executes code from the PR head, including -# for fork PRs. The Databricks bearer is sent only to the configured workspace. +# This gate runs the default branch's script and reads PR patches through the GitHub +# API; it never executes code from the PR head. It runs automatically for in-repo PRs. +# Fork PRs get no secrets on `pull_request`, so an org member comments +# `/user-journey-check` to run it for them. on: - pull_request_target: # zizmor: ignore[dangerous-triggers] + pull_request: types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled] + issue_comment: + types: [created] permissions: contents: read -concurrency: - group: user-journey-test-required-${{ github.event.pull_request.number }} - cancel-in-progress: true - jobs: require-user-journey-test: name: User Journey Test Required - if: ${{ !github.event.pull_request.draft }} + if: >- + ${{ + (github.event_name == 'pull_request' && + !github.event.pull_request.draft && + github.event.pull_request.head.repo.full_name == github.repository) || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + github.event.comment.body == '/user-journey-check' && + contains(fromJSON('["MEMBER", "OWNER"]'), github.event.comment.author_association)) + }} + # Job-level so ordinary PR comments, which skip this job, never cancel a check. + concurrency: + group: user-journey-test-required-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true permissions: contents: read issues: read - pull-requests: read + pull-requests: write + statuses: write runs-on: group: databricks-protected-runner-group labels: linux-ubuntu-latest @@ -38,13 +51,14 @@ jobs: persist-credentials: false - name: Require meaningful user-journey coverage + id: gate # Temporarily advisory while runner access is blocked by the org IP allowlist. # Keep failures visible in logs without failing the required job check. continue-on-error: true env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} DATABRICKS_HOST: ${{ secrets.UCODE_TEST_WORKSPACE }} DATABRICKS_BEARER: ${{ secrets.DATABRICKS_BEARER }} USER_JOURNEY_JUDGE_MODEL: ${{ vars.UG_CI_USER_JOURNEY_JUDGE_MODEL }} @@ -53,3 +67,27 @@ jobs: run: | : "${USER_JOURNEY_JUDGE_MODEL:?Set the UG_CI_USER_JOURNEY_JUDGE_MODEL repository variable to a Responses-capable system.ai model}" python3 .github/scripts/user-journey-required/check.py + + # Comment-triggered runs are not attached to the PR, so report back on the evaluated commit. + - name: Report the result on the fork PR + if: ${{ github.event_name == 'issue_comment' }} + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.issue.number }} + OUTCOME: ${{ steps.gate.outcome }} + SHA: ${{ steps.gate.outputs.sha }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + sha=$SHA + if ! [[ "$sha" =~ ^[0-9a-f]{40}$ ]]; then + gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \ + --body "User-journey check could not pin the evaluated commit; comment \`/user-journey-check\` to retry: $RUN_URL" + exit 1 + fi + state=$([ "$OUTCOME" = success ] && echo success || echo failure) + gh api -X POST "repos/$GITHUB_REPOSITORY/statuses/$sha" -f state="$state" \ + -f context="User Journey Test Required" -f description="User-journey check $OUTCOME" \ + -f target_url="$RUN_URL" >/dev/null + gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \ + --body "User-journey check $OUTCOME on ${sha:0:7}: $RUN_URL" diff --git a/AGENTS.md b/AGENTS.md index b4223bacd..369d5a7da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,6 +27,7 @@ Tests live in `tests/`. - Use `acquire_exclusive_file_lock` / `release_file_lock` from `ucode.os_compatibility.file_lock_cross_os` instead of calling `fcntl.flock`, `fcntl.lockf`, or `msvcrt.locking` directly. Ruff enforces this outside the compatibility module and tests. - Use `launcher.exec_or_spawn` when handing the terminal to an agent. Shell strings and explicit executable overrides keep their existing subprocess semantics. - Add or update focused tests for behavior changes. +- Never trigger a workflow with `pull_request_target`; `tests/test_integration_contract.py` rejects it. Use `pull_request` for checks that need no secrets, and a member-only `issue_comment` command that checks out the default branch (as in `fork-integration.yml`) for checks that need secrets on fork PRs. - Do not modify generated or lock files unless the dependency graph intentionally changes. ## Smart-routing configuration diff --git a/tests/README.md b/tests/README.md index 9e9a0667b..2b3bccb2f 100644 --- a/tests/README.md +++ b/tests/README.md @@ -411,6 +411,15 @@ or skipped dependency fails the gate, and a running integration suite keeps it pending. The advisory Windows installation and headless lanes are not part of `All integration tests` yet. The descriptive jobs provide the actual coverage and diagnostics. +Fork PRs get no secrets, so their `pull_request` run skips every credentialed integration +job. After reviewing a PR, an org member comments `/integration-test` to run the Integration +workflow with secrets on the PR's current head commit, or `/integration-test ` to run +only if the head is still the commit they reviewed. It posts a `Fork integration` status, +and works on in-repo PRs too as an on-demand rerun. Each new push needs a new command. +It runs main's workflow definitions and never caches fork builds. `/ug-review` requests +the UG rubric review the same way, and `/user-journey-check` runs the user-journey gate, +which runs automatically only for in-repo PRs. + ## Gaps and deferred scope Custom OAuth lock tests cover release after use, Windows contention retries, and diff --git a/tests/test_integration_contract.py b/tests/test_integration_contract.py index 8e41aac1f..e46de2bb3 100644 --- a/tests/test_integration_contract.py +++ b/tests/test_integration_contract.py @@ -187,6 +187,158 @@ def test_ci_windows_lanes_skip_fork_pull_requests(): ) in job, f"{name} logs in to JFrog with OIDC on Windows, which fork PRs can't do" +MEMBER_ONLY = 'contains(fromJSON(\'["MEMBER", "OWNER"]\'), github.event.comment.author_association)' + + +def _workflow(name): + return (Path(__file__).parent.parent / ".github/workflows" / name).read_text() + + +def test_fork_integration_runs_only_member_requested_pr_heads(): + workflow = _workflow("fork-integration.yml") + authorize = workflow.split("\n authorize:\n", 1)[1].split("\n reject:\n", 1)[0] + reject = workflow.split("\n reject:\n", 1)[1].split("\n integration:\n", 1)[0] + integration = workflow.split("\n integration:\n", 1)[1].split("\n report:\n", 1)[0] + report = workflow.split("\n report:\n", 1)[1] + + assert re.search(r"(?m)^on:\n issue_comment:\n types: \[created\]\n", workflow) + assert "pull_request_target" not in workflow + assert "actions/checkout" not in workflow, "Only integration.yml checks out the pinned commit" + command = "startsWith(github.event.comment.body, '/integration-test ')" + assert command in authorize and command in reject + assert MEMBER_ONLY in authorize + assert f"!{MEMBER_ONLY}" in reject + assert "only Databricks org members can run integration tests" in reject + assert "set -euo pipefail" in authorize + assert '[[ "$sha" =~ ^[0-9a-f]{40}$ ]]' in authorize + assert "state=pending" in authorize + assert "needs: authorize" in integration + assert "group: fork-integration-${{ github.event.issue.number }}" in integration + assert "uses: ./.github/workflows/integration.yml" in integration + assert "ref: ${{ needs.authorize.outputs.sha }}" in integration + assert "needs.authorize.result == 'success'" in report + assert "repos/$GITHUB_REPOSITORY/statuses/$SHA" in report + assert "concurrency:" not in workflow.split("\njobs:\n", 1)[0], ( + "Workflow-level concurrency would let any PR comment cancel a run" + ) + + +HEAD = "0123456789abcdef0123456789abcdef01234567" + + +@pytest.mark.skipif(sys.platform == "win32", reason="the authorize step runs bash on Linux") +@pytest.mark.parametrize( + ("comment", "head_repo", "accepted", "reason"), + [ + ("/integration-test", "fork/unity-gateway", True, ""), + (f"/integration-test {HEAD[:7]}", "fork/unity-gateway", True, ""), + (f"/integration-test {HEAD}", "fork/unity-gateway", True, ""), + ("/integration-test", "databricks/unity-gateway", True, ""), + ("/integration-test 1234567", "fork/unity-gateway", False, "the PR head is now 0123456"), + ("/integration-test please", "fork/unity-gateway", False, "use `/integration-test`"), + ("/integration-test", None, False, "fork repository no longer exists"), + ], +) +def test_fork_integration_authorize_pins_the_pr_head_or_the_reviewed_commit( + tmp_path, comment, head_repo, accepted, reason +): + workflow = _workflow("fork-integration.yml") + authorize = workflow.split("\n authorize:\n", 1)[1].split("\n reject:\n", 1)[0] + script = textwrap.dedent(authorize.split(" run: |\n", 1)[1]) + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + pr = {"head": {"sha": HEAD, "repo": {"full_name": head_repo} if head_repo else None}} + (tmp_path / "pr.json").write_text(json.dumps(pr)) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + "#!/bin/sh\n" + f'echo "$*" >> "{tmp_path}/calls"\n' + f'case "$*" in "api repos/databricks/unity-gateway/pulls/7") cat "{tmp_path}/pr.json";; esac\n' + ) + fake_gh.chmod(0o755) + output = tmp_path / "github-output" + result = subprocess.run( + ["bash", "-c", script], + env={ + "PATH": f"{bin_dir}:{os.environ['PATH']}", + "GITHUB_REPOSITORY": "databricks/unity-gateway", + "GITHUB_OUTPUT": str(output), + "PR_NUMBER": "7", + "COMMENT_ID": "99", + "COMMENT_BODY": comment, + "RUN_URL": "https://example/run", + }, + capture_output=True, + text=True, + timeout=10, + ) + calls = (tmp_path / "calls").read_text() + + if accepted: + assert result.returncode == 0, result.stdout + result.stderr + assert output.read_text() == f"sha={HEAD}\n" + assert f"statuses/{HEAD} -f state=pending" in calls + else: + assert result.returncode != 0 + assert not output.exists() + assert reason in calls and "statuses/" not in calls + + +def test_no_workflow_uses_pull_request_target(): + # GitHub disables pull_request_target by default in public repos from 2026-11-02. + workflows = sorted((Path(__file__).parent.parent / ".github/workflows").glob("*.yml")) + + assert workflows + assert [path.name for path in workflows if "pull_request_target" in path.read_text()] == [] + + +def test_user_journey_gate_runs_for_in_repo_prs_or_a_member_command(): + workflow = _workflow("user-journey-test-required.yml") + trigger = workflow.split("\non:\n", 1)[1].split("\npermissions:\n", 1)[0] + + assert " pull_request:\n" in trigger and " issue_comment:\n types: [created]" in trigger + assert "github.event.pull_request.head.repo.full_name == github.repository" in workflow + assert "github.event.comment.body == '/user-journey-check'" in workflow + assert MEMBER_ONLY in workflow + assert "ref: ${{ github.event.repository.default_branch }}" in workflow + assert "concurrency:" not in workflow.split("\njobs:\n", 1)[0] + + +def test_user_journey_fork_report_uses_the_evaluated_commit(): + report = _workflow("user-journey-test-required.yml").split( + "Report the result on the fork PR", 1 + )[1] + + assert "SHA: ${{ steps.gate.outputs.sha }}" in report + assert "repos/$GITHUB_REPOSITORY/statuses/$sha" in report + assert "/pulls/" not in report and ".head.sha" not in report + + +def test_ug_review_runs_only_on_member_command_from_the_default_branch(): + workflow = _workflow("ug-review.yml") + + assert re.search(r"(?m)^on:\n issue_comment:\n types: \[created\]\n", workflow) + assert "pull_request_target" not in workflow + assert "github.event.comment.body == '/ug-review'" in workflow + assert MEMBER_ONLY in workflow + assert "ref: ${{ github.event.repository.default_branch }}" in workflow + assert "PR_NUMBER: ${{ github.event.issue.number }}" in workflow + assert "concurrency:" not in workflow.split("\njobs:\n", 1)[0] + + +def test_integration_checkouts_and_caches_are_safe_for_fork_heads(): + workflow = (Path(__file__).parent.parent / ".github/workflows/integration.yml").read_text() + checkouts = re.findall(r"(?m)^ - uses: actions/checkout@.*\n((?: .*\n)*)", workflow) + caches = re.findall(r"(?m)^ - uses: astral-sh/setup-uv@.*\n((?: .*\n)*)", workflow) + + assert checkouts and caches + for block in checkouts: + assert "ref: ${{ inputs.ref || github.sha }}" in block, block + assert "persist-credentials: false" in block, block + for block in caches: + assert "enable-cache: ${{ inputs.ref == '' }}" in block, block + + def test_windows_integration_ci_uses_shared_claude_version(): workflow = Path(__file__).parent.parent / ".github/workflows/integration.yml" contents = workflow.read_text() diff --git a/tests/test_ug_review_bot.py b/tests/test_ug_review_bot.py index 4fa23c4dc..b7f1f7f33 100644 --- a/tests/test_ug_review_bot.py +++ b/tests/test_ug_review_bot.py @@ -1,4 +1,4 @@ -"""Focused tests for the label-triggered UG review bot.""" +"""Focused tests for the comment-triggered UG review bot.""" from __future__ import annotations diff --git a/tests/test_user_journey_gate.py b/tests/test_user_journey_gate.py index 6f2a6769d..4609a55c3 100644 --- a/tests/test_user_journey_gate.py +++ b/tests/test_user_journey_gate.py @@ -158,3 +158,56 @@ def test_waiver_command_must_match_exactly(): ] assert gate._waiver_status(comments, {"rohita5l", "lilly-luo"}) == (False, "") + + +EVALUATED = "a" * 40 + + +def _gate_with_heads(monkeypatch, tmp_path, *heads, filename="README.md"): + output = tmp_path / "github-output" + output.touch() + monkeypatch.setenv("REPO", "databricks/unity-gateway") + monkeypatch.setenv("PR_NUMBER", "1") + monkeypatch.setenv("GITHUB_OUTPUT", str(output)) + pulls = iter({"title": "Change", "head": {"sha": sha}} for sha in heads) + files = [{"filename": filename, "status": "modified", "patch": "+change"}] + + def gh_json(path, paginate=False): + if path.endswith("/files"): + return files + if path.endswith("/comments"): + return [] + return next(pulls) + + monkeypatch.setattr(gate, "_gh_json", gh_json) + return gate.main(), output.read_text() + + +def test_gate_records_the_head_its_diff_was_read_at(monkeypatch, tmp_path): + assert _gate_with_heads(monkeypatch, tmp_path, EVALUATED, EVALUATED) == ( + 0, + f"sha={EVALUATED}\n", + ) + + +def test_gate_fails_without_a_verdict_sha_when_the_head_moves(monkeypatch, tmp_path, capsys): + assert _gate_with_heads(monkeypatch, tmp_path, EVALUATED, "b" * 40) == (1, "") + assert ( + "The PR head changed while its diff was read (aaaaaaa -> bbbbbbb)" + in capsys.readouterr().out + ) + + +def test_gate_records_the_head_when_the_verdict_fails(monkeypatch, tmp_path): + policy = tmp_path / "AGENTS.md" + policy.write_text("policy") + monkeypatch.setenv("TEST_POLICY_PATH", str(policy)) + monkeypatch.setenv("USER_JOURNEY_WAIVER_ADMINS", "rohita5l") + monkeypatch.setattr(gate, "_judge", lambda title, diff, test_policy: (True, "new launch flow")) + + assert _gate_with_heads( + monkeypatch, tmp_path, EVALUATED, EVALUATED, filename="src/ucode/cli.py" + ) == ( + 1, + f"sha={EVALUATED}\n", + )