diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 3d7cfd2..dbba1b0 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -16,7 +16,7 @@ reference and its `harness-ref` input to the same immutable commit SHA. ## Gateway setup: local CI and managed deployment -The reviewer invokes [`setup-openshell`](../actions/setup-openshell/action.yml) +For local CI, the reviewer invokes [`setup-openshell`](../actions/setup-openshell/action.yml) to install the pinned OpenShell CLI and wait for the local CI gateway. The [`scripts/pr-review-local.sh`](../../scripts/pr-review-local.sh) wrapper creates the temporary workspace/providers and configures inference. It calls @@ -25,16 +25,18 @@ afterward. The review script stages the diff in `prepare`, checks eligibility, renders the PR-specific policy, invokes the CLI, and validates output. The CLI composes the task and manages its sandbox lifecycle. -The CLI already supports a direct managed-gateway connection. Moving this -review job to the intended managed StackRox deployment still requires platform -ownership of workspace membership, provider credentials and their refresh or -expiry, matching inference routes, and CI network access. A pre-provisioned -provider name does not by itself keep a short-lived GitHub token usable. -See [managed reviewer requirements](../../docs/ci.md#managed-reviewer-transition). - -Once that contract is established, replace the job's local setup and temporary -provider bootstrap with managed authentication and `pr-review.sh run`. Preserve the task's allowed -operations and equivalent OpenShell policy and provider boundaries. +Set the caller repository's `OPENSHELL_GATEWAY_ENDPOINT` and complete the +[managed connection configuration](../../docs/ci.md#managed-reviewer-transition) +to run the same review against HyperShell. This path calls `pr-review.sh run` +directly with OIDC connection metadata and a gateway service-account secret. +It skips local OpenShell installation, Google authentication, and temporary +provider setup. Partial managed configuration fails before the task runs. + +The platform supplies workspace membership, the `github-review` provider and +credential refresh, and the Gemini 2.5 Pro `inference.local` route. The host's +GitHub App token still serves PR metadata checks; it does not update the managed +provider. `OPENSHELL_RUNNER` selects a Linux runner with access to the gateway +and issuer. The default remains `ubuntu-latest`. Comments may be posted during agent execution. Artifacts retain diagnostics; cleanup or cancellation does not undo GitHub operations that already succeeded. diff --git a/.github/workflows/pr-review-reusable.yml b/.github/workflows/pr-review-reusable.yml index 0d3688e..a9d15b3 100644 --- a/.github/workflows/pr-review-reusable.yml +++ b/.github/workflows/pr-review-reusable.yml @@ -23,9 +23,11 @@ on: type: string secrets: VERTEX_AI_SERVICE_ACCOUNT_KEY: - required: true + required: false # Local gateway only. OPENSHELL_GITHUB_APP_PRIVATE_KEY: required: true + OPENSHELL_OIDC_CLIENT_SECRET: + required: false # Managed gateway only. permissions: contents: read @@ -36,16 +38,37 @@ jobs: (!github.event.pull_request.draft || inputs.allow-draft-reviews) && (((github.event.action == 'labeled' || github.event.action == 'unlabeled') && github.event.label.name == 'ai-review') || (github.event.action != 'labeled' && github.event.action != 'unlabeled' && contains(github.event.pull_request.labels.*.name, 'ai-review'))) - runs-on: ubuntu-latest + # Use a Linux runner with access to the managed gateway AND its OIDC issuer. + runs-on: ${{ vars.OPENSHELL_RUNNER || 'ubuntu-latest' }} timeout-minutes: 15 concurrency: group: ai-review-${{ github.repository }}-${{ github.event.pull_request.number }} cancel-in-progress: true + env: + # An endpoint selects the existing direct SDK connection; no CLI registration. + OPENSHELL_GATEWAY: '' + OPENSHELL_GATEWAY_ENDPOINT: ${{ vars.OPENSHELL_GATEWAY_ENDPOINT }} + OPENSHELL_WORKSPACE: ${{ vars.OPENSHELL_WORKSPACE }} + OPENSHELL_OIDC_ISSUER: ${{ vars.OPENSHELL_OIDC_ISSUER }} + OPENSHELL_OIDC_CLIENT_ID: ${{ vars.OPENSHELL_OIDC_CLIENT_ID }} + OPENSHELL_OIDC_AUDIENCE: ${{ vars.OPENSHELL_OIDC_AUDIENCE }} steps: - name: Validate immutable Harness revision env: HARNESS_REF: ${{ inputs.harness-ref }} run: '[[ "$HARNESS_REF" =~ ^[0-9a-f]{40}$ ]]' + - name: Validate gateway configuration + env: + OPENSHELL_OIDC_CLIENT_SECRET: ${{ secrets.OPENSHELL_OIDC_CLIENT_SECRET }} + VERTEX_AI_SERVICE_ACCOUNT_KEY: ${{ secrets.VERTEX_AI_SERVICE_ACCOUNT_KEY }} + run: | + if [[ -n "$OPENSHELL_GATEWAY_ENDPOINT$OPENSHELL_WORKSPACE$OPENSHELL_OIDC_ISSUER$OPENSHELL_OIDC_CLIENT_ID$OPENSHELL_OIDC_AUDIENCE$OPENSHELL_OIDC_CLIENT_SECRET" ]]; then + for key in OPENSHELL_GATEWAY_ENDPOINT OPENSHELL_WORKSPACE OPENSHELL_OIDC_ISSUER OPENSHELL_OIDC_CLIENT_ID OPENSHELL_OIDC_AUDIENCE OPENSHELL_OIDC_CLIENT_SECRET; do + [[ -n "${!key}" ]] || { echo "::error::Managed review requires $key"; exit 1; } + done + else + [[ -n "$VERTEX_AI_SERVICE_ACCOUNT_KEY" ]] || { echo '::error::Local review requires VERTEX_AI_SERVICE_ACCOUNT_KEY'; exit 1; } + fi # Checkout only default-branch content from the caller. The PR head is data. - uses: actions/checkout@v7 with: @@ -96,19 +119,19 @@ jobs: REVIEW_HEAD: ${{ github.event.pull_request.head.sha }} run: bash scripts/pr-review.sh prepare - uses: ./harness/.github/actions/setup-openshell - if: steps.prepare.outputs.eligible == 'true' + if: steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT == '' env: OPENSHELL_ROOT: harness - name: Authenticate to Google Cloud - if: steps.prepare.outputs.eligible == 'true' + if: steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT == '' uses: google-github-actions/auth@7c6bc770dae815cd3e89ee6cdf493a5fab2cc093 # v3.0.0 with: project_id: ${{ vars.VERTEX_AI_PROJECT_ID }} credentials_json: ${{ secrets.VERTEX_AI_SERVICE_ACCOUNT_KEY }} - uses: google-github-actions/setup-gcloud@aa5489c8933f4cc7a4f7d45035b3b1440c9c10db # v3.0.1 - if: steps.prepare.outputs.eligible == 'true' + if: steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT == '' - name: Review in isolated OpenShell workspace - if: steps.prepare.outputs.eligible == 'true' + if: steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT == '' working-directory: harness env: GH_TOKEN: ${{ steps.openshell-app-token.outputs.token }} @@ -125,6 +148,18 @@ jobs: echo "::add-mask::$token" export GOOGLE_VERTEX_AI_TOKEN="$token" bash scripts/pr-review-local.sh + - name: Review on managed OpenShell + if: steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT != '' + working-directory: harness + env: + OPENSHELL_OIDC_CLIENT_SECRET: ${{ secrets.OPENSHELL_OIDC_CLIENT_SECRET }} + GH_TOKEN: ${{ steps.openshell-app-token.outputs.token }} + ALLOW_DRAFT_REVIEWS: ${{ inputs.allow-draft-reviews }} + REVIEW_REPOSITORY: ${{ github.repository }} + REVIEW_PR: ${{ github.event.pull_request.number }} + REVIEW_HEAD: ${{ github.event.pull_request.head.sha }} + REVIEW_SKILL: ${{ github.workspace }}/harness/skills/pr-review/SKILL.md + run: bash scripts/pr-review.sh run - uses: actions/upload-artifact@v7 if: always() && steps.prepare.outputs.eligible == 'true' with: diff --git a/README.md b/README.md index 36b731b..fd1e1b0 100644 --- a/README.md +++ b/README.md @@ -12,13 +12,13 @@ inside the sandbox. Trusted setup supplies a GitHub App token scoped to the target repository and required permissions. OpenShell holds the provider credential and mediates GitHub REST requests using a task-specific policy. -The intended StackRox deployment connects repository workflows to a -platform-managed gateway. The CLI already supports local and direct managed -connections; the current reusable reviewer uses -[`setup-openshell`](.github/actions/setup-openshell/action.yml) and a -[local setup wrapper](scripts/pr-review-local.sh) to prepare a local CI gateway -and temporary workspace. [`pr-review.sh`](scripts/pr-review.sh) prepares and -runs the review against that target or an already-configured target. +Repository workflows can use a HyperShell-managed gateway through the existing +OpenShell SDK connection. The reusable reviewer selects that path when the +caller configures `OPENSHELL_GATEWAY_ENDPOINT`; platform setup supplies workspace +access, providers, and the v0.0.109 `inference.local` route. Without managed +connection settings, [`setup-openshell`](.github/actions/setup-openshell/action.yml) +and the [local wrapper](scripts/pr-review-local.sh) prepare temporary CI resources. +[`pr-review.sh`](scripts/pr-review.sh) prepares and runs the same review task. ## What an agent can do @@ -122,23 +122,17 @@ data. The `ai-review` label is explicit opt-in. See | [`scripts/pr-review.sh`](scripts/pr-review.sh) | Stage the diff, check PR eligibility, render the PR policy, invoke the CLI, and validate the output | | [`harness` CLI](runner/) | Compose the task and manage its sandbox lifecycle | -The current reviewer uses a local gateway on the CI runner. The CLI's direct -managed-gateway connection is implemented, but the reusable reviewer has not -been switched to that connection and platform bootstrap contract. - -In the intended managed deployment, the GitHub job authenticates to a gateway -operated outside that job. The platform owns workspace membership, provider -lifecycle, and matching inference routes. The task retains its behavior and -allowed operations when the managed environment supplies equivalent providers, -policy support, and inference configuration. - -A pre-provisioned provider name still needs usable credentials. The managed -integration must establish who mints or refreshes short-lived GitHub App tokens, -how their repository and permission scope is selected, and how credentials -expire or are replaced. It also needs an agreed workspace boundary and CI -network access. The documented HyperShell environment, for example, currently -requires access to its VPN-only OIDC issuer. See -[the managed transition requirements](docs/ci.md#managed-reviewer-transition). +The managed path authenticates to an existing gateway with a service account +and creates only the task sandbox. The platform owns workspace membership, +provider credentials and refresh, and the matching inference route. The local +path uses the setup action and wrapper above. + +Configure the caller's connection variables and secret using the +[managed reviewer instructions](docs/ci.md#managed-reviewer-transition). +The selected Linux runner must reach both the gateway and its OIDC issuer; +use a runner on the Red Hat network when the issuer is private. The integration +target is HyperShell's OpenShell v0.0.109 deployment. Local CLI and SDK pins +remain unchanged; the exact SDK/server combination still requires live validation. ## Run a task locally diff --git a/docs/ci.md b/docs/ci.md index a955e09..dcf77bd 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -7,8 +7,9 @@ main/tag pushes publish images and update the shared registry cache. ## HyperShell validation HyperShell is the managed OpenShell environment used by these validation -examples. Validation runs locally from the Red Hat network because its OIDC -issuer is VPN-only. The `harness` CLI connects directly through the +examples. The existing local smoke tests use the Red Hat network because that +issuer is private. Managed review CI can use a Linux runner with the same network +access; a public gateway alone does not establish issuer reachability. The `harness` CLI connects directly through the OpenShell Go SDK. It does not persist a gateway registration or use a gateway administrator account at runtime. @@ -74,10 +75,12 @@ endpoints. The Actions job's `contents: read` permission is a separate token boundary from these explicitly requested GitHub App permissions. The workflow accepts the App Client ID as -`openshell-github-app-client-id`; callers explicitly forward only -`VERTEX_AI_SERVICE_ACCOUNT_KEY` and `OPENSHELL_GITHUB_APP_PRIVATE_KEY`. The -token is used on the trusted host for `gh` and native provider bootstrap, then -passed to OpenShell as the provider credential. It is never included in +`openshell-github-app-client-id`; callers explicitly forward +`OPENSHELL_GITHUB_APP_PRIVATE_KEY` and the credential for their gateway path: +`VERTEX_AI_SERVICE_ACCOUNT_KEY` for local setup, or +`OPENSHELL_OIDC_CLIENT_SECRET` for managed execution. The App token is used on +the trusted host for `gh`. Local setup also registers it as the provider +credential; the managed platform supplies its own scoped provider credential. It is never included in sandbox environment variables, payloads, agent arguments, or artifacts. Installation tokens expire after one hour and are revoked by the token action after the job. @@ -92,7 +95,8 @@ default branch. Once `AI review` is on the default branch, add `ai-review` to an open, non-draft PR. It reviews the full diff on labeling and each pushed head; newer runs cancel older ones. Removing the label, closing, or drafting the PR disables review. -It uses the Vertex secret/variables above. Summaries show status, head SHA, and +Local runs use the Vertex secret/variables above; managed runs use the connection +configuration below. Summaries show status, head SHA, and an artifact link. Seven-day artifacts hold input revisions, diff/hash, execution metadata, raw output/diagnostics, and `review.txt`. Reviews are advisory inline comments only; they do not approve, request changes, or merge. @@ -156,50 +160,115 @@ format and a separate publication stage remain deferred. ## Current reviewer setup -The [reusable workflow](../.github/workflows/pr-review-reusable.yml) invokes -[`setup-openshell`](../.github/actions/setup-openshell/action.yml), which installs -the pinned OpenShell CLI and waits for gateway readiness. This CI path uses a -local gateway. [`scripts/pr-review-local.sh`](../scripts/pr-review-local.sh) -creates a temporary workspace, registers `github-review` and `vertex-review`, -and configures inference. It calls [`pr-review.sh run`](../scripts/pr-review.sh), -then removes the providers, any profile it imported, and the workspace. +The [reusable workflow](../.github/workflows/pr-review-reusable.yml) uses the +caller's repository or organization variables to select the connection. With no +managed connection settings, it invokes [`setup-openshell`](../.github/actions/setup-openshell/action.yml) +and [`pr-review-local.sh`](../scripts/pr-review-local.sh) for temporary local +workspace, providers, and inference setup. With a complete managed connection, +it runs [`pr-review.sh run`](../scripts/pr-review.sh) directly. -`pr-review.sh` handles PR checks, diff preparation, policy rendering, and output -validation. It invokes the existing `harness workflow apply` command with a -unique sandbox name. The CLI owns sandbox execution and deletion, including -normal cancellation; the local wrapper waits for it before tearing down setup. +`pr-review.sh` handles PR checks, policy rendering, and output validation. The +existing CLI owns sandbox execution and deletion, including normal cancellation. +Generated sandbox and temporary workspace names fit v0.0.109's 19-character limit. ## Managed reviewer transition -Direct managed-gateway connectivity is already implemented in the CLI. The -reusable reviewer still uses the local setup above; moving it requires an -agreed managed integration contract: - -- **Runtime access:** a CI identity with the required workspace membership, - plus network access to the gateway and OIDC issuer. The HyperShell issuer - described here is reachable only from the Red Hat network/VPN. -- **Workspace isolation:** an explicit choice of shared or dedicated workspace - and the task's provider names and allowed operations. -- **Provider credential lifecycle:** platform ownership of GitHub App token - minting or refresh, repository and permission selection, and credential - replacement or expiry. Pre-provisioning a provider name does not keep an - expired installation token usable. -- **Inference and policy:** a matching provider/model route and equivalent - policy enforcement, so ordinary task runs can use existing references. - -Once these requirements are met, replace `setup-openshell`, Google bootstrap, -and `pr-review-local.sh` in the job with managed authentication and -`bash scripts/pr-review.sh run`. Keep host GitHub authentication for preparation -and PR checks. The task inputs, review command, and allowed GitHub operations -stay the same. The reusable workflow currently exposes only the local CI path; -the review command provides the execution step for a future managed caller. - -The POC deliberately fixes the task document, `github-review` provider name, -and Gemini 2.5 Pro model. Configure a managed target through the existing -workflow format and provide those resources before invoking it. - -The following bootstrap examples describe the managed validation environment; -they are not evidence that the reusable reviewer has completed this transition. +This path targets an existing HyperShell OpenShell **v0.0.109** gateway. Keep +`inference.local` for now. Configure these **repository or organization Actions +variables in the caller**, not environment-scoped variables: + +| Variable | Value | +|---|---| +| `OPENSHELL_GATEWAY_ENDPOINT` | HTTPS gateway URL; selects managed execution | +| `OPENSHELL_WORKSPACE` | Workspace the service-account subject can access | +| `OPENSHELL_OIDC_ISSUER` | HTTPS issuer from the gateway connection metadata | +| `OPENSHELL_OIDC_CLIENT_ID` | Gateway service-account client ID | +| `OPENSHELL_OIDC_AUDIENCE` | That gateway's audience | +| `OPENSHELL_RUNNER` | Optional Linux runner label, such as a dedicated `hypershell-ci` label; default `ubuntu-latest` | + +Store `OPENSHELL_OIDC_CLIENT_SECRET` as an Actions secret and forward it explicitly +from the caller. Any partial managed connection fails validation; it does not +fall back to creating a local gateway. The workflow clears `OPENSHELL_GATEWAY` +so a runner's named CLI registration cannot override the direct connection. +The secret is supplied only to configuration validation and managed execution. + +The runner needs Bash, `gh`, `jq`, GNU `timeout`, OpenSSL, and access to both the +gateway and issuer. The workflow installs Go and builds the trusted harness. +The managed path does not install the OpenShell CLI or authenticate to Google. +Use a runner on the Red Hat network for a private issuer. A runner label selects +an existing runner; it does not provision network access. + +Before enabling review, the platform owner must: + +1. Grant the gateway service-account subject `user` access to the selected + workspace. Use a repository-specific workspace for this POC's fixed provider + names and repository-scoped credentials. +2. Import the task's endpointless `github-review` profile and create the provider + instance named `github-review`, using a token scoped to the target repository + with `Contents: read` and `Pull requests: read/write`. Own its refresh or + replacement. The host's newly minted App token is only used for metadata + checks on this path; it is not uploaded to the managed provider. +3. Configure and verify the workspace's `inference.local` route for Gemini 2.5 + Pro and keep its Vertex credentials usable. The Haiku smoke example below + exercises a different model and is not proof that this review route works. +4. Verify the effective REST method/path restrictions and credential binding on + the deployed gateway. HyperShell's reviewed v109 gateway configuration + disables process-binary-aware network policy, so do not assume the task's + binary restrictions are enforced there. + +Native profile import already accepts a directory in v109: +`openshell provider profile import --from tasks/github-pr-reviewer/openshell/providers`. +Run it under platform bootstrap authority with the intended gateway/workspace. +It imports definitions only and is create-only; it does not refresh credentials +or provision provider instances. Ordinary review jobs do not call it. + +### Activate a consuming repository + +After publishing this change, update both the reusable workflow `uses` reference +and `harness-ref` to the same full commit SHA containing this integration. Then +forward the gateway secret in that caller's existing `secrets` block: + +```yaml +secrets: + OPENSHELL_GITHUB_APP_PRIVATE_KEY: ${{ secrets.OPENSHELL_GITHUB_APP_PRIVATE_KEY }} + OPENSHELL_OIDC_CLIENT_SECRET: ${{ secrets.OPENSHELL_OIDC_CLIENT_SECRET }} +``` + +Keep the existing GitHub App client-ID input. A managed-only caller can omit the +Vertex secret. The repository's checked-in `ai-review.yml` still pins the older +workflow; this change deliberately does not invent a future commit SHA or +activate a deployment. Do not forward the new secret while still calling the +old workflow, which does not declare it. + +### Verification and later changes + +On the chosen runner, first verify service-account access with the existing +SDK lifecycle smoke, then run the review against a designated test PR. Verify +an allowed inline comment, denial outside the allowed PR/methods, and sandbox +cleanup on success, failure, and cancellation. Confirm authentication still +works after access-token expiry. Static checks and fake commands do not prove +any of those live properties. The existing `test/hypershell-lifecycle.sh` is a +local/VPN helper and skips in CI; it cannot be the CI acceptance check. The +configured managed review path above executes in CI without that helper. + +The local CLI/SDK remain at their existing v0.0.110 pins. Qualify the SDK against +the actual vendor v109 gateway image; no server upgrade is required by this +patch. Upstream [#2907](https://github.com/NVIDIA/OpenShell/pull/2907) provides a +future replacement for custom OIDC token plumbing. Upstream +[#3195](https://github.com/NVIDIA/OpenShell/pull/3195) removes `inference.local`; +migrate setup and agent/provider configuration when HyperShell adopts it. +Neither change needs a new task runner interface now. HyperShell +[#267](https://github.com/openshift-online/hypershell/pull/267) provisions and tests +its own platform; the reviewer does not depend on that pipeline or its cluster +administration credentials. + +Review remains advisory. A later approval task can submit an explicit review for +the reviewed commit with separately granted permissions. Issue-to-PR can use its +own task policy. Merge decisions stay in repository workflows; a successful +runner result is not an approval. + +The following bootstrap examples describe the separate managed smoke-test +environment, not the reviewer workspace or evidence of a completed CI run. ## One-time platform bootstrap diff --git a/scripts/pr-review-local.sh b/scripts/pr-review-local.sh index ef87a2e..969eadc 100755 --- a/scripts/pr-review-local.sh +++ b/scripts/pr-review-local.sh @@ -6,7 +6,8 @@ cd "$(dirname "$0")/.." : "${GOOGLE_VERTEX_AI_TOKEN:?set a short-lived Vertex token}" "${VERTEX_AI_PROJECT_ID:?set Vertex project}" : "${GITHUB_TOKEN:?set the repository-scoped GitHub App token for bootstrap}" gateway="${OPENSHELL_GATEWAY:-openshell}" -workspace="review-$(openssl rand -hex 12)" +# v0.0.109 caps workspace names at 19 characters. +workspace="review-$(openssl rand -hex 6)" created_workspace=false created_vertex=false created_github=false diff --git a/scripts/pr-review.sh b/scripts/pr-review.sh index c77b4a3..0e07db8 100644 --- a/scripts/pr-review.sh +++ b/scripts/pr-review.sh @@ -98,7 +98,8 @@ run_review() { -e "s|\${REVIEW_PR}|$REVIEW_PR|g" \ "$policy_template" > "$REVIEW_POLICY" - sandbox_name="review-$(openssl rand -hex 12)" + # v0.0.109 caps sandbox names at 19 characters. + sandbox_name="review-$(openssl rand -hex 6)" ( ulimit -f 2048 # Bound raw diagnostic output as well as runtime. exec timeout -s TERM -k 35s 8m ./harness workflow apply tasks/github-pr-reviewer/workflow/opencode-harness.yaml \ diff --git a/tasks/github-pr-reviewer/README.md b/tasks/github-pr-reviewer/README.md index 158faaa..8dd62b4 100644 --- a/tasks/github-pr-reviewer/README.md +++ b/tasks/github-pr-reviewer/README.md @@ -17,8 +17,8 @@ diff as untrusted data. - The provider instance must exist when the sandbox starts. The current [`scripts/pr-review-local.sh`](../../scripts/pr-review-local.sh) wrapper creates it in a temporary workspace from a repository-scoped GitHub App token. A managed - integration must supply the instance and its credential lifecycle through - trusted setup. The profile contains metadata only, never a credential. + integration supplies the instance and its credential lifecycle through + platform setup (see [managed CI](../../docs/ci.md#managed-reviewer-transition)). The profile contains metadata only, never a credential. - Setup must also configure `inference.local` for the task's Gemini 2.5 Pro model. The task consumes that route without reconciling it. diff --git a/test/pr_review_test.go b/test/pr_review_test.go index 19a8c8f..655575c 100644 --- a/test/pr_review_test.go +++ b/test/pr_review_test.go @@ -19,7 +19,7 @@ func TestPRReview(t *testing.T) { if err != nil { t.Fatal(err) } - for _, scenario := range []string{"success", "unlabeled", "stale", "oversized", "tampered", "agent-failure", "provider-failure", "cleanup-failure", "local-success", "local-cancel", "partial-provider-failure", "workspace-failure", "profile-read-failure", "existing-profile", "status-failure", "cancel", "truncated", "malformed-trailing", "incomplete", "empty", "error", "tool_use", "tool_exit", "tool_missing_exit", "read-tool", "tool_recovered", "unrelated-422", "unrelated-422-line", "unrelated-422-comment", "unrelated-comment", "success-then-failure", "comment-position"} { + for _, scenario := range []string{"success", "direct-success", "unlabeled", "stale", "oversized", "tampered", "agent-failure", "provider-failure", "cleanup-failure", "local-success", "local-cancel", "partial-provider-failure", "workspace-failure", "profile-read-failure", "existing-profile", "status-failure", "cancel", "truncated", "malformed-trailing", "incomplete", "empty", "error", "tool_use", "tool_exit", "tool_missing_exit", "read-tool", "tool_recovered", "unrelated-422", "unrelated-422-line", "unrelated-422-comment", "unrelated-comment", "success-then-failure", "comment-position"} { t.Run(scenario, func(t *testing.T) { local := scenario == "provider-failure" || scenario == "cleanup-failure" || scenario == "local-success" || scenario == "local-cancel" || scenario == "partial-provider-failure" || scenario == "workspace-failure" || scenario == "profile-read-failure" || scenario == "existing-profile" root := t.TempDir() @@ -92,6 +92,9 @@ func TestPRReview(t *testing.T) { cmd = exec.CommandContext(ctx, "bash", filepath.Join(root, "scripts/pr-review-local.sh")) } cmd.Env = prepare.Env + if scenario == "direct-success" { + cmd.Env = append(cmd.Env, "OPENSHELL_GATEWAY=", "OPENSHELL_GATEWAY_ENDPOINT=https://gateway.example.test", "OPENSHELL_OIDC_ISSUER=https://issuer.example.test", "OPENSHELL_OIDC_CLIENT_ID=review-ci", "OPENSHELL_OIDC_AUDIENCE=review-gateway", "OPENSHELL_OIDC_CLIENT_SECRET=fake-oidc-secret") + } if local { cmd.Env = append(cmd.Env, "GOOGLE_VERTEX_AI_TOKEN=fake", "VERTEX_AI_PROJECT_ID=test-project", "GITHUB_TOKEN=fake") } @@ -115,7 +118,7 @@ func TestPRReview(t *testing.T) { } } err = cmd.Wait() - if (err == nil) != (scenario == "success" || scenario == "stale" || scenario == "local-success" || scenario == "existing-profile" || scenario == "comment-position" || scenario == "read-tool" || scenario == "tool_recovered") { + if (err == nil) != (scenario == "success" || scenario == "direct-success" || scenario == "stale" || scenario == "local-success" || scenario == "existing-profile" || scenario == "comment-position" || scenario == "read-tool" || scenario == "tool_recovered") { t.Fatalf("unexpected result: %v\n%s", err, logs.String()) } trace, _ := os.ReadFile(filepath.Join(root, "trace")) @@ -134,7 +137,11 @@ func TestPRReview(t *testing.T) { t.Fatalf("existing-target review performed setup or forced a target: %s", trace) } } - if !strings.Contains(string(trace), "target managed-test shared-test") { + target := "target managed-test shared-test" + if scenario == "direct-success" { + target = "target shared-test" + } + if !strings.Contains(string(trace), target) { t.Fatal("configured target environment was not preserved") } } else { @@ -165,13 +172,91 @@ func TestPRReview(t *testing.T) { } } summary, _ := os.ReadFile(filepath.Join(root, "review/summary.md")) - if strings.Contains(string(summary), "AI review: completed") != (scenario == "success" || scenario == "local-success" || scenario == "existing-profile" || scenario == "cleanup-failure" || scenario == "comment-position" || scenario == "read-tool" || scenario == "tool_recovered") || strings.Contains(string(summary), "MODEL_OUTPUT") { + if strings.Contains(string(summary), "AI review: completed") != (scenario == "success" || scenario == "direct-success" || scenario == "local-success" || scenario == "existing-profile" || scenario == "cleanup-failure" || scenario == "comment-position" || scenario == "read-tool" || scenario == "tool_recovered") || strings.Contains(string(summary), "MODEL_OUTPUT") { t.Fatalf("incorrect or model-controlled summary: %s", summary) } }) } } +func TestPRReviewGatewayConfiguration(t *testing.T) { + workflow := parseWorkflow(t, string(mustRead(t, "../.github/workflows/pr-review-reusable.yml"))) + job := workflow.Jobs["review"] + var validate, managed workflowStep + localSteps := 0 + for _, step := range job.Steps { + if step.Name == "Validate gateway configuration" { + validate = step + } + if step.Run == "bash scripts/pr-review.sh run" { + managed = step + } + if step.Uses == "./harness/.github/actions/setup-openshell" || strings.HasPrefix(step.Uses, "google-github-actions/") || strings.Contains(step.Run, "bash scripts/pr-review-local.sh") { + localSteps++ + if step.If != "steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT == ''" { + t.Fatalf("managed review could execute local setup: %+v", step) + } + } + } + if localSteps != 4 || validate.Run == "" || managed.If != "steps.prepare.outputs.eligible == 'true' && vars.OPENSHELL_GATEWAY_ENDPOINT != ''" { + t.Fatal("missing gateway validation or mutually exclusive review paths") + } + if job.RunsOn != "${{ vars.OPENSHELL_RUNNER || 'ubuntu-latest' }}" || job.Env["OPENSHELL_GATEWAY"] != "" { + t.Fatal("managed job must select its runner and use the direct gateway connection") + } + if managed.Env["OPENSHELL_OIDC_CLIENT_SECRET"] != "${{ secrets.OPENSHELL_OIDC_CLIENT_SECRET }}" || managed.Env["GH_TOKEN"] != "${{ steps.openshell-app-token.outputs.token }}" { + t.Fatal("managed execution needs gateway authentication and host GitHub authentication") + } + trigger := workflow.On["workflow_call"] + for _, key := range []string{"VERTEX_AI_SERVICE_ACCOUNT_KEY", "OPENSHELL_OIDC_CLIENT_SECRET"} { + secret, ok := trigger.Secrets[key] + if !ok || secret.Required { + t.Fatalf("%s must be validated only for its gateway path", key) + } + } + values := map[string]string{ + "OPENSHELL_GATEWAY_ENDPOINT": "https://gateway.example.test", + "OPENSHELL_WORKSPACE": "review-ci", + "OPENSHELL_OIDC_ISSUER": "https://issuer.example.test", + "OPENSHELL_OIDC_CLIENT_ID": "review-ci", + "OPENSHELL_OIDC_AUDIENCE": "review-gateway", + "OPENSHELL_OIDC_CLIENT_SECRET": "fake-oidc-secret", + } + for key := range values { + if key != "OPENSHELL_OIDC_CLIENT_SECRET" && job.Env[key] != "${{ vars."+key+" }}" { + t.Fatalf("managed connection does not forward %s", key) + } + } + cases := []string{"local", "local-missing-secret", "managed", "secret-only"} + for key := range values { + cases = append(cases, key) + } + for _, scenario := range cases { + t.Run(scenario, func(t *testing.T) { + cmd := exec.CommandContext(t.Context(), "bash", "-eu", "-c", validate.Run) + cmd.Env = os.Environ() + for key, value := range values { + if strings.HasPrefix(scenario, "local") || scenario == key || (scenario == "secret-only" && key != "OPENSHELL_OIDC_CLIENT_SECRET") { + value = "" + } + cmd.Env = append(cmd.Env, key+"="+value) + } + vertex := "" + if scenario == "local" { + vertex = "fake-vertex-secret" + } + cmd.Env = append(cmd.Env, "VERTEX_AI_SERVICE_ACCOUNT_KEY="+vertex) + out, err := cmd.CombinedOutput() + if (err == nil) != (scenario == "local" || scenario == "managed") { + t.Fatalf("unexpected configuration result: %v\n%s", err, out) + } + if strings.Contains(string(out), "fake-oidc-secret") || strings.Contains(string(out), "fake-vertex-secret") { + t.Fatal("configuration validation disclosed a credential") + } + }) + } +} + func TestGitHubAppTokenIsHostOnly(t *testing.T) { script, err := os.ReadFile("../scripts/pr-review-local.sh") if err != nil { @@ -290,6 +375,8 @@ type workflowSecret struct { } type workflowJob struct { + RunsOn string `yaml:"runs-on"` + Env map[string]string `yaml:"env"` Uses string `yaml:"uses"` With map[string]string `yaml:"with"` Secrets map[string]string `yaml:"secrets"` @@ -297,6 +384,9 @@ type workflowJob struct { } type workflowStep struct { + Name string `yaml:"name"` + If string `yaml:"if"` + Run string `yaml:"run"` Uses string `yaml:"uses"` With map[string]string `yaml:"with"` Env map[string]string `yaml:"env"` @@ -346,6 +436,16 @@ if [[ "${0##*/}" == gh ]]; then fi exit 0 fi +# Match the v0.0.109 gateway's routable-name constraint, not the generator. +if [[ "$1 ${2:-}" == 'workspace create' || "$1 ${2:-}" == 'workflow apply' ]]; then + args=("$@") + for ((i=0; i<${#args[@]}; i++)); do + if [[ "${args[i]}" == --name ]]; then + name="${args[i+1]}" + [[ -n "$name" && ${#name} -le 19 ]] || exit 1 + fi + done +fi case "$1 ${2:-}" in 'workspace create') [[ "$FAKE_SCENARIO" != workspace-failure ]] ;; 'provider list-profiles') @@ -356,6 +456,11 @@ case "$1 ${2:-}" in if [[ "$FAKE_SCENARIO" == partial-provider-failure && "$*" == *'--name github-review '* ]]; then exit 1; fi ;; 'workspace delete') [[ "$FAKE_SCENARIO" != cleanup-failure ]] ;; 'workflow apply') + if [[ "$FAKE_SCENARIO" == direct-success ]]; then + [[ -z "${OPENSHELL_GATEWAY:-}" && "$OPENSHELL_GATEWAY_ENDPOINT" == https://gateway.example.test && + "$OPENSHELL_OIDC_ISSUER" == https://issuer.example.test && "$OPENSHELL_OIDC_CLIENT_ID" == review-ci && + "$OPENSHELL_OIDC_AUDIENCE" == review-gateway && "$OPENSHELL_OIDC_CLIENT_SECRET" == fake-oidc-secret ]] || exit 1 + fi printf 'target %s %s\n' "${OPENSHELL_GATEWAY:-}" "${OPENSHELL_WORKSPACE:-}" >> "$TRACE" touch "$READY" printf 'diagnostic without trailing newline' >&2