Skip to content

Skip Windows CI lanes on fork PRs - #1080

Merged
rohita5l merged 2 commits into
mainfrom
david/ci-windows-fork-guard
Oct 12, 2026
Merged

rohita5l merged 2 commits into
mainfrom
david/ci-windows-fork-guard

Conversation

@david-siqi-liu

Copy link
Copy Markdown
Collaborator

The Windows unit and agent launch lanes added to ci.yml in #1025 log in to JFrog with a GitHub OIDC token (id-token: write). Pull requests from forks never get one, so every Windows job in ci.yml fails on fork PRs ("Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable"), and the test, All agent tests and e2e gates fail with them. See #901 and #1049. #1043 fixed the same problem for integration.yml, but #1025 merged afterwards and its contract test only reads integration.yml.

  • test and e2e-shards build their os matrix with the same fork guard integration.yml uses, so fork PRs run the Linux lanes only. Same-repo PRs, push and merge_group still run Windows. The gates need no change because fork PRs never create the Windows jobs.
  • tests/test_integration_contract.py: test_ci_windows_lanes_skip_fork_pull_requests requires every ci.yml job that uses the JFrog login to carry the guard. Restoring the old os list fails it.

Fork PRs still get no Windows coverage. Running the credentialed suites for a fork PR after a team member approves remains the follow-up noted in #1043.

This pull request and its description were written by Isaac.

The Windows unit and e2e lanes (#1025) log in to JFrog with a GitHub OIDC
token, which fork PRs never get, so every Windows job failed on fork PRs.
Run those matrices Linux-only for fork PRs, using the same guard as
integration.yml (#1043), and extend the fork contract test to ci.yml.

Co-authored-by: Isaac <no-reply@databricks.com>
Comment thread .github/workflows/ci.yml
@rohita5l
rohita5l disabled auto-merge October 12, 2026 13:09
@rohita5l
rohita5l added this pull request to the merge queue Oct 12, 2026
Merged via the queue into main with commit 852d5ff Oct 12, 2026
54 checks passed
@rohita5l
rohita5l deleted the david/ci-windows-fork-guard branch October 12, 2026 13:14
max-rozen-oss-db added a commit to max-rozen-oss-db/ucode that referenced this pull request Oct 12, 2026
Drops databricks#1080's fork guard and contract test: without JFrog the Windows lanes
need no OIDC token, so fork PRs can run them.

Co-authored-by: Isaac <no-reply@databricks.com>
david-siqi-liu pushed a commit to yaoharry/unity-gateway that referenced this pull request Oct 13, 2026
Fork PRs fail every Windows job in `ci.yml` because
`windows-server-latest` can't reach public PyPI or npm, so it installs
through JFrog with a GitHub OIDC token, and fork PRs never get one
(`Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable`, see databricks#1049).
This moves the Windows lanes to GitHub-hosted `windows-latest`, which
reaches public registries, and removes JFrog.

- `ci.yml`, `integration.yml`: `windows-server-latest` →
`windows-latest`. Drop the JFrog login steps, `id-token: write`, the
OIDC credential env, the JFrog index/registry URLs, and the Windows-only
install branches.
- `installation-windows` is credential-free now, so it drops its fork
guard and runs on fork PRs. Live Windows jobs still skip forks because
they need workspace secrets.
- `scripts/run_integration.py`: remove the installer credential
passthrough, `npm_user_config`, and `.npmrc` writing that databricks#786 added for
JFrog. Nothing sets those env vars anymore. `--default-index`,
`--npm-registry`, and output redaction stay.
- Docs/comments updated.

Reverts databricks#1080: its `os` matrix fork guard and
`test_ci_windows_lanes_skip_fork_pull_requests` only existed because the
Windows lanes needed a JFrog OIDC token. This PR is opened from a fork
to show the Windows lanes pass under fork conditions.

Known gap: JFrog served a Gemini CLI build under ucode's version
ceiling; public npm serves the latest, so `TestGeminiLaunch` will skip
on Windows like it already does on Linux.

Tests: `uv run pytest tests/test_integration_contract.py
tests/test_integration_runner.py` (80 passed after merging main), `just
lint` clean. This fork PR's CI is green on `windows-latest` with no OIDC
errors: Windows unit tests ran in full (3908 passed), Windows
installation tests passed, and the agent CLIs installed from public npm.
The credentialed e2e and live Windows journeys skip on forks, so they
still need a same-repo run.

_This pull request and its description were written by the Isaac LLM
tool (Claude Code, session `f636bd94-7485-4f34-ae16-732fc5ee0867`)._

---------

Co-authored-by: Isaac <no-reply@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants