diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 3d7cfd2..44ebcba 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -13,6 +13,10 @@ the sandboxed agent to read the selected PR and post inline comments to it. The token's repository permissions and the policy's PR-specific HTTP methods and paths are separate restrictions. Consumers should pin both the workflow reference and its `harness-ref` input to the same immutable commit SHA. +Codex callers also declare the pre-provisioned provider names through +`required-providers`; the run records the availability check as +`provider-check.json` and stops before sandbox creation when a provider is +missing. ## Gateway setup: local CI and managed deployment diff --git a/.github/workflows/pr-review-reusable.yml b/.github/workflows/pr-review-reusable.yml index f9f9b8f..763276f 100644 --- a/.github/workflows/pr-review-reusable.yml +++ b/.github/workflows/pr-review-reusable.yml @@ -31,7 +31,12 @@ on: description: Existing OpenShell provider used by the Codex Responses API route required: false type: string - default: openai-review + default: openai-inference + required-providers: + description: JSON array of existing OpenShell providers required by this task + required: false + type: string + default: '[]' codex-model: description: Model requested by the Codex reviewer required: false @@ -77,12 +82,16 @@ jobs: CODEX_PROVIDER: ${{ inputs.codex-inference-provider }} CODEX_MODEL: ${{ inputs.codex-model }} CODEX_WORKSPACE: ${{ inputs.codex-workspace }} + REQUIRED_PROVIDERS: ${{ inputs.required-providers }} run: | [[ "$REVIEW_AGENT" == opencode || "$REVIEW_AGENT" == codex ]] + jq -e 'type == "array" and all(.[]; type == "string" and test("^[A-Za-z0-9][A-Za-z0-9_.-]{0,62}$"))' <<< "$REQUIRED_PROVIDERS" >/dev/null if [[ "$REVIEW_AGENT" == codex ]]; then - [[ "$CODEX_PROVIDER" =~ ^[A-Za-z0-9_.-]+$ ]] + [[ "$CODEX_PROVIDER" =~ ^[A-Za-z0-9][A-Za-z0-9_.-]{0,62}$ ]] [[ "$CODEX_MODEL" =~ ^[A-Za-z0-9_.@/-]+$ ]] [[ "$CODEX_WORKSPACE" =~ ^[A-Za-z0-9][A-Za-z0-9_.-]{0,62}$ ]] + jq -e --arg provider "$CODEX_PROVIDER" 'index($provider) != null' <<< "$REQUIRED_PROVIDERS" >/dev/null + jq -e 'index("github-review") != null' <<< "$REQUIRED_PROVIDERS" >/dev/null fi # Checkout only default-branch content from the caller. The PR head is data. - uses: actions/checkout@v7 @@ -159,6 +168,7 @@ jobs: CODEX_INFERENCE_PROVIDER: ${{ inputs.codex-inference-provider }} CODEX_MODEL: ${{ inputs.codex-model }} CODEX_WORKSPACE: ${{ inputs.codex-workspace }} + TASK_PROVIDERS: ${{ inputs.required-providers }} GITHUB_TOKEN: ${{ steps.openshell-app-token.outputs.token }} REVIEW_REPOSITORY: ${{ github.repository }} REVIEW_PR: ${{ github.event.pull_request.number }} diff --git a/docs/ci.md b/docs/ci.md index 4aaa24c..8272bf3 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -143,7 +143,7 @@ The reusable workflow also supports `review-agent: codex` with the keeps the existing OpenCode/Vertex path available. Codex uses the gateway's `inference.local` Responses API route, so the caller must arrange a platform-owned OpenAI-compatible provider first (the default name is -`openai-review`) in a dedicated workspace. No OpenAI key is passed through the +`openai-inference`) in a dedicated workspace. No OpenAI key is passed through the workflow or sandbox. Trusted callers select it with: @@ -152,11 +152,15 @@ Trusted callers select it with: with: review-label: stackrox-ai-review review-agent: codex - codex-inference-provider: openai-review + codex-inference-provider: openai-inference codex-model: gpt-5.6-luna codex-workspace: codex-review + required-providers: '["github-review", "openai-inference"]' ``` +The simple harness verifies each declared provider before creating the sandbox +and fails the run if one is unavailable in the selected gateway workspace. + The Codex task fixes reasoning effort to `xhigh`. The outer OpenShell policy continues to control filesystem and GitHub egress, and the Codex path does not require the Vertex service-account secret. @@ -173,7 +177,9 @@ preparation instead. Select a registered gateway/workspace with connection (see [workflow contract](#workflow-contract)). The review command uses the selected target and only creates its task sandbox. It needs host `gh` authentication for PR checks, while the platform supplies the `github-review` -provider with usable credentials and the Gemini 2.5 Pro inference route. +provider with usable credentials. OpenCode requires the Gemini 2.5 Pro inference +route; Codex requires the configured `CODEX_INFERENCE_PROVIDER` (default: +`openai-inference`). Unit tests use fake commands, not Vertex. The agent can already publish inline comments directly through the allowed API endpoint. A structured findings @@ -193,6 +199,10 @@ then removes the providers, any profile it imported, and the workspace. 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. +The review wrapper delegates that execution through the shared +[`scripts/run-task.sh`](../scripts/run-task.sh) adapter, which is also suitable +for task bundles with additional provider attachments such as the read-only +ACS triage task. ## Managed reviewer transition diff --git a/scripts/pr-review.sh b/scripts/pr-review.sh index 103c334..cea5bf5 100755 --- a/scripts/pr-review.sh +++ b/scripts/pr-review.sh @@ -12,7 +12,7 @@ gateway="${OPENSHELL_GATEWAY:-openshell}" allow_draft_reviews="${ALLOW_DRAFT_REVIEWS:-false}" review_agent="${REVIEW_AGENT:-opencode}" review_label="${REVIEW_LABEL:-stackrox-ai-review}" -codex_inference_provider="${CODEX_INFERENCE_PROVIDER:-openai-review}" +codex_inference_provider="${CODEX_INFERENCE_PROVIDER:-openai-inference}" codex_model="${CODEX_MODEL:-gpt-5.6-luna}" codex_workspace="${CODEX_WORKSPACE:-}" case "$review_agent" in @@ -37,6 +37,8 @@ configured_target=false if [[ "$review_agent" == codex ]]; then workspace="$codex_workspace" sandbox_name="codex-$(openssl rand -hex 6)" + configured_target=true + github_provider=github-review else workspace="${OPENSHELL_WORKSPACE:-rev-$RANDOM-$$}" sandbox_name="review-$(openssl rand -hex 6)" @@ -199,9 +201,8 @@ run_review() { ( ulimit -f 2048 # Bound raw diagnostic output as well as runtime. - exec timeout -s TERM -k 35s 8m ./harness workflow apply "$workflow_file" \ - --gateway "$gateway" --workspace "$workspace" --output-dir "$REVIEW_DIR" \ - --result-file "$REVIEW_DIR/execution.json" + OPENSHELL_GATEWAY="$gateway" OPENSHELL_WORKSPACE="$workspace" \ + exec scripts/run-task.sh "$workflow_file" "$REVIEW_DIR" "$REVIEW_DIR/execution.json" ) > "$REVIEW_DIR/agent.ndjson" 2> "$REVIEW_DIR/agent.stderr" & apply_pid=$! set +e diff --git a/scripts/run-task.sh b/scripts/run-task.sh new file mode 100755 index 0000000..5bdb1b0 --- /dev/null +++ b/scripts/run-task.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Execute one trusted task bundle through the harness workflow runner. +set -euo pipefail +umask 077 +cd "$(dirname "$0")/.." + +workflow_file="${1:?usage: run-task.sh WORKFLOW OUTPUT_DIR RESULT_FILE}" +output_dir="${2:?usage: run-task.sh WORKFLOW OUTPUT_DIR RESULT_FILE}" +result_file="${3:?usage: run-task.sh WORKFLOW OUTPUT_DIR RESULT_FILE}" + +[[ "$output_dir" == /* && "$result_file" == /* ]] || { + echo "task output and result paths must be absolute" >&2 + exit 1 +} +[[ "$workflow_file" != /* && "$workflow_file" != *..* ]] || { + echo "task workflow must be a trusted repository-relative path" >&2 + exit 1 +} + +gateway="${OPENSHELL_GATEWAY:-openshell}" +workspace="${OPENSHELL_WORKSPACE:-}" +task_timeout="${TASK_TIMEOUT:-8m}" +kill_after="${TASK_KILL_AFTER:-35s}" +[[ "$task_timeout" =~ ^[0-9]+[smh]$ && "$kill_after" =~ ^[0-9]+[smh]$ ]] || { + echo "TASK_TIMEOUT and TASK_KILL_AFTER must be durations such as 8m or 35s" >&2 + exit 1 +} + +check_required_providers() { + local required="${TASK_PROVIDERS:-[]}" + mkdir -p "$output_dir" + if ! jq -e 'type == "array" and all(.[]; type == "string" and test("^[A-Za-z0-9][A-Za-z0-9_.-]{0,62}$"))' <<< "$required" >/dev/null; then + echo "TASK_PROVIDERS must be a JSON array of provider names" >&2 + exit 1 + fi + + local report="$output_dir/provider-check.json" + local provider + local provider_args=(--gateway "$gateway") + [[ -n "$workspace" ]] && provider_args+=(--workspace "$workspace") + if jq -e 'length == 0' <<< "$required" >/dev/null; then + jq -n --arg gateway "$gateway" --arg workspace "$workspace" \ + '{status:"skipped", gateway:$gateway, workspace:$workspace, providers:[]}' > "$report" + return + fi + while IFS= read -r provider; do + if ! timeout 60s openshell provider get "${provider_args[@]}" "$provider" >/dev/null 2>&1; then + jq -n --arg gateway "$gateway" --arg workspace "$workspace" --arg provider "$provider" \ + '{status:"failed", gateway:$gateway, workspace:$workspace, missing:[$provider]}' > "$report" + echo "required OpenShell provider is unavailable: $provider (gateway=$gateway workspace=${workspace:-default})" >&2 + exit 1 + fi + done < <(jq -r '.[]' <<< "$required") + + jq -n --arg gateway "$gateway" --arg workspace "$workspace" --argjson providers "$required" \ + '{status:"available", gateway:$gateway, workspace:$workspace, providers:$providers}' > "$report" +} + +check_required_providers + +args=(workflow apply "$workflow_file" --output-dir "$output_dir" --result-file "$result_file") +[[ -n "$gateway" ]] && args+=(--gateway "$gateway") +[[ -n "$workspace" ]] && args+=(--workspace "$workspace") + +exec timeout -s TERM -k "$kill_after" "$task_timeout" ./harness "${args[@]}" diff --git a/tasks/README.md b/tasks/README.md index 776e29e..49a31e1 100644 --- a/tasks/README.md +++ b/tasks/README.md @@ -17,6 +17,27 @@ gateway; provider instances and their credentials remain gateway-owned. The `workflow/` directory contains the task-specific agent behavior and the optional version 1 harness workflow document. +## Composable execution + +Task bundles are composable through the version 1 workflow document. The +workflow combines the agent, inference route, sandbox policy, provider +attachments, payloads, source checkout, and outputs. The shared +[`scripts/run-task.sh`](../scripts/run-task.sh) adapter executes one trusted +workflow and captures its result. + +Callers may set `TASK_PROVIDERS` to a JSON array of pre-provisioned provider +names. The adapter checks those names in the selected gateway workspace before +starting the workflow and writes the result to `provider-check.json`; it never +creates providers or handles their credentials. + +The initial Codex pull-request reviewer composes Codex inference with +read-only GitHub pull-request access and the narrowly scoped comment +operation. A future triage task can use the same adapter and add providers +such as read-only GCS and Jira in its own workflow without adding +triage-specific logic to the reviewer runner. The provider list and policy +remain the executable capability boundary; a separate capability allowlist is +intentionally deferred. + OpenShell has no single native task-bundle file abstraction. A task can run with native OpenShell by using the image, policy, provider, and agent command with `openshell sandbox create` and upload commands. The `harness` CLI composes diff --git a/tasks/github-pr-reviewer/README.md b/tasks/github-pr-reviewer/README.md index ceb9332..ccdb48a 100644 --- a/tasks/github-pr-reviewer/README.md +++ b/tasks/github-pr-reviewer/README.md @@ -1,11 +1,21 @@ # GitHub pull-request reviewer -This task bundle reads a pull-request diff and lets the sandboxed agent post -inline review comments through OpenShell's GitHub REST proxy. The instructions -request at most three concrete comments; the policy restricts endpoints, not -comment count or finding quality. It grants no push, label, approval, or merge -operations. The trusted caller must obtain current PR metadata and stage the -diff as untrusted data. +This task bundle reads a pull-request diff and lets the sandboxed Codex agent +post inline review comments through OpenShell's GitHub REST proxy. The +instructions request at most three concrete comments; the policy restricts +endpoints, not comment count or finding quality. It grants no push, label, +approval, or merge operations. The trusted caller must obtain current PR +metadata and stage the diff as untrusted data. + +The task's initial composition is deliberately small: + +- Codex inference through the gateway's `inference.local` route. +- Read-only GitHub pull-request access, plus inline comments on that exact PR. + +The shared [`scripts/run-task.sh`](../../scripts/run-task.sh) adapter executes +this bundle. Other tasks can reuse the adapter and add providers and policy +rules in their own bundle; the reviewer wrapper does not need to know about +those task-specific capabilities. ## Layout @@ -19,12 +29,14 @@ diff as untrusted data. 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. -- Setup must also configure `inference.local` for the task's Gemini 2.5 Pro - model. The task consumes that route without reconciling it. +- The OpenCode path must configure `inference.local` for the task's Gemini 2.5 + Pro model. The task consumes that route without reconciling it; the Codex + path uses its pre-provisioned OpenAI-compatible route instead. -[`scripts/pr-review.sh`](../../scripts/pr-review.sh) prepares and runs the -review against the configured target. The local wrapper supplies temporary -setup around its `run` command; managed callers supply platform setup. +[`scripts/pr-review.sh`](../../scripts/pr-review.sh) prepares the review and +delegates task execution to the shared adapter. The local wrapper supplies +temporary setup around its `run` command; managed callers supply platform +setup. The workflow is trusted host-side code. The diff and GitHub responses are untrusted input and must never be treated as instructions. The only permitted diff --git a/tasks/github-pr-reviewer/openshell/README.md b/tasks/github-pr-reviewer/openshell/README.md index 0335af7..769b6ea 100644 --- a/tasks/github-pr-reviewer/openshell/README.md +++ b/tasks/github-pr-reviewer/openshell/README.md @@ -12,7 +12,7 @@ The sandbox must attach an existing `github-review` provider instance. The endpointless profile in `providers/github-review.yaml` describes the credential shape but contains no credential value. The OpenCode workflow expects the gateway's `vertex-review` inference provider. The opt-in Codex workflow expects -an existing OpenAI-compatible provider, named `openai-review` by default. Both +an existing OpenAI-compatible provider, named `openai-inference` by default. Both use the platform-owned `inference.local` route and neither provider is created by this workload. diff --git a/test/pr_review_test.go b/test/pr_review_test.go index 85fa0dc..1075e99 100644 --- a/test/pr_review_test.go +++ b/test/pr_review_test.go @@ -49,7 +49,8 @@ func TestPRReview(t *testing.T) { t.Fatalf("validator must be executable: mode %o", validatorMode) } codexValidator := mustRead(t, "../scripts/review/validate-codex-output.sh") - for name, data := range map[string][]byte{"scripts/pr-review.sh": script, "scripts/pr-review-local.sh": mustRead(t, "../scripts/pr-review-local.sh"), "scripts/review/validate-agent-output.sh": validator, "scripts/review/validate-codex-output.sh": codexValidator, "harness": []byte(fakeReviewCommand), "openshell": []byte(fakeReviewCommand), "gh": []byte(fakeReviewCommand), "review-policy.yaml": []byte("version: 1\nnetwork_policies: {}\n"), "output": nil, "step-summary": nil} { + taskRunner := mustRead(t, "../scripts/run-task.sh") + for name, data := range map[string][]byte{"scripts/pr-review.sh": script, "scripts/pr-review-local.sh": mustRead(t, "../scripts/pr-review-local.sh"), "scripts/run-task.sh": taskRunner, "scripts/review/validate-agent-output.sh": validator, "scripts/review/validate-codex-output.sh": codexValidator, "harness": []byte(fakeReviewCommand), "openshell": []byte(fakeReviewCommand), "gh": []byte(fakeReviewCommand), "review-policy.yaml": []byte("version: 1\nnetwork_policies: {}\n"), "output": nil, "step-summary": nil} { mode := os.FileMode(0o700) if name == "scripts/review/validate-agent-output.sh" { mode = validatorMode @@ -146,8 +147,8 @@ func TestPRReview(t *testing.T) { if scenario != "codex-success" && !strings.Contains(string(trace), "target managed-test shared-test") { t.Fatal("configured target environment was not preserved") } - if scenario == "codex-success" && !strings.Contains(string(trace), "sandbox delete") { - t.Fatal("Codex review did not clean up its sandbox") + if scenario == "codex-success" && strings.Contains(string(trace), "sandbox delete") { + t.Fatal("Codex review wrapper attempted to clean up a sandbox it did not create") } } else { if strings.Contains(string(trace), "workspace delete") == (scenario == "workspace-failure") { @@ -184,6 +185,90 @@ func TestPRReview(t *testing.T) { } } +func TestRunTaskProviderPreflight(t *testing.T) { + root := t.TempDir() + if err := os.Mkdir(filepath.Join(root, "scripts"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "scripts", "run-task.sh"), mustRead(t, "../scripts/run-task.sh"), 0o700); err != nil { + t.Fatal(err) + } + const fakeOpenShell = `#!/usr/bin/env bash +set -eu +printf '%s\n' "$*" >> "$TRACE" +[[ "$*" != *missing-provider* ]] +` + const fakeHarness = `#!/usr/bin/env bash +set -eu +printf '%s\n' "$*" >> "$TRACE" +result_file="" +for ((i = 1; i <= $#; i++)); do + if [[ "${!i}" == --result-file ]]; then + next=$((i + 1)) + result_file="${!next}" + fi +done +printf '%s\n' '{"status":"succeeded","phase":"complete"}' > "$result_file" +` + for name, data := range map[string][]byte{"openshell": []byte(fakeOpenShell), "harness": []byte(fakeHarness)} { + if err := os.WriteFile(filepath.Join(root, name), data, 0o700); err != nil { + t.Fatal(err) + } + } + + run := func(outputDir, resultFile, trace, providers string) ([]byte, error) { + cmd := exec.Command("bash", filepath.Join(root, "scripts", "run-task.sh"), "tasks/test.yaml", outputDir, resultFile) + cmd.Env = append(os.Environ(), + "PATH="+root+string(os.PathListSeparator)+os.Getenv("PATH"), + "TRACE="+trace, + "OPENSHELL_GATEWAY=managed-test", + "OPENSHELL_WORKSPACE=codex-workspace", + "TASK_PROVIDERS="+providers, + ) + return cmd.CombinedOutput() + } + + successOutput := filepath.Join(root, "success") + successResult := filepath.Join(successOutput, "execution.json") + successTrace := filepath.Join(root, "success.trace") + if output, err := run(successOutput, successResult, successTrace, `["github-review","openai-inference"]`); err != nil { + t.Fatalf("provider preflight succeeded: %v\n%s", err, output) + } + report := string(mustRead(t, filepath.Join(successOutput, "provider-check.json"))) + if !strings.Contains(report, `"status": "available"`) || !strings.Contains(report, `"openai-inference"`) { + t.Fatalf("provider preflight report = %s", report) + } + if !strings.Contains(string(mustRead(t, successTrace)), "workflow apply") { + t.Fatal("provider preflight did not run the trusted task") + } + + emptyOutput := filepath.Join(root, "empty") + emptyResult := filepath.Join(emptyOutput, "execution.json") + emptyTrace := filepath.Join(root, "empty.trace") + if output, err := run(emptyOutput, emptyResult, emptyTrace, `[ ]`); err != nil { + t.Fatalf("empty provider declaration failed: %v\n%s", err, output) + } + report = string(mustRead(t, filepath.Join(emptyOutput, "provider-check.json"))) + if !strings.Contains(report, `"status": "skipped"`) { + t.Fatalf("empty provider report = %s", report) + } + + failureOutput := filepath.Join(root, "failure") + failureResult := filepath.Join(failureOutput, "execution.json") + failureTrace := filepath.Join(root, "failure.trace") + output, err := run(failureOutput, failureResult, failureTrace, `["missing-provider"]`) + if err == nil || !strings.Contains(string(output), "missing-provider") { + t.Fatalf("missing provider was not rejected: %v\n%s", err, output) + } + if strings.Contains(string(mustRead(t, failureTrace)), "workflow apply") { + t.Fatal("missing provider reached the trusted task") + } + report = string(mustRead(t, filepath.Join(failureOutput, "provider-check.json"))) + if !strings.Contains(report, `"status": "failed"`) || !strings.Contains(report, `"missing-provider"`) { + t.Fatalf("provider failure report = %s", report) + } +} + func TestGitHubAppTokenIsHostOnly(t *testing.T) { script, err := os.ReadFile("../scripts/pr-review-local.sh") if err != nil { @@ -246,7 +331,7 @@ func TestGitHubAppTokenIsHostOnly(t *testing.T) { } for name, want := range map[string]string{ "review-agent": "opencode", - "codex-inference-provider": "openai-review", + "codex-inference-provider": "openai-inference", "codex-model": "gpt-5.6-luna", } { input, ok := sharedTrigger.Inputs[name] @@ -254,6 +339,9 @@ func TestGitHubAppTokenIsHostOnly(t *testing.T) { t.Fatalf("shared review workflow default %s = %#v, want %s", name, input, want) } } + if input, ok := sharedTrigger.Inputs["required-providers"]; !ok || input.Default != "[]" { + t.Fatalf("shared review workflow default required-providers = %#v, want []", input) + } for _, name := range []string{"VERTEX_AI_SERVICE_ACCOUNT_KEY", "OPENSHELL_GITHUB_APP_PRIVATE_KEY"} { if _, ok := sharedTrigger.Secrets[name]; !ok { t.Fatalf("shared review workflow does not declare secret %s", name)