diff --git a/.github/workflows/codeboarding-sync.yml b/.github/workflows/codeboarding-sync.yml index 2700bc0..2660515 100644 --- a/.github/workflows/codeboarding-sync.yml +++ b/.github/workflows/codeboarding-sync.yml @@ -141,6 +141,9 @@ jobs: - uses: ./ with: mode: sync + # CodeBoarding's own repositories run on the CodeBoarding plan. + llm: license + license_key: ${{ secrets.CODEBOARDING_LICENSE }} sync_strategy: ${{ inputs.sync_strategy || 'push' }} force_full: ${{ inputs.force_full || false }} # App token authenticates the baseline push so the commit is attributed diff --git a/.github/workflows/codeboarding.yml b/.github/workflows/codeboarding.yml index 16330cc..74b9bc9 100644 --- a/.github/workflows/codeboarding.yml +++ b/.github/workflows/codeboarding.yml @@ -111,4 +111,8 @@ jobs: echo "::warning::CodeBoarding GitHub App token is unavailable; falling back to github-actions[bot]. Check CODEBOARDING_APP_PRIVATE_KEY formatting if app credentials are configured." - uses: ./ with: + # Named, not inferred: the action refuses to guess where credentials come + # from, so every workflow says which of hosted / license / a provider it + # wants. CodeBoarding's own repositories run on the CodeBoarding plan. + llm: anthropic github_token: ${{ steps.codeboarding-app-token-client.outputs.token || steps.codeboarding-app-token-app.outputs.token || github.token }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7c8219e..e910cb9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,6 +40,8 @@ jobs: exit 1 fi python -m pip install --disable-pip-version-check "$requirement" + - name: Check the provider table against the installed release + run: python -m unittest tests.test_provider_table_drift -v - name: Render with the installed CodeBoarding release run: >- python scripts/diff_to_mermaid.py diff --git a/AGENTS.md b/AGENTS.md index d727d58..fd9fbbd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,19 @@ pinned to a release in `action.yml`. `scripts/engine_adapter.py` is the CLI adapter into it (no analysis logic lives there). Engine changes reach users only when that pin is bumped *and* a new action release ships. +## Bumping the engine pin + +`scripts/action/supported-providers.json` mirrors the pinned release's `LLM_PROVIDERS`. +Credentials are validated before the engine is installed — that is what makes a +misconfigured run fail in seconds instead of a minute — so the action cannot ask +the engine at run time and keeps this copy instead. + +When you change the `codeboarding==` pin in `action.yml`, update that file in the +same commit: its `engine` field, and any provider whose selection variables +changed. `tests/test_provider_table_drift.py` runs in the `core-compatibility` CI +job and fails when they disagree. Adding a provider also means adding its +`_api_key` input to `action.yml`; `tests/test_action_inputs.py` checks that. + ## Protected tests Some tests encode a behavioural contract that is expensive to rediscover once @@ -35,6 +48,12 @@ Protected tests: its merge base, never against the base branch tip. Comparing against the tip attributes other people's commits to the pull request and reports them backwards, as removals. +- `tests/test_action_auth.py::test_a_named_provider_never_falls_back_to_codeboarding_credentials` + — a workflow that names a provider runs on that provider or fails. The action + used to read an empty key as "no preference" and resolve it to CodeBoarding's + hosted tier, so a repository that had not added its secret yet went green + while running on another vendor's model and CodeBoarding's money, silently. + Any change that reintroduces a credential fallback breaks this test. ## Releases @@ -56,6 +75,13 @@ messages, so every commit and PR title must follow Conventional Commits: - `fix:` → patch bump - `feat!:` / `fix!:` or a `BREAKING CHANGE:` footer → major bump. Avoid unless intended: consumers pinned to the old major tag never receive it automatically. + +A deliberate exception exists. The explicit-credentials change (`llm` required, +no fallback) is a breaking change that shipped as `feat:`, not `feat!:`. A major +bump moves adopters to `v2` and freezes `v1`, which would have left every +existing workflow on the old silent-fallback behaviour forever — the opposite of +the intent. Shipping it as a minor bump on the moving `v1` tag is what makes +adopters actually receive it. Do not "correct" this to `feat!:` after the fact. - `chore:` / `docs:` / `ci:` / `refactor:` / `test:` → ride along in the next release but do not trigger one. diff --git a/README.md b/README.md index 9166cd3..22827d3 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,9 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: - - uses: CodeBoarding/CodeBoarding-action@v2 + - uses: CodeBoarding/CodeBoarding-action@v1 + with: + llm: hosted # or license, or a provider name -- see Authentication ``` Automatic runs update one sticky **CodeBoarding review** comment. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment. @@ -98,29 +100,93 @@ Fork pull requests never carry an analysis forward. They are reviewed on request ## Authentication and providers -With no LLM inputs, the action uses CodeBoarding's hosted OpenRouter tier. It mints short-lived GitHub OIDC credentials per request, so the job needs `id-token: write` and no stored LLM secret. - -For a direct provider, pass its name and key: +The `llm` input is required and says where analysis credentials come from. There are +three answers, and the action never picks one for you: ```yaml - - uses: CodeBoarding/CodeBoarding-action@v2 - with: - llm_provider: anthropic - llm_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + with: + llm: hosted # CodeBoarding's free tier +``` +```yaml + with: + llm: license # a CodeBoarding plan + license_key: ${{ secrets.CODEBOARDING_LICENSE }} +``` +```yaml + with: + llm: anthropic # your own provider key + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} ``` -The action maps a provider name to the environment variable Core uses (`anthropic` → `ANTHROPIC_API_KEY`, `openai` → `OPENAI_API_KEY`, and so on). `aws`/`aws_bedrock` maps to `AWS_BEARER_TOKEN_BEDROCK`. Provider names following the standard convention are not restricted by an action-side allowlist; the pinned Core release remains the source of truth for which providers it implements. - -CodeBoarding 0.13.10 supports OpenRouter, OpenAI-compatible endpoints, Anthropic-compatible endpoints, Google, Vercel AI Gateway, AWS Bedrock, Cerebras, DeepSeek, GLM, Kimi, OrcaRouter, Ollama, and LiteLLM. See Core's [`agents/llm_config.py`](https://github.com/CodeBoarding/CodeBoarding/blob/main/agents/llm_config.py) for current defaults and endpoint variables. In particular, Ollama needs `OLLAMA_BASE_URL` or `OLLAMA_HOST`, and LiteLLM needs `LITELLM_BASE_URL` on the action step. +`hosted` and `license` run through CodeBoarding's proxy and need `id-token: write`, which +mints short-lived credentials per request and stores no LLM secret in your repository. A +provider key is used directly and needs no OIDC permission. -A CodeBoarding license keeps the hosted OIDC path but removes hosted quota limits: +**An empty value is never a fallback.** If you name a provider and its key is missing -- +because the secret does not exist yet, or is misspelt — the run fails in its first +seconds and says which input and which secret to fix. It does not quietly analyze on +CodeBoarding's hosted tier instead. That was the old behaviour, and it meant a repository +could report an Anthropic review that Anthropic never produced. -```yaml - with: - license_key: ${{ secrets.CODEBOARDING_LICENSE }} -``` +The same rule makes the combinations explicit rather than order-dependent: -`llm_api_key` takes precedence over `license_key`. A direct provider key does not require `id-token: write`; hosted free and licensed usage does. +| Workflow says | Result | +|---|---| +| nothing | refused: `llm` is required | +| `llm: hosted` | the free tier | +| `llm: hosted` + any provider key | refused: pick one | +| `llm: hosted` + `license_key` | refused: use `llm: license` | +| `llm: license` without `license_key` | refused: names the secret to add | +| `llm: anthropic` + `anthropic_api_key` | Anthropic, directly | +| `llm: anthropic`, key empty or absent | refused: names the input and the secret | +| `llm: anthropic` + `openai_api_key` | refused: a second provider's key | +| `llm: anthropic` + key + `license_key` | Anthropic, on a CodeBoarding plan | + +A licence alongside your own key is deliberately allowed: it says "my CodeBoarding plan, +my own tokens". Direct provider calls never reach our proxy, so nothing meters that +combination today; it is recorded and reported, not enforced. + +### Providers + +Each provider has its own inputs, so which key a workflow uses is readable from the file +without knowing any precedence rules. + +| `llm` | Provider | Inputs | Needs at least one of | +|---|---|---|---| +| `anthropic` | Anthropic | `anthropic_api_key` | `anthropic_api_key` | +| `aws_bedrock` | AWS Bedrock | `aws_bedrock_api_key`, `aws_bedrock_region` | `aws_bedrock_api_key` | +| `cerebras` | Cerebras | `cerebras_api_key` | `cerebras_api_key` | +| `deepseek` | DeepSeek | `deepseek_api_key`, `deepseek_base_url` | `deepseek_api_key` or `deepseek_base_url` | +| `glm` | GLM | `glm_api_key`, `glm_base_url` | `glm_api_key` or `glm_base_url` | +| `google` | Google Gemini | `google_api_key` | `google_api_key` | +| `kimi` | Kimi | `kimi_api_key`, `kimi_base_url` | `kimi_api_key` or `kimi_base_url` | +| `litellm` | LiteLLM | `litellm_api_key`, `litellm_base_url` | `litellm_base_url` | +| `ollama` | Ollama | `ollama_api_key`, `ollama_base_url` | `ollama_base_url` | +| `openai` | OpenAI | `openai_api_key`, `openai_base_url` | `openai_api_key` or `openai_base_url` | +| `openrouter` | OpenRouter | `openrouter_api_key` | `openrouter_api_key` | +| `orcarouter` | OrcaRouter | `orcarouter_api_key` | `orcarouter_api_key` | +| `vercel` | Vercel AI Gateway | `vercel_api_key`, `vercel_base_url` | `vercel_api_key` or `vercel_base_url` | + +Each provider has exactly one accepted spelling, and its inputs are named after it, so +`llm: X` always pairs with `X_api_key`. There are no aliases: a second spelling is another +thing to document and keep in step, and an unrecognised value is refused with the accepted +list. `ollama` and `litellm` are selected by their endpoint rather than a key, which is why +a key alone does not configure them — that mirrors how Core itself decides. + +This table is generated from [`scripts/action/supported-providers.json`](scripts/action/supported-providers.json), +which mirrors the CodeBoarding release this action pins. `tests/test_provider_table_drift.py` +installs that release in CI and fails if the two disagree, so a provider cannot be added +to Core and silently stay unreachable here. + +### Reporting + +Every run reports what it resolved, so the answer never has to be inferred from behaviour: + +- outputs `llm_tier` (`hosted`, `license`, `byok`, `byok+license`), `llm_provider`, and + `llm_config_error` (empty when configured); +- a job-summary table naming the tier and provider; +- on a configuration failure, an error annotation and — in review mode — a pull request + comment with the fix, so the person who has to add the secret sees it where they are. ## Model selection @@ -189,9 +255,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: - - uses: CodeBoarding/CodeBoarding-action@v2 + - uses: CodeBoarding/CodeBoarding-action@v1 with: mode: sync + llm: hosted target_branch: main force_full: ${{ inputs.force_full || false }} ``` @@ -209,9 +276,10 @@ permissions: id-token: write # ... - - uses: CodeBoarding/CodeBoarding-action@v2 + - uses: CodeBoarding/CodeBoarding-action@v1 with: mode: sync + llm: hosted target_branch: main sync_strategy: pull_request ``` @@ -225,9 +293,11 @@ With the default `github.token`, the repository or organization must allow GitHu | Input | Mode | Default | Description | |---|---|---|---| | `mode` | both | `review` | `review` or `sync`. | -| `llm_api_key` | both | empty | Direct-provider key. With the default provider, empty selects hosted OIDC usage. | -| `llm_provider` | both | `openrouter` | Provider for `llm_api_key`. | -| `license_key` | both | empty | License for unmetered hosted usage. | +| `llm` | both | **required** | `hosted`, `license`, or a provider name. No default. | +| `_api_key` | both | empty | That provider's key, e.g. `anthropic_api_key`. See [Providers](#providers). | +| `_base_url` | both | empty | That provider's endpoint, where it has one. | +| `aws_bedrock_region` | both | empty | Bedrock region. Core defaults to `us-east-1`. | +| `license_key` | both | empty | CodeBoarding license. Required by `llm: license`. | | `model` | both | empty | Default model for both analysis and parsing. | | `agent_model` | both | empty | Analysis-only override for `model`. | | `parsing_model` | both | empty | Parsing-only override for `model`. | @@ -237,12 +307,15 @@ With the default `github.token`, the repository or organization must allow GitHu | `force_full` | sync | `false` | Ignore the committed baseline for this run. | | `warmstart_retention_days` | review | `1` | Days to keep the reusable analysis. Only the next run reads it. | -The `/codeboarding` command, comment heading, Mermaid direction (`LR`), hosted webview URL, rolling sync branch, commit message, and CodeBoarding 0.13.10 version are intentionally fixed in v2 rather than exposed as configuration. +The `/codeboarding` command, comment heading, Mermaid direction (`LR`), hosted webview URL, rolling sync branch, commit message, and CodeBoarding 0.13.10 version are intentionally fixed rather than exposed as configuration. ## Outputs | Output | Mode | Description | |---|---|---| +| `llm_tier` | both | `hosted`, `license`, `byok`, or `byok+license`. | +| `llm_provider` | both | Provider the run used. | +| `llm_config_error` | both | Configuration failure code, empty when configured. | | `diagram_md` | review | Path to the rendered Mermaid block on the runner. | | `n_changed` | review | Number of changed components. | | `truncated` | review | Whether the graph was reduced to fit GitHub limits. | diff --git a/action.yml b/action.yml index 220246e..e09a074 100644 --- a/action.yml +++ b/action.yml @@ -9,16 +9,108 @@ inputs: description: 'review posts a PR architecture diff; sync updates the versioned analysis baseline.' required: false default: 'review' - llm_api_key: - description: 'Optional direct-provider key. With the default provider, empty selects hosted OIDC usage.' + llm: + description: 'Required. Where analysis credentials come from: hosted, license, or a provider name (anthropic, aws_bedrock, cerebras, deepseek, glm, google, kimi, litellm, ollama, openai, openrouter, orcarouter, vercel).' + required: true + license_key: + description: 'CodeBoarding license key. Required by llm: license; optional alongside a provider key.' required: false default: '' - llm_provider: - description: 'Provider for llm_api_key, for example openrouter, anthropic, openai, google, or ollama.' + # Anthropic - selected by llm: anthropic + anthropic_api_key: + description: 'Anthropic API key, used when llm is anthropic. Sets ANTHROPIC_API_KEY.' required: false - default: 'openrouter' - license_key: - description: 'Optional CodeBoarding license for unmetered hosted usage.' + default: '' + # AWS Bedrock - selected by llm: aws_bedrock + aws_bedrock_api_key: + description: 'AWS Bedrock API key, used when llm is aws_bedrock. Sets AWS_BEARER_TOKEN_BEDROCK.' + required: false + default: '' + aws_bedrock_region: + description: 'AWS Bedrock region, used when llm is aws_bedrock. Sets AWS_DEFAULT_REGION.' + required: false + default: '' + # Cerebras - selected by llm: cerebras + cerebras_api_key: + description: 'Cerebras API key, used when llm is cerebras. Sets CEREBRAS_API_KEY.' + required: false + default: '' + # DeepSeek - selected by llm: deepseek + deepseek_api_key: + description: 'DeepSeek API key, used when llm is deepseek. Sets DEEPSEEK_API_KEY.' + required: false + default: '' + deepseek_base_url: + description: 'DeepSeek endpoint, used when llm is deepseek. Sets DEEPSEEK_BASE_URL.' + required: false + default: '' + # GLM - selected by llm: glm + glm_api_key: + description: 'GLM API key, used when llm is glm. Sets GLM_API_KEY.' + required: false + default: '' + glm_base_url: + description: 'GLM endpoint, used when llm is glm. Sets GLM_BASE_URL.' + required: false + default: '' + # Google Gemini - selected by llm: google + google_api_key: + description: 'Google Gemini API key, used when llm is google. Sets GOOGLE_API_KEY.' + required: false + default: '' + # Kimi - selected by llm: kimi + kimi_api_key: + description: 'Kimi API key, used when llm is kimi. Sets KIMI_API_KEY.' + required: false + default: '' + kimi_base_url: + description: 'Kimi endpoint, used when llm is kimi. Sets KIMI_BASE_URL.' + required: false + default: '' + # LiteLLM - selected by llm: litellm + litellm_api_key: + description: 'LiteLLM API key, used when llm is litellm. Sets LITELLM_API_KEY.' + required: false + default: '' + litellm_base_url: + description: 'LiteLLM endpoint, used when llm is litellm. Sets LITELLM_BASE_URL.' + required: false + default: '' + # Ollama - selected by llm: ollama + ollama_api_key: + description: 'Ollama API key, used when llm is ollama. Sets OLLAMA_API_KEY.' + required: false + default: '' + ollama_base_url: + description: 'Ollama endpoint, used when llm is ollama. Sets OLLAMA_BASE_URL.' + required: false + default: '' + # OpenAI - selected by llm: openai + openai_api_key: + description: 'OpenAI API key, used when llm is openai. Sets OPENAI_API_KEY.' + required: false + default: '' + openai_base_url: + description: 'OpenAI endpoint, used when llm is openai. Sets OPENAI_BASE_URL.' + required: false + default: '' + # OpenRouter - selected by llm: openrouter + openrouter_api_key: + description: 'OpenRouter API key, used when llm is openrouter. Sets OPENROUTER_API_KEY.' + required: false + default: '' + # OrcaRouter - selected by llm: orcarouter + orcarouter_api_key: + description: 'OrcaRouter API key, used when llm is orcarouter. Sets ORCAROUTER_API_KEY.' + required: false + default: '' + # Vercel AI Gateway - selected by llm: vercel + vercel_api_key: + description: 'Vercel AI Gateway API key, used when llm is vercel. Sets VERCEL_API_KEY.' + required: false + default: '' + vercel_base_url: + description: 'Vercel AI Gateway endpoint, used when llm is vercel. Sets VERCEL_BASE_URL.' required: false default: '' model: @@ -54,6 +146,15 @@ inputs: required: false default: '1' outputs: + llm_tier: + description: 'Resolved credential tier: hosted, license, byok, or byok+license.' + value: ${{ steps.llm.outputs.tier }} + llm_provider: + description: 'Provider this run used for analysis.' + value: ${{ steps.llm.outputs.provider }} + llm_config_error: + description: 'Stable code for a credential configuration failure, empty when configured.' + value: ${{ steps.llm.outputs.error }} diagram_md: description: 'Path to the rendered Mermaid review diagram.' value: ${{ steps.review_render.outputs.diagram_md }} @@ -130,6 +231,81 @@ runs: COMMENT_ID: ${{ github.event.comment.id }} run: GH_HOST="${GITHUB_SERVER_URL#*://}" gh api -X POST "repos/${REPOSITORY}/issues/comments/${COMMENT_ID}/reactions" -f content=eyes >/dev/null + # Ahead of the credential check rather than after the checkout, because that check + # is a Python program. Hosted runners ship a system python3 and would not notice; + # a self-hosted runner without one would fail a perfectly valid configuration. It + # costs a couple of seconds and still leaves the check ahead of the engine install, + # which is the part worth failing before. + - name: Setup Python + if: steps.guard.outputs.skip != 'true' + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + # Resolved before the checkout and the engine install, so a workflow that names a + # provider it has no key for fails within seconds, naming the input to fix, instead + # of running to completion on credentials it never asked for. Nothing downstream + # re-decides this: the plan it writes is what the analysis runs with. + - name: Check LLM configuration + id: llm + if: steps.guard.outputs.skip != 'true' + continue-on-error: true + shell: bash + env: + ACTION_PATH: ${{ github.action_path }} + CB_IN_LLM: ${{ inputs.llm }} + CB_IN_LICENSE_KEY: ${{ inputs.license_key }} + CB_IN_ANTHROPIC_API_KEY: ${{ inputs.anthropic_api_key }} + CB_IN_AWS_BEDROCK_API_KEY: ${{ inputs.aws_bedrock_api_key }} + CB_IN_AWS_BEDROCK_REGION: ${{ inputs.aws_bedrock_region }} + CB_IN_CEREBRAS_API_KEY: ${{ inputs.cerebras_api_key }} + CB_IN_DEEPSEEK_API_KEY: ${{ inputs.deepseek_api_key }} + CB_IN_DEEPSEEK_BASE_URL: ${{ inputs.deepseek_base_url }} + CB_IN_GLM_API_KEY: ${{ inputs.glm_api_key }} + CB_IN_GLM_BASE_URL: ${{ inputs.glm_base_url }} + CB_IN_GOOGLE_API_KEY: ${{ inputs.google_api_key }} + CB_IN_KIMI_API_KEY: ${{ inputs.kimi_api_key }} + CB_IN_KIMI_BASE_URL: ${{ inputs.kimi_base_url }} + CB_IN_LITELLM_API_KEY: ${{ inputs.litellm_api_key }} + CB_IN_LITELLM_BASE_URL: ${{ inputs.litellm_base_url }} + CB_IN_OLLAMA_API_KEY: ${{ inputs.ollama_api_key }} + CB_IN_OLLAMA_BASE_URL: ${{ inputs.ollama_base_url }} + CB_IN_OPENAI_API_KEY: ${{ inputs.openai_api_key }} + CB_IN_OPENAI_BASE_URL: ${{ inputs.openai_base_url }} + CB_IN_OPENROUTER_API_KEY: ${{ inputs.openrouter_api_key }} + CB_IN_ORCAROUTER_API_KEY: ${{ inputs.orcarouter_api_key }} + CB_IN_VERCEL_API_KEY: ${{ inputs.vercel_api_key }} + CB_IN_VERCEL_BASE_URL: ${{ inputs.vercel_base_url }} + run: "$GITHUB_ACTION_PATH/scripts/action/verify-credentials.sh" + + # Said in the pull request, not only in the Actions tab, because the person who has + # to add the secret is the one reading the pull request. + - name: Report LLM configuration failure + if: steps.guard.outputs.skip != 'true' && steps.llm.outputs.error != '' && steps.guard.outputs.mode == 'review' && steps.guard.outputs.pr_number != '' + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: ${{ steps.guard.outputs.comment_id }} + number: ${{ steps.guard.outputs.pr_number }} + GITHUB_TOKEN: ${{ inputs.github_token }} + message: | + ### CodeBoarding review - not configured + + ${{ steps.llm.outputs.details }} + + No analysis ran, and no CodeBoarding hosted usage was consumed. + + run [${{ github.run_id }}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) - attempt ${{ github.run_attempt }} + + - name: Stop on LLM configuration failure + if: steps.guard.outputs.skip != 'true' && steps.llm.outputs.error != '' + shell: bash + env: + CB_ERROR: ${{ steps.llm.outputs.error }} + run: | + echo "CodeBoarding did not run: $CB_ERROR" >&2 + exit 1 + - name: Post review progress if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' continue-on-error: true @@ -156,12 +332,6 @@ runs: persist-credentials: false path: .codeboarding-target - - name: Setup Python - if: steps.guard.outputs.skip != 'true' - uses: actions/setup-python@v5 - with: - python-version: '3.12' - - name: Setup Java for CodeBoarding if: steps.guard.outputs.skip != 'true' uses: actions/setup-java@v5 @@ -188,9 +358,6 @@ runs: shell: bash env: ACTION_PATH: ${{ github.action_path }} - LLM_API_KEY: ${{ inputs.llm_api_key }} - LLM_PROVIDER: ${{ inputs.llm_provider }} - LICENSE_KEY: ${{ inputs.license_key }} run: "$GITHUB_ACTION_PATH/scripts/action/configure-auth.sh" - name: Resolve analysis identity @@ -203,7 +370,8 @@ runs: MERGE_BASE_SHA: ${{ steps.guard.outputs.merge_base_sha }} PR_NUMBER: ${{ steps.guard.outputs.pr_number }} IS_FORK: ${{ steps.guard.outputs.is_fork }} - LLM_PROVIDER: ${{ inputs.llm_provider }} + LLM_PROVIDER: ${{ steps.llm.outputs.provider }} + BACKEND_ID: ${{ steps.llm.outputs.backend_id }} MODEL: ${{ inputs.model }} AGENT_MODEL_INPUT: ${{ inputs.agent_model }} PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} @@ -449,8 +617,12 @@ runs: GITHUB_TOKEN: ${{ inputs.github_token }} path: ${{ steps.review_body.outputs.path }} + # Skipped when the run stopped on a credential problem: that path already replaced + # this same sticky comment with the input and secret to fix, and "see the workflow + # logs" posted over the top of it would send the reader hunting for what they had + # just been told. - name: Post review failure - if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_comment.outcome != 'success' + if: failure() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'review' && steps.review_comment.outcome != 'success' && steps.llm.outputs.error == '' continue-on-error: true uses: marocchino/sticky-pull-request-comment@v2 with: diff --git a/scripts/action/configure-auth.sh b/scripts/action/configure-auth.sh index 0a5beda..6abcc9d 100755 --- a/scripts/action/configure-auth.sh +++ b/scripts/action/configure-auth.sh @@ -1,44 +1,32 @@ #!/usr/bin/env bash -# Selects direct or hosted authentication and stores scoped credentials for analysis. +# Starts the hosted credential relay when the resolved plan calls for it. +# +# The plan itself was decided by verify-credentials.sh before the checkout; this step only +# does the part that needs the engine's Python present. A direct-provider run has +# nothing to do here: its key never leaves the runner. set -euo pipefail AUTH_DIR="${RUNNER_TEMP}/codeboarding-auth" HOSTED_PROXY_URL="https://auduihjmm4b735zci7vyabuikq0hppqn.lambda-url.us-east-1.on.aws" umask 077 -rm -rf "$AUTH_DIR" -mkdir -p "$AUTH_DIR" -provider="$(printf '%s' "${LLM_PROVIDER:-openrouter}" | tr '[:upper:]-' '[:lower:]_' | tr -cd 'a-z0-9_')" -[ -n "$provider" ] || { echo "::error::llm_provider is empty."; exit 1; } -printf '%s' "$provider" > "$AUTH_DIR/provider-name" -if [ -n "${LLM_API_KEY:-}" ] || [ "$provider" != openrouter ]; then - case "$provider" in - aws|aws_bedrock) provider_env="AWS_BEARER_TOKEN_BEDROCK" ;; - *) provider_env="$(printf '%s' "$provider" | tr '[:lower:]' '[:upper:]')_API_KEY" ;; - esac - printf '%s' "$provider_env" > "$AUTH_DIR/provider-env" - if [ -n "${LLM_API_KEY:-}" ]; then - key="$(printf '%s' "$LLM_API_KEY" | tr -d '[:space:]' | sed -e 's/^"//;s/"$//' -e "s/^'//;s/'\$//" -e "s/^${provider_env}=//" -e 's/^"//;s/"$//' -e "s/^'//;s/'\$//")" - printf '::add-mask::%s\n::add-mask::%s\n' "$LLM_API_KEY" "$key" - printf '%s' "$key" > "$AUTH_DIR/provider-key" - fi - echo "Using direct $provider credentials." - exit 0 -fi -if [ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" ] || [ -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ]; then - echo "::error::Missing OIDC token. Add permissions: id-token: write." && exit 1 + +if [ ! -s "$AUTH_DIR/tier" ]; then + echo "::error::CodeBoarding analysis credentials are unavailable; the configuration step did not run." + exit 1 fi +TIER="$(cat "$AUTH_DIR/tier")" + +case "$TIER" in + hosted|license) ;; + *) echo "Using direct $(cat "$AUTH_DIR/provider-name") credentials."; exit 0 ;; +esac + READY="$AUTH_DIR/ready-port" PID="$AUTH_DIR/relay.pid" LOG="$AUTH_DIR/relay.log" -LICENSE_FILE="$AUTH_DIR/license.txt" -: > "$LICENSE_FILE" -if [ -n "${LICENSE_KEY:-}" ]; then - echo "::add-mask::$LICENSE_KEY" - printf '%s' "$LICENSE_KEY" > "$LICENSE_FILE" -fi RELAY_ARGS=(--upstream-base-url "$HOSTED_PROXY_URL" --ready-file "$READY") -if [ -s "$LICENSE_FILE" ]; then - RELAY_ARGS+=(--license-file "$LICENSE_FILE") +if [ -s "$AUTH_DIR/license.txt" ]; then + RELAY_ARGS+=(--license-file "$AUTH_DIR/license.txt") fi python3 "$ACTION_PATH/scripts/oidc_relay.py" "${RELAY_ARGS[@]}" > "$LOG" 2>&1 & @@ -59,6 +47,7 @@ if [ ! -s "$READY" ]; then fi PORT="$(cat "$READY")" -printf '%s' "OPENROUTER_API_KEY" > "$AUTH_DIR/provider-env" -printf '%s' "github-actions-oidc-relay" > "$AUTH_DIR/provider-key" -printf '%s' "http://127.0.0.1:$PORT" > "$AUTH_DIR/base-url" +mkdir -p "$AUTH_DIR/env" +printf '%s' "github-actions-oidc-relay" > "$AUTH_DIR/env/OPENROUTER_API_KEY" +printf '%s' "http://127.0.0.1:$PORT" > "$AUTH_DIR/env/OPENROUTER_BASE_URL" +echo "Using CodeBoarding hosted credentials ($TIER)." diff --git a/scripts/action/credential_check.py b/scripts/action/credential_check.py new file mode 100755 index 0000000..bfe6054 --- /dev/null +++ b/scripts/action/credential_check.py @@ -0,0 +1,386 @@ +#!/usr/bin/env python3 +"""The run's LLM credentials: what they resolve to, or what to tell the user instead. + +Both halves live here on purpose. The `message` on every ConfigError is the sentence the +user actually reads -- verify-credentials.sh puts it in the step output, and action.yml posts +that same string as the pull request comment, the error annotation and the job summary -- +so the rule and its explanation are written together and cannot drift apart. + +One provider, chosen explicitly by the `llm` input, and nothing is ever reached by an +empty string falling through to a default. A misconfigured run fails here -- before the +checkout and the engine install -- naming the input and the secret to fix, rather than +succeeding on someone else's credentials or failing later inside the engine. + +Reads the action's inputs from CB_IN_* environment variables (prefixed so that wiring an +input can never itself set a provider selection variable), and writes the resolved +environment to an auth directory the analysis steps read. Prints a JSON summary that +carries no secret values. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import sys +from pathlib import Path + +DOCS = "https://github.com/CodeBoarding/CodeBoarding-action#authentication-and-providers" +SETTINGS_HINT = "Settings -> Secrets and variables -> Actions" +TABLE = Path(__file__).resolve().parent / "supported-providers.json" + + +#: Every reason this module can refuse a configuration. +#: +#: Declared rather than left implicit in the raise sites, for two reasons. These codes are +#: an interface: the action emits them as `llm_config_error` and the webview keys on them, +#: so adding one silently is a contract change nobody reviewed. And a code with no test is +#: invisible -- `license_with_provider_key` shipped untested precisely because nothing +#: enumerated the set. `test_llm_contract.py` asserts every entry here is exercised. +ERROR_CODES = frozenset( + { + "missing_llm", + "unknown_llm", + "missing_provider_key", + "missing_license_key", + "missing_id_token", + "hosted_with_provider_key", + "hosted_with_license", + "license_with_provider_key", + "foreign_provider_key", + } +) + + +class ConfigError(Exception): + """A configuration the action refuses to run, with the code the webview keys on. + + Two renderings, because they go to surfaces with different rules. ``message`` is one + plain line, for the ``::error::`` annotation, which cannot carry newlines. ``details`` + is markdown for the pull request comment and the job summary, where a link and a + snippet the reader can copy are worth far more than a sentence describing them. + """ + + def __init__(self, code: str, message: str, details: str | None = None) -> None: + super().__init__(message) + assert code in ERROR_CODES, f"undeclared error code: {code}" + self.code = code + self.message = message + self.details = details or message + + +def secrets_url(environ: dict[str, str]) -> str | None: + """This repository's "new secret" page, when the runner tells us which repo we are in.""" + repo = environ.get("GITHUB_REPOSITORY", "") + if not repo: + return None + server = environ.get("GITHUB_SERVER_URL", "https://github.com").rstrip("/") + return f"{server}/{repo}/settings/secrets/actions/new" + + +def workflow_path(environ: dict[str, str]) -> str: + """The workflow file to edit, named rather than left for the reader to find. + + GITHUB_WORKFLOW_REF is ``owner/repo/.github/workflows/x.yml@refs/...``; a repository + can have several workflows calling this action, so "add it to your workflow" is not + an instruction someone can follow without guessing. + """ + ref = environ.get("GITHUB_WORKFLOW_REF", "") + path = ref.split("@", 1)[0] + _, _, tail = path.partition("/") + _, _, tail = tail.partition("/") + return tail or "your CodeBoarding workflow" + + +def _add_secret(environ: dict[str, str], secret: str, what: str) -> str: + """Step one of every credential remedy: put the value in the repository.""" + url = secrets_url(environ) + where = f"[Add a repository secret]({url})" if url else "Add a repository secret" + return f"{where} named `{secret}`, with {what} as the value." + + +def _wire_it(environ: dict[str, str], llm: str, lines: list[str]) -> str: + """Step two: the exact YAML, in the exact file, indented as it will sit there.""" + body = "\n".join(f" {line}" for line in lines) + return ( + f"In `{workflow_path(environ)}`, the CodeBoarding step's `with:` block " + f"needs to read:\n\n```yaml\n with:\n llm: {llm}\n{body}\n```" + ) + + +def load_table(path: Path = TABLE) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +def _clean_key(raw: str) -> str: + """Undo the ways a pasted key arrives wrapped: whitespace, quotes, a VAR= prefix.""" + value = re.sub(r"\s+", "", raw) + for _ in range(2): + value = re.sub(r'^"(.*)"$', r"\1", value) + value = re.sub(r"^'(.*)'$", r"\1", value) + value = re.sub(r"^[A-Z0-9_]+=", "", value) + for _ in range(2): + value = re.sub(r'^"(.*)"$', r"\1", value) + value = re.sub(r"^'(.*)'$", r"\1", value) + return value + + +def read_inputs(table: dict, environ: dict[str, str]) -> dict[str, str]: + """Every provider input that carries a value, keyed by input name.""" + values: dict[str, str] = {} + for provider in table["providers"].values(): + for input_name in provider["inputs"]: + raw = environ.get(f"CB_IN_{input_name.upper()}", "") + value = _clean_key(raw) if input_name.endswith("_api_key") else raw.strip() + if value: + values[input_name] = value + return values + + +def owner_of(table: dict, input_name: str) -> str: + for name, provider in table["providers"].items(): + if input_name in provider["inputs"]: + return name + raise KeyError(input_name) + + +def _provider_list(table: dict) -> str: + return ", ".join(sorted(table["providers"])) + + +def _reject_provider_inputs(table: dict, given: dict[str, str], llm: str, tier: str) -> None: + """`llm: hosted`/`license` run on CodeBoarding's credentials; a provider key means the + workflow is asking for two different things at once.""" + if not given: + return + input_name = sorted(given)[0] + provider = owner_of(table, input_name) + raise ConfigError( + f"{tier}_with_provider_key", + f"`llm: {llm}` runs on CodeBoarding's hosted tier, but `{input_name}` is set. " + f"Remove it, or set `llm: {provider}` to run on that key instead.", + ) + + +def _require_id_token(llm: str, environ: dict[str, str]) -> None: + if environ.get("ACTIONS_ID_TOKEN_REQUEST_URL"): + return + raise ConfigError( + "missing_id_token", + f"`llm: {llm}` authenticates with a GitHub OIDC token, which this job cannot mint. " + "Add `permissions:` with `id-token: write` to the job that uses this action.", + "\n\n".join( + [ + f"`llm: {llm}` authenticates with a GitHub OIDC token, which this job cannot mint.", + f"In `{workflow_path(environ)}`, the job running this action needs:", + "```yaml\n permissions:\n id-token: write\n```", + "No secret is involved: the token is minted per request and never stored.", + ] + ), + ) + + +def _resolve_byok(table: dict, name: str, given: dict[str, str], environ: dict[str, str]) -> dict: + provider = table["providers"][name] + foreign = sorted(i for i in given if owner_of(table, i) != name) + if foreign: + other = owner_of(table, foreign[0]) + raise ConfigError( + "foreign_provider_key", + f"`llm: {name}` is selected, but `{foreign[0]}` is set, which configures " + f"`{other}`. Set only {provider['label']}'s inputs, or change `llm` to `{other}`.", + ) + + env = {var: given[i] for i, var in provider["inputs"].items() if i in given} + # Core selects a provider when one of its selection_envs is set. Anything else -- an + # API key that is not a selection variable, a region -- cannot make it usable, which + # is why a key alone does not configure ollama or litellm. + if not any(env.get(var) for var in provider["selection_envs"]): + wanted = [i for i, var in provider["inputs"].items() if var in provider["selection_envs"]] + keys = [i for i in wanted if i.endswith("_api_key")] + needed = " or ".join(f"`{i}`" for i in wanted) + # A base URL is configuration, not a credential; only send people to the secrets + # page for the inputs that actually belong there. + if keys: + secret = provider["inputs"][keys[0]] + fix = f"Add the {secret} repository secret ({SETTINGS_HINT}) and wire it as `{keys[0]}`." + details = "\n\n".join( + [ + f"`llm: {name}` needs {needed}, and none is set.", + "**1.** " + _add_secret(environ, secret, f"your {provider['label']} API key"), + "**2.** " + _wire_it(environ, name, [f"{keys[0]}: ${{{{ secrets.{secret} }}}}"]), + ] + ) + else: + fix = f"Set `{wanted[0]}` on the action step to your {provider['label']} endpoint." + details = "\n\n".join( + [ + f"`llm: {name}` needs {needed}, and none is set.", + _wire_it(environ, name, [f"{wanted[0]}: https://your-{name}-host"]), + ] + ) + raise ConfigError( + "missing_provider_key", + f"`llm: {name}` needs {needed}, and none is set. {fix}", + details, + ) + return env + + +def resolve(table: dict, environ: dict[str, str]) -> dict: + """The whole contract. Returns a plan; raises ConfigError with the reason otherwise.""" + llm = environ.get("CB_IN_LLM", "").strip().lower() + license_key = environ.get("CB_IN_LICENSE_KEY", "").strip() + given = read_inputs(table, environ) + + if not llm: + raise ConfigError( + "missing_llm", + "The `llm` input is required and has no default. Set it to `hosted` " + "(CodeBoarding's free tier), `license` (a CodeBoarding plan), or one of: " + f"{_provider_list(table)}. See {DOCS}.", + ) + + if llm == "hosted": + _reject_provider_inputs(table, given, llm, "hosted") + if license_key: + raise ConfigError( + "hosted_with_license", + "`llm: hosted` is the free tier and never spends a licence. Set `llm: license` " + "to run your CodeBoarding plan, or remove `license_key`.", + ) + _require_id_token(llm, environ) + return {"tier": "hosted", "provider": table["hosted_provider"], "env": {}} + + if llm == "license": + _reject_provider_inputs(table, given, llm, "license") + if not license_key: + raise ConfigError( + "missing_license_key", + "`llm: license` needs `license_key`, which is empty. Add the " + f"CODEBOARDING_LICENSE repository secret ({SETTINGS_HINT}) and wire it.", + "\n\n".join( + [ + "`llm: license` needs `license_key`, which is empty.", + "**1.** " + _add_secret(environ, "CODEBOARDING_LICENSE", "your CodeBoarding licence key"), + "**2.** " + _wire_it(environ, "license", ["license_key: ${{ secrets.CODEBOARDING_LICENSE }}"]), + ] + ), + ) + _require_id_token(llm, environ) + return { + "tier": "license", + "provider": table["hosted_provider"], + "license": license_key, + "env": {}, + } + + name = llm + if name not in table["providers"]: + raise ConfigError( + "unknown_llm", + f"`llm: {environ.get('CB_IN_LLM', '').strip()}` is not a value this action " + f"understands. Use `hosted`, `license`, or one of: {_provider_list(table)}. " + f"See {DOCS}.", + ) + + env = _resolve_byok(table, name, given, environ) + # A licence alongside a provider key is deliberately allowed, not an error: it says + # "my CodeBoarding plan, my own tokens". Nothing enforces it yet -- direct provider + # calls never reach our proxy -- so it is recorded for the surfaces that read it. + return { + "tier": "byok+license" if license_key else "byok", + "provider": name, + "env": env, + } + + +def _is_endpoint(var: str) -> bool: + """Configuration rather than credential: safe to name in an artifact, worth hashing.""" + return var.endswith(("_BASE_URL", "_HOST")) or var == "AWS_DEFAULT_REGION" + + +def write_auth_dir(table: dict, plan: dict, auth_dir: Path) -> None: + """Lay the plan out as files the later steps read, readable only by this user.""" + os.umask(0o077) + env_dir = auth_dir / "env" + env_dir.mkdir(parents=True, exist_ok=True) + (auth_dir / "tier").write_text(plan["tier"], encoding="utf-8") + (auth_dir / "provider-name").write_text(plan["provider"], encoding="utf-8") + for var, value in plan["env"].items(): + (env_dir / var).write_text(value, encoding="utf-8") + if plan.get("license"): + (auth_dir / "license.txt").write_text(plan["license"], encoding="utf-8") + # Every variable core knows about, minus the ones this run actually resolved, so + # with-auth.sh can strip the rest without carrying its own copy of the list to fall + # behind on. + # + # Keyed on what was RESOLVED, not on which provider was selected. Sparing every + # variable the selected provider could use would leave an inherited value in place + # for the ones it did not: `llm: openai` with only `openai_base_url` set would let a + # stray OPENAI_API_KEY from the job environment supply the credentials, which is the + # same silent substitution this contract exists to prevent, one provider narrower. + keep = set(plan["env"]) + everything = { + var + for provider in table["providers"].values() + for var in list(provider["selection_envs"]) + list(provider["inputs"].values()) + } + # Trailing newline, because `read` returns non-zero on an unterminated final line and + # a `while read` loop therefore skips it. Without it the alphabetically last variable + # was never unset: an inherited VERCEL_BASE_URL survived an Anthropic run and let core + # see two providers configured. with-auth.sh guards the same case from its side. + # What the run talks to, minus anything secret. The reusable-analysis name is built + # from this: pointing `openai_base_url` at a different backend, or moving Bedrock to + # another region, produces different analysis, so it must not restore a bundle built + # against the old one. Keys are deliberately excluded -- they do not change what the + # model says, and rotating one should not throw away a warm start. + backend = [f"{plan['tier']}:{plan['provider']}"] + backend += [f"{var}={value}" for var, value in sorted(plan["env"].items()) if _is_endpoint(var)] + (auth_dir / "backend-id").write_text("\n".join(backend), encoding="utf-8") + + foreign = sorted(everything - keep) + (auth_dir / "foreign-envs").write_text("".join(f"{var}\n" for var in foreign), encoding="utf-8") + + +def main(argv: list[str]) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--auth-dir", type=Path, help="Write the resolved plan here.") + args = parser.parse_args(argv) + + table = load_table() + try: + plan = resolve(table, dict(os.environ)) + except ConfigError as error: + json.dump( + { + "ok": False, + "error": error.code, + "message": error.message, + "details": error.details, + }, + sys.stdout, + ) + print() + return 1 + + if args.auth_dir: + write_auth_dir(table, plan, args.auth_dir) + json.dump( + { + "ok": True, + "error": "", + "message": "", + "details": "", + "tier": plan["tier"], + "provider": plan["provider"], + }, + sys.stdout, + ) + print() + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/action/state-names.sh b/scripts/action/state-names.sh index a6379c7..e91f3a0 100755 --- a/scripts/action/state-names.sh +++ b/scripts/action/state-names.sh @@ -30,8 +30,11 @@ fi ignore_digest=none ignore_file="$CHECKOUT_DIR/.codeboarding/.codeboardingignore" [ ! -f "$ignore_file" ] || ignore_digest="$(digest < "$ignore_file")" -model_digest="$(printf '%s\n%s\n%s\n%s\n' \ - "${LLM_PROVIDER:-}" "${MODEL:-}" "${AGENT_MODEL_INPUT:-}" "${PARSING_MODEL_INPUT:-}" | digest)" +# BACKEND_ID covers the tier and any endpoint or region the run was pointed at, so moving +# `openai_base_url` to another gateway invalidates a warm start the way changing the model +# does. It carries no key: rotating a secret must not throw away reusable analysis. +model_digest="$(printf '%s\n%s\n%s\n%s\n%s\n' \ + "${LLM_PROVIDER:-}" "${BACKEND_ID:-}" "${MODEL:-}" "${AGENT_MODEL_INPUT:-}" "${PARSING_MODEL_INPUT:-}" | digest)" cfg="$(printf '%s\n%s\n%s\n%s\n' \ "$STATE_SCHEMA" "$engine_version" "$ignore_digest" "$model_digest" | digest)" diff --git a/scripts/action/supported-providers.json b/scripts/action/supported-providers.json new file mode 100644 index 0000000..213cebd --- /dev/null +++ b/scripts/action/supported-providers.json @@ -0,0 +1,157 @@ +{ + "_comment": [ + "The provider contract this action enforces, mirroring the LLM_PROVIDERS table of the", + "CodeBoarding release pinned in action.yml. tests/test_provider_table_drift.py installs", + "that release and fails if the two disagree, so bumping the pin without updating this", + "file cannot ship: a provider core added but this table lacks would otherwise be", + "unreachable, and one core removed would be accepted here and fail mid-analysis.", + "", + "Each key is the ONE value `llm:` accepts for that provider, and each provider's inputs", + "are named after it, so `llm: X` always pairs with `X_api_key`. There are no aliases:", + "a second spelling is a second thing to document, test and keep in step, and the error", + "message lists the accepted values anyway. Where core's internal name differs from the", + "one we ask people to type, `core` carries the translation and nothing else does.", + "", + "'selection_envs' is the load-bearing field. Core selects a provider when ANY of them", + "is set, so 'at least one is non-empty' is exactly what makes a provider usable -- this", + "action does not invent a stricter or looser rule of its own. An api-key env that is not", + "a selection env (AWS_DEFAULT_REGION, OLLAMA_API_KEY) therefore cannot select a provider", + "on its own, which is why ollama and litellm need their base URL and not just a key." + ], + "engine": "0.13.10", + "hosted_provider": "openrouter", + "providers": { + "openrouter": { + "label": "OpenRouter", + "selection_envs": [ + "OPENROUTER_API_KEY" + ], + "inputs": { + "openrouter_api_key": "OPENROUTER_API_KEY" + } + }, + "orcarouter": { + "label": "OrcaRouter", + "selection_envs": [ + "ORCAROUTER_API_KEY" + ], + "inputs": { + "orcarouter_api_key": "ORCAROUTER_API_KEY" + } + }, + "anthropic": { + "label": "Anthropic", + "selection_envs": [ + "ANTHROPIC_API_KEY" + ], + "inputs": { + "anthropic_api_key": "ANTHROPIC_API_KEY" + } + }, + "openai": { + "label": "OpenAI", + "selection_envs": [ + "OPENAI_API_KEY", + "OPENAI_BASE_URL" + ], + "inputs": { + "openai_api_key": "OPENAI_API_KEY", + "openai_base_url": "OPENAI_BASE_URL" + } + }, + "google": { + "label": "Google Gemini", + "selection_envs": [ + "GOOGLE_API_KEY" + ], + "inputs": { + "google_api_key": "GOOGLE_API_KEY" + } + }, + "vercel": { + "label": "Vercel AI Gateway", + "selection_envs": [ + "VERCEL_API_KEY", + "VERCEL_BASE_URL" + ], + "inputs": { + "vercel_api_key": "VERCEL_API_KEY", + "vercel_base_url": "VERCEL_BASE_URL" + } + }, + "aws_bedrock": { + "core": "aws", + "label": "AWS Bedrock", + "selection_envs": [ + "AWS_BEARER_TOKEN_BEDROCK" + ], + "inputs": { + "aws_bedrock_api_key": "AWS_BEARER_TOKEN_BEDROCK", + "aws_bedrock_region": "AWS_DEFAULT_REGION" + } + }, + "cerebras": { + "label": "Cerebras", + "selection_envs": [ + "CEREBRAS_API_KEY" + ], + "inputs": { + "cerebras_api_key": "CEREBRAS_API_KEY" + } + }, + "deepseek": { + "label": "DeepSeek", + "selection_envs": [ + "DEEPSEEK_API_KEY", + "DEEPSEEK_BASE_URL" + ], + "inputs": { + "deepseek_api_key": "DEEPSEEK_API_KEY", + "deepseek_base_url": "DEEPSEEK_BASE_URL" + } + }, + "glm": { + "label": "GLM", + "selection_envs": [ + "GLM_API_KEY", + "GLM_BASE_URL" + ], + "inputs": { + "glm_api_key": "GLM_API_KEY", + "glm_base_url": "GLM_BASE_URL" + } + }, + "kimi": { + "label": "Kimi", + "selection_envs": [ + "KIMI_API_KEY", + "KIMI_BASE_URL" + ], + "inputs": { + "kimi_api_key": "KIMI_API_KEY", + "kimi_base_url": "KIMI_BASE_URL" + } + }, + "ollama": { + "label": "Ollama", + "selection_envs": [ + "OLLAMA_BASE_URL", + "OLLAMA_HOST" + ], + "inputs": { + "ollama_api_key": "OLLAMA_API_KEY", + "ollama_base_url": "OLLAMA_BASE_URL" + } + }, + "litellm": { + "label": "LiteLLM", + "selection_envs": [ + "LITELLM_BASE_URL" + ], + "inputs": { + "litellm_api_key": "LITELLM_API_KEY", + "litellm_base_url": "LITELLM_BASE_URL" + } + } + } +} diff --git a/scripts/action/verify-credentials.sh b/scripts/action/verify-credentials.sh new file mode 100755 index 0000000..47fcd73 --- /dev/null +++ b/scripts/action/verify-credentials.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# Resolves the run's LLM credentials before anything expensive happens. +# +# Runs ahead of the checkout and the engine install so a misconfigured workflow fails in +# seconds with the input to fix named, rather than after a minute of setup or, worse, +# succeeding quietly on credentials the workflow did not ask for. +set -euo pipefail + +AUTH_DIR="${RUNNER_TEMP}/codeboarding-auth" +umask 077 +rm -rf "$AUTH_DIR" + +# Mask before resolving: a value that never reaches the log cannot leak from a later +# failure. The cleaned forms are masked again below, since trimming a pasted wrapper +# produces a string GitHub has not been told about yet. +while IFS= read -r var; do + [ -z "${!var:-}" ] || echo "::add-mask::${!var}" +done < <(compgen -v | grep -E '^CB_IN_.*_API_KEY$' || true) +[ -z "${CB_IN_LICENSE_KEY:-}" ] || echo "::add-mask::${CB_IN_LICENSE_KEY}" + +set +e +plan="$(python3 "${ACTION_PATH}/scripts/action/credential_check.py" --auth-dir "$AUTH_DIR")" +resolved=$? +set -e + +field() { PLAN="$plan" python3 -c ' +import json, os, sys +print(json.loads(os.environ["PLAN"]).get(sys.argv[1], ""))' "$1"; } + +tier="$(field tier)" +provider="$(field provider)" +error="$(field error)" +message="$(field message)" +details="$(field details)" + +backend_id="" +[ ! -s "$AUTH_DIR/backend-id" ] || backend_id="$(cksum < "$AUTH_DIR/backend-id" | cut -d' ' -f1)" + +{ + echo "tier=$tier" + echo "backend_id=$backend_id" + echo "provider=$provider" + echo "error=$error" + echo "message<> "$GITHUB_OUTPUT" + +if [ "$resolved" -ne 0 ]; then + echo "::error title=CodeBoarding LLM configuration::${message}" + { + echo "### CodeBoarding could not start" + echo + echo "$details" + echo + echo "No analysis ran, and no CodeBoarding hosted usage was consumed." + } >> "${GITHUB_STEP_SUMMARY:-/dev/null}" + rm -rf "$AUTH_DIR" + exit 1 +fi + +# The trimmed values are what the analysis actually exports; mask those too. Endpoints +# and regions are configuration rather than credentials and stay readable, so a wrong +# one can be spotted in the log. +if [ -d "$AUTH_DIR/env" ]; then + for file in "$AUTH_DIR/env"/*; do + [ -e "$file" ] || continue + case "${file##*/}" in + *_BASE_URL|*_HOST|AWS_DEFAULT_REGION) continue ;; + esac + echo "::add-mask::$(cat "$file")" + done +fi +[ ! -s "$AUTH_DIR/license.txt" ] || echo "::add-mask::$(cat "$AUTH_DIR/license.txt")" + +case "$tier" in + hosted) label="CodeBoarding's hosted free tier" ;; + license) label="a CodeBoarding plan, on CodeBoarding's hosted tier" ;; + byok) label="your own ${provider} key" ;; + byok+license) label="your own ${provider} key, with a CodeBoarding plan" ;; + *) label="${tier}" ;; +esac +echo "CodeBoarding is running on ${label}." +{ + echo "### CodeBoarding configuration" + echo + echo "| | |" + echo "|---|---|" + echo "| Tier | \`${tier}\` |" + echo "| Provider | \`${provider}\` |" +} >> "${GITHUB_STEP_SUMMARY:-/dev/null}" diff --git a/scripts/action/with-auth.sh b/scripts/action/with-auth.sh index f1dc636..1b4f1ad 100755 --- a/scripts/action/with-auth.sh +++ b/scripts/action/with-auth.sh @@ -1,10 +1,7 @@ #!/usr/bin/env bash -# Runs one command with scoped provider credentials, then removes those credentials. +# Runs one command with the resolved provider credentials, then removes them. set -euo pipefail AUTH_DIR="${RUNNER_TEMP}/codeboarding-auth" -PROVIDER_FILE="$AUTH_DIR/provider-name" -PROVIDER_ENV_FILE="$AUTH_DIR/provider-env" -PROVIDER_KEY_FILE="$AUTH_DIR/provider-key" cleanup() { if [ -s "$AUTH_DIR/relay.pid" ]; then kill "$(cat "$AUTH_DIR/relay.pid")" 2>/dev/null || true @@ -12,37 +9,30 @@ cleanup() { rm -rf "$AUTH_DIR" } trap cleanup EXIT -if [ ! -s "$PROVIDER_FILE" ] || [ ! -s "$PROVIDER_ENV_FILE" ]; then + +if [ ! -s "$AUTH_DIR/tier" ] || [ ! -s "$AUTH_DIR/provider-name" ]; then echo "::error::CodeBoarding analysis credentials are unavailable." exit 1 fi -PROVIDER="$(cat "$PROVIDER_FILE")" -case "$PROVIDER" in - ollama) [ -n "${OLLAMA_BASE_URL:-${OLLAMA_HOST:-}}" ] || { echo "::error::ollama requires OLLAMA_BASE_URL or OLLAMA_HOST." >&2; exit 1; } ;; - litellm) [ -n "${LITELLM_BASE_URL:-}" ] || { echo "::error::litellm requires LITELLM_BASE_URL." >&2; exit 1; } ;; -esac - -# Core selects a provider from its environment. Remove inherited selectors for -# every provider except the one explicitly requested by this action invocation. -selectors=(OPENAI_API_KEY OPENAI_BASE_URL VERCEL_API_KEY VERCEL_BASE_URL ANTHROPIC_API_KEY GOOGLE_API_KEY - AWS_BEARER_TOKEN_BEDROCK CEREBRAS_API_KEY OLLAMA_API_KEY OLLAMA_BASE_URL OLLAMA_HOST DEEPSEEK_API_KEY - DEEPSEEK_BASE_URL GLM_API_KEY GLM_BASE_URL KIMI_API_KEY KIMI_BASE_URL OPENROUTER_API_KEY LITELLM_API_KEY LITELLM_BASE_URL) -for selector in "${selectors[@]}"; do - case "$PROVIDER:$selector" in - openai:OPENAI_*|vercel:VERCEL_*|anthropic:ANTHROPIC_*|google:GOOGLE_*|aws:AWS_*|aws_bedrock:AWS_*|cerebras:CEREBRAS_*|ollama:OLLAMA_*|deepseek:DEEPSEEK_*|glm:GLM_*|kimi:KIMI_*|openrouter:OPENROUTER_*|litellm:LITELLM_*) ;; - *) unset "$selector" ;; - esac -done -PROVIDER_ENV="$(cat "$PROVIDER_ENV_FILE")" -if [ -s "$PROVIDER_KEY_FILE" ]; then - export "$PROVIDER_ENV=$(cat "$PROVIDER_KEY_FILE")" -fi -if [ -s "$AUTH_DIR/base-url" ]; then - OPENROUTER_BASE_URL="$(cat "$AUTH_DIR/base-url")" - export OPENROUTER_BASE_URL +# Core picks a provider from whatever its environment happens to hold, so a variable the +# caller exported for something else could select a provider this run never asked for. +# The list of what to strip is written by the resolver from the provider table, rather +# than kept here, so it cannot fall behind the pinned engine. +if [ -s "$AUTH_DIR/foreign-envs" ]; then + # `|| [ -n "$selector" ]` so a file whose last line is unterminated still strips that + # entry: `read` reports failure at EOF even when it read a partial line, and losing the + # last variable silently is exactly how a second provider stays configured. + while IFS= read -r selector || [ -n "$selector" ]; do + [ -z "$selector" ] || unset "$selector" + done < "$AUTH_DIR/foreign-envs" fi +for file in "$AUTH_DIR/env"/*; do + [ -e "$file" ] || continue + export "${file##*/}=$(cat "$file")" +done + export CODEBOARDING_SOURCE=github_action unset ACTIONS_ID_TOKEN_REQUEST_URL ACTIONS_ID_TOKEN_REQUEST_TOKEN if [ -n "${MODEL:-}" ]; then diff --git a/tests/test_action_auth.py b/tests/test_action_auth.py index 1caecc7..57f4909 100644 --- a/tests/test_action_auth.py +++ b/tests/test_action_auth.py @@ -1,4 +1,4 @@ -"""Tests for the composite action's provider credential boundary.""" +"""Tests for the composite action's credential boundary, as the runner exercises it.""" from __future__ import annotations @@ -9,101 +9,253 @@ import unittest from pathlib import Path - ROOT = Path(__file__).resolve().parent.parent +PREFLIGHT = ROOT / "scripts" / "action" / "verify-credentials.sh" CONFIGURE_AUTH = ROOT / "scripts" / "action" / "configure-auth.sh" WITH_AUTH = ROOT / "scripts" / "action" / "with-auth.sh" class ActionAuthTests(unittest.TestCase): - def _configure(self, provider: str, key: str, **extra_env: str) -> tuple[subprocess.CompletedProcess, Path]: + def setUp(self) -> None: + self.temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self.temp_dir.cleanup) + + def _preflight(self, **inputs: str) -> tuple[subprocess.CompletedProcess, Path, dict[str, str]]: temp_dir = Path(self.temp_dir.name) runner_temp = temp_dir / "runner" - runner_temp.mkdir() - output = temp_dir / "output" - github_env = temp_dir / "github-env" + runner_temp.mkdir(exist_ok=True) + output = temp_dir / "github-output" + output.write_text("", encoding="utf-8") env = { "PATH": os.environ["PATH"], "ACTION_PATH": str(ROOT), - "GITHUB_ENV": str(github_env), "GITHUB_OUTPUT": str(output), - "LICENSE_KEY": "", - "LLM_API_KEY": key, - "LLM_PROVIDER": provider, + "GITHUB_STEP_SUMMARY": str(temp_dir / "summary.md"), "RUNNER_TEMP": str(runner_temp), - **extra_env, + **inputs, } - result = subprocess.run( - [str(CONFIGURE_AUTH)], + result = subprocess.run([str(PREFLIGHT)], env=env, capture_output=True, text=True, check=False) + return result, runner_temp / "codeboarding-auth", self._outputs(output) + + @staticmethod + def _outputs(path: Path) -> dict[str, str]: + """Parse the runner's key=value and key< subprocess.CompletedProcess: + runner_temp = Path(self.temp_dir.name) / "runner" + env = {"PATH": os.environ["PATH"], "RUNNER_TEMP": str(runner_temp), **extra_env} + return subprocess.run( + [str(WITH_AUTH), "bash", "-c", script], env=env, capture_output=True, text=True, check=False, ) - return result, runner_temp / "codeboarding-auth" - def setUp(self) -> None: - self.temp_dir = tempfile.TemporaryDirectory() + # -- the guarantee ----------------------------------------------------- - def tearDown(self) -> None: - self.temp_dir.cleanup() + def test_a_named_provider_never_falls_back_to_codeboarding_credentials(self) -> None: + """PROTECTED TEST -- a workflow that names a provider runs on that provider or + not at all. - def test_maps_provider_keys_without_exporting_them_to_github_env(self) -> None: - cases = { - "openai": "OPENAI_API_KEY", - "vercel": "VERCEL_API_KEY", - "anthropic": "ANTHROPIC_API_KEY", - "google": "GOOGLE_API_KEY", - "aws": "AWS_BEARER_TOKEN_BEDROCK", - "aws_bedrock": "AWS_BEARER_TOKEN_BEDROCK", - "cerebras": "CEREBRAS_API_KEY", - "deepseek": "DEEPSEEK_API_KEY", - "glm": "GLM_API_KEY", - "kimi": "KIMI_API_KEY", - "openrouter": "OPENROUTER_API_KEY", - } - for provider, expected_env in cases.items(): - with self.subTest(provider=provider): - self.temp_dir.cleanup() - self.temp_dir = tempfile.TemporaryDirectory() - result, auth_dir = self._configure(provider, "fake-key") - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - self.assertEqual((auth_dir / "provider-env").read_text(), expected_env) - self.assertEqual((auth_dir / "provider-key").read_text(), "fake-key") - self.assertFalse((Path(self.temp_dir.name) / "github-env").exists()) - mode = stat.S_IMODE((auth_dir / "provider-key").stat().st_mode) - self.assertEqual(mode & 0o077, 0) - - def test_maps_standard_custom_provider_names_without_an_action_allowlist(self) -> None: - result, auth_dir = self._configure("acme-ai", "fake-key") + This action used to treat an empty key as "no preference" and resolve it to + CodeBoarding's hosted OpenRouter tier. A repository that asked for Anthropic and + had not added its secret yet therefore went green while running on a different + vendor, a different model, and CodeBoarding's money -- and nothing in the run + said so. Falling back is never the right answer to an unanswered question here: + the workflow named a provider, so the only honest outcomes are that provider or + a failure that says what is missing. + """ + result, auth_dir, outputs = self._preflight( + CB_IN_LLM="anthropic", + CB_IN_ANTHROPIC_API_KEY="", + ACTIONS_ID_TOKEN_REQUEST_URL="https://oidc.example/token", + ACTIONS_ID_TOKEN_REQUEST_TOKEN="request-token", + ) + + self.assertNotEqual(result.returncode, 0) + self.assertEqual(outputs["error"], "missing_provider_key") + self.assertFalse(auth_dir.exists(), "credentials were staged for a refused run") + self.assertNotIn("openrouter", result.stdout.lower()) + self.assertNotIn("OPENROUTER_API_KEY", result.stdout) + # And the analysis cannot proceed on whatever the environment happened to hold. + scoped = self._with_auth("true", OPENROUTER_API_KEY="inherited") + self.assertNotEqual(scoped.returncode, 0) + + # -- resolution -------------------------------------------------------- + + def test_provider_key_is_staged_privately_and_exported_for_analysis(self) -> None: + result, auth_dir, outputs = self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="fake=key") self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - self.assertEqual((auth_dir / "provider-env").read_text(), "ACME_AI_API_KEY") + self.assertEqual(outputs["tier"], "byok") + self.assertEqual(outputs["provider"], "anthropic") + self.assertEqual(outputs["error"], "") - def test_normalizes_pasted_provider_key_wrappers(self) -> None: - result, auth_dir = self._configure("openrouter", " 'OPENROUTER_API_KEY=\"fake-key\"' \n") + key_file = auth_dir / "env" / "ANTHROPIC_API_KEY" + self.assertEqual(key_file.read_text(encoding="utf-8"), "fake=key") + self.assertEqual(stat.S_IMODE(key_file.stat().st_mode) & 0o077, 0) + self.assertIn("::add-mask::fake=key", result.stdout) + scoped = self._with_auth( + 'test "$ANTHROPIC_API_KEY" = "fake=key" && test "$CODEBOARDING_SOURCE" = github_action' + ) + self.assertEqual(scoped.returncode, 0, scoped.stderr or scoped.stdout) + self.assertFalse(auth_dir.exists(), "credentials outlived the analysis command") + + def test_selectors_for_other_providers_are_stripped_from_the_analysis(self) -> None: + """Core picks a provider from its environment, so an inherited key must not vote.""" + result, _, _ = self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k") + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + + scoped = self._with_auth( + 'test -z "${OPENAI_API_KEY:-}" && test -z "${OPENAI_BASE_URL:-}" && ' + 'test -z "${OPENROUTER_API_KEY:-}" && test -z "${LITELLM_BASE_URL:-}" && ' + 'test -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}"', + OPENAI_API_KEY="inherited", + OPENAI_BASE_URL="https://inherited.example", + OPENROUTER_API_KEY="inherited", + LITELLM_BASE_URL="https://inherited.example", + ACTIONS_ID_TOKEN_REQUEST_URL="https://oidc.example/token", + ) + self.assertEqual(scoped.returncode, 0, scoped.stderr or scoped.stdout) + + def test_an_inherited_key_cannot_supply_a_provider_selected_by_its_endpoint(self) -> None: + """The same substitution as the protected test, one provider narrower. + + `llm: openai` with only an endpoint is a legitimate keyless configuration. Sparing + every variable OpenAI *could* use, rather than the ones this run actually resolved, + would leave a stray OPENAI_API_KEY from the job environment to credential the run. + """ + result, auth_dir, _ = self._preflight(CB_IN_LLM="openai", CB_IN_OPENAI_BASE_URL="https://proxy.example/v1") self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - self.assertEqual((auth_dir / "provider-key").read_text(), "fake-key") + self.assertIn("OPENAI_API_KEY", (auth_dir / "foreign-envs").read_text(encoding="utf-8")) + + scoped = self._with_auth( + 'test -z "${OPENAI_API_KEY:-}" && test "$OPENAI_BASE_URL" = https://proxy.example/v1', + OPENAI_API_KEY="inherited", + ) + self.assertEqual(scoped.returncode, 0, scoped.stderr or scoped.stdout) - def test_ollama_and_litellm_keys_use_core_environment_names(self) -> None: - for provider in ("ollama", "litellm"): - with self.subTest(provider=provider): - self.temp_dir.cleanup() - self.temp_dir = tempfile.TemporaryDirectory() - result, auth_dir = self._configure(provider, "fake-key") - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - self.assertEqual((auth_dir / "provider-env").read_text(), f"{provider.upper()}_API_KEY") - self.assertEqual((auth_dir / "provider-key").read_text(), "fake-key") + def test_a_provider_input_left_empty_is_stripped_not_inherited(self) -> None: + """An unset region means the engine's default, never whatever the job exported.""" + result, auth_dir, _ = self._preflight(CB_IN_LLM="aws_bedrock", CB_IN_AWS_BEDROCK_API_KEY="k") + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + self.assertIn("AWS_DEFAULT_REGION", (auth_dir / "foreign-envs").read_text(encoding="utf-8")) - def test_keyless_direct_provider_does_not_fall_back_to_hosted_openrouter(self) -> None: - result, auth_dir = self._configure("ollama", "") + def test_the_last_foreign_selector_is_stripped_like_every_other(self) -> None: + """The alphabetically last entry is the one a `while read` loop drops. + `"\\n".join(...)` left the file unterminated, so `read` failed at EOF and never ran + the body for that record. VERCEL_BASE_URL sorts last, so an Anthropic run inherited + it and core saw two providers configured. The earlier stripping test passed + throughout, because none of the variables it named was last. + """ + result, auth_dir, _ = self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k") self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - self.assertEqual((auth_dir / "provider-env").read_text(), "OLLAMA_API_KEY") - self.assertFalse((auth_dir / "provider-key").exists()) + listed = (auth_dir / "foreign-envs").read_text(encoding="utf-8") + self.assertTrue(listed.endswith("\n"), "an unterminated final record is silently skipped") + last = listed.strip().splitlines()[-1] + + scoped = self._with_auth(f'test -z "${{{last}:-}}"', **{last: "inherited"}) + self.assertEqual(scoped.returncode, 0, f"{last} survived into the analysis") + + def test_endpoint_and_region_changes_reach_the_reusable_analysis_name(self) -> None: + """Two runs that talk to different backends must not share a warm start.""" + secret = "sk-distinctive-value" + first, auth_dir, outputs = self._preflight( + CB_IN_LLM="openai", CB_IN_OPENAI_API_KEY=secret, CB_IN_OPENAI_BASE_URL="https://a.example/v1" + ) + self.assertEqual(first.returncode, 0, first.stderr or first.stdout) + recorded = (auth_dir / "backend-id").read_text(encoding="utf-8") + self.assertIn("https://a.example/v1", recorded) + self.assertNotIn(secret, recorded, "an artifact name must never be built from a key") + moved = outputs["backend_id"] - def test_hosted_auth_relays_to_aws_proxy_instead_of_openrouter(self) -> None: + self.temp_dir.cleanup() + self.temp_dir = tempfile.TemporaryDirectory() + _, _, other = self._preflight( + CB_IN_LLM="openai", CB_IN_OPENAI_API_KEY=secret, CB_IN_OPENAI_BASE_URL="https://b.example/v1" + ) + self.assertNotEqual(moved, other["backend_id"], "a different endpoint must not reuse analysis") + + def test_rotating_a_key_does_not_throw_away_reusable_analysis(self) -> None: + """The backend id names what the run talks to, never the secret it talks with.""" + _, _, first = self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="old-key") + self.temp_dir.cleanup() + self.temp_dir = tempfile.TemporaryDirectory() + _, _, second = self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="new-key") + self.assertEqual(first["backend_id"], second["backend_id"]) + + def test_model_inputs_keep_their_precedence(self) -> None: + self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k") + scoped = self._with_auth( + 'test "$AGENT_MODEL" = analysis-model && test "$PARSING_MODEL" = shared-model', + MODEL="shared-model", + AGENT_MODEL_INPUT="analysis-model", + PARSING_MODEL_INPUT="", + ) + self.assertEqual(scoped.returncode, 0, scoped.stderr or scoped.stdout) + + def test_refusal_reports_a_code_and_an_actionable_message(self) -> None: + result, _, outputs = self._preflight(CB_IN_LLM="hosted", CB_IN_OPENAI_API_KEY="k") + + self.assertNotEqual(result.returncode, 0) + self.assertEqual(outputs["error"], "hosted_with_provider_key") + self.assertIn("openai_api_key", outputs["message"]) + self.assertIn("::error title=CodeBoarding LLM configuration::", result.stdout) + # The annotation carries the one-line form; the summary carries the copyable one. + annotation = next(line for line in result.stdout.splitlines() if line.startswith("::error title=")) + self.assertNotIn("```", annotation) + summary = (Path(self.temp_dir.name) / "summary.md").read_text(encoding="utf-8") + self.assertIn("CodeBoarding could not start", summary) + # "Remove one of these" has two valid fixes, so it stays prose; the remedies that + # have one exact answer carry a snippet, which the next test covers. + self.assertIn("openai_api_key", outputs["details"]) + + def test_a_refusal_with_one_exact_fix_carries_it_into_the_summary(self) -> None: + result, _, outputs = self._preflight( + CB_IN_LLM="anthropic", + GITHUB_REPOSITORY="acme/widgets", + GITHUB_SERVER_URL="https://github.com", + ) + self.assertNotEqual(result.returncode, 0) + summary = (Path(self.temp_dir.name) / "summary.md").read_text(encoding="utf-8") + self.assertIn("```yaml", summary, "the summary should show the line to add") + self.assertIn("secrets/actions/new", summary, "and the page to click") + self.assertIn("anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}", outputs["details"]) + + def test_successful_run_reports_its_tier_and_provider(self) -> None: + """The webview reads this to show what a repository is actually running on.""" + result, _, outputs = self._preflight( + CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k", CB_IN_LICENSE_KEY="lic" + ) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + self.assertEqual(outputs["tier"], "byok+license") + summary = (Path(self.temp_dir.name) / "summary.md").read_text(encoding="utf-8") + self.assertIn("byok+license", summary) + self.assertIn("anthropic", summary) + + # -- hosted tiers ------------------------------------------------------ + + def test_hosted_auth_relays_to_the_codeboarding_proxy(self) -> None: temp_dir = Path(self.temp_dir.name) fake_bin = temp_dir / "bin" fake_bin.mkdir() @@ -124,99 +276,65 @@ def test_hosted_auth_relays_to_aws_proxy_instead_of_openrouter(self) -> None: ) fake_python.chmod(0o755) - result, auth_dir = self._configure( - "openrouter", - "", + result, auth_dir, outputs = self._preflight( + CB_IN_LLM="license", + CB_IN_LICENSE_KEY="a-license", ACTIONS_ID_TOKEN_REQUEST_URL="https://oidc.example/token", ACTIONS_ID_TOKEN_REQUEST_TOKEN="request-token", - CAPTURED_ARGS=str(captured_args), - PATH=f"{fake_bin}:{os.environ['PATH']}", ) - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - args = captured_args.read_text(encoding="utf-8").splitlines() - upstream_index = args.index("--upstream-base-url") + 1 - self.assertEqual( - args[upstream_index], - "https://auduihjmm4b735zci7vyabuikq0hppqn.lambda-url.us-east-1.on.aws", + self.assertEqual(outputs["tier"], "license") + self.assertIn("::add-mask::a-license", result.stdout) + + configured = subprocess.run( + [str(CONFIGURE_AUTH)], + env={ + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "ACTION_PATH": str(ROOT), + "RUNNER_TEMP": str(temp_dir / "runner"), + "CAPTURED_ARGS": str(captured_args), + "ACTIONS_ID_TOKEN_REQUEST_URL": "https://oidc.example/token", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN": "request-token", + }, + capture_output=True, + text=True, + check=False, ) - self.assertNotIn("https://openrouter.ai/api/v1", args) - self.assertEqual((auth_dir / "provider-key").read_text(), "github-actions-oidc-relay") + self.assertEqual(configured.returncode, 0, configured.stderr or configured.stdout) + args = captured_args.read_text(encoding="utf-8").splitlines() + upstream = args[args.index("--upstream-base-url") + 1] + self.assertEqual(upstream, "https://auduihjmm4b735zci7vyabuikq0hppqn.lambda-url.us-east-1.on.aws") + self.assertIn("--license-file", args) + self.assertEqual((auth_dir / "env" / "OPENROUTER_API_KEY").read_text(), "github-actions-oidc-relay") + self.assertEqual((auth_dir / "env" / "OPENROUTER_BASE_URL").read_text(), "http://127.0.0.1:12345") - def test_failed_relay_start_removes_credentials_and_process(self) -> None: + def test_direct_provider_runs_start_no_relay(self) -> None: temp_dir = Path(self.temp_dir.name) fake_bin = temp_dir / "bin" fake_bin.mkdir() - relay_pid = temp_dir / "relay-pid" - (fake_bin / "python3").write_text( - '#!/usr/bin/env bash\nprintf \'%s\' "$$" > "$RELAY_PID"\nexec /bin/sleep 30\n', - encoding="utf-8", - ) - (fake_bin / "sleep").write_text("#!/usr/bin/env bash\n/bin/sleep 0.01\n", encoding="utf-8") - for path in (fake_bin / "python3", fake_bin / "sleep"): - path.chmod(0o755) - - result, auth_dir = self._configure( - "openrouter", - "", - ACTIONS_ID_TOKEN_REQUEST_URL="https://oidc.example/token", - ACTIONS_ID_TOKEN_REQUEST_TOKEN="request-token", - LICENSE_KEY="license", - RELAY_PID=str(relay_pid), - PATH=f"{fake_bin}:{os.environ['PATH']}", - ) - - self.assertNotEqual(result.returncode, 0) - self.assertFalse(auth_dir.exists()) - with self.assertRaises(ProcessLookupError): - os.kill(int(relay_pid.read_text(encoding="utf-8")), 0) - - def test_with_auth_scopes_credentials_source_and_model_precedence(self) -> None: - result, auth_dir = self._configure("anthropic", "fake=key") - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - command = [ - str(WITH_AUTH), - "bash", - "-c", - 'test "$ANTHROPIC_API_KEY" = "fake=key" && ' - 'test -z "${OPENAI_API_KEY:-}" && ' - 'test -z "${OPENAI_BASE_URL:-}" && ' - 'test -z "${OPENROUTER_API_KEY:-}" && ' - 'test -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" && ' - 'test -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" && ' - 'test "$CODEBOARDING_SOURCE" = github_action && ' - 'test "$AGENT_MODEL" = analysis-model && ' - 'test "$PARSING_MODEL" = shared-model', - ] - env = { - "PATH": os.environ["PATH"], - "RUNNER_TEMP": str(auth_dir.parent), - "MODEL": "shared-model", - "AGENT_MODEL_INPUT": "analysis-model", - "PARSING_MODEL_INPUT": "", - "OPENAI_API_KEY": "inherited-key", - "OPENAI_BASE_URL": "https://inherited.example", - "ACTIONS_ID_TOKEN_REQUEST_URL": "https://oidc.example/token", - "ACTIONS_ID_TOKEN_REQUEST_TOKEN": "request-token", - } - scoped = subprocess.run(command, env=env, capture_output=True, text=True, check=False) - self.assertEqual(scoped.returncode, 0, scoped.stderr or scoped.stdout) - self.assertFalse(auth_dir.exists()) + marker = temp_dir / "relay-started" + (fake_bin / "python3").write_text(f'#!/usr/bin/env bash\ntouch "{marker}"\n', encoding="utf-8") + (fake_bin / "python3").chmod(0o755) - def test_keyless_provider_requires_its_endpoint(self) -> None: - result, auth_dir = self._configure("ollama", "") - self.assertEqual(result.returncode, 0, result.stderr or result.stdout) - - scoped = subprocess.run( - [str(WITH_AUTH), "true"], - env={"PATH": os.environ["PATH"], "RUNNER_TEMP": str(auth_dir.parent)}, + self._preflight(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k") + configured = subprocess.run( + [str(CONFIGURE_AUTH)], + env={ + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "ACTION_PATH": str(ROOT), + "RUNNER_TEMP": str(temp_dir / "runner"), + }, capture_output=True, text=True, check=False, ) + self.assertEqual(configured.returncode, 0, configured.stderr or configured.stdout) + self.assertFalse(marker.exists(), "a direct-provider run contacted the hosted relay") + def test_analysis_refuses_to_run_without_a_resolved_plan(self) -> None: + scoped = self._with_auth("true") self.assertNotEqual(scoped.returncode, 0) - self.assertIn("ollama requires OLLAMA_BASE_URL or OLLAMA_HOST", scoped.stderr) + self.assertIn("credentials are unavailable", scoped.stdout + scoped.stderr) if __name__ == "__main__": diff --git a/tests/test_action_inputs.py b/tests/test_action_inputs.py new file mode 100644 index 0000000..bf8aeef --- /dev/null +++ b/tests/test_action_inputs.py @@ -0,0 +1,104 @@ +"""action.yml must expose, and wire, exactly the contract the provider table describes. + +The resolver only ever sees what action.yml hands it. A provider in the table with no +input declared is unreachable; an input declared but not wired reads as empty and is +refused as "you did not set your key" when the user did. Both are silent, so they are +checked here rather than discovered in a repository. +""" + +from __future__ import annotations + +import json +import re +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +ACTION = (ROOT / "action.yml").read_text(encoding="utf-8") +TABLE = json.loads((ROOT / "scripts" / "action" / "supported-providers.json").read_text(encoding="utf-8")) + + +def declared_inputs() -> dict[str, str]: + """Input name -> its declaration block, read from action.yml's inputs section.""" + section = ACTION[ACTION.index("\ninputs:\n") : ACTION.index("\noutputs:\n")] + blocks = re.split(r"\n {2}(?=[a-z0-9_]+:\n)", section) + found = {} + for block in blocks: + match = re.match(r"\s*([a-z0-9_]+):\n", block) + if match: + found[match.group(1)] = block + return found + + +def table_inputs() -> set[str]: + return {i for p in TABLE["providers"].values() for i in p["inputs"]} + + +class ActionInputTests(unittest.TestCase): + def setUp(self) -> None: + self.inputs = declared_inputs() + + def test_every_provider_input_is_declared(self) -> None: + missing = sorted(table_inputs() - set(self.inputs)) + self.assertEqual(missing, [], f"provider inputs missing from action.yml: {missing}") + + def test_every_provider_input_is_wired_to_the_resolver(self) -> None: + for name in sorted(table_inputs()): + with self.subTest(input=name): + self.assertIn( + f"CB_IN_{name.upper()}: ${{{{ inputs.{name} }}}}", + ACTION, + f"{name} is declared but never reaches the resolver", + ) + + def test_no_declared_provider_input_is_absent_from_the_table(self) -> None: + """An input the table does not own can never be read, so it would mislead.""" + suffixes = ("_api_key", "_base_url", "_region") + declared = {n for n in self.inputs if n.endswith(suffixes) and n not in {"license_key", "github_token"}} + self.assertEqual(sorted(declared - table_inputs()), []) + + def test_llm_is_required_and_has_no_default(self) -> None: + block = self.inputs["llm"] + self.assertIn("required: true", block) + self.assertNotIn("default:", block) + + def test_the_inferred_credential_inputs_are_gone(self) -> None: + """`llm_api_key`/`llm_provider` are what made a fallback expressible at all.""" + for stale in ("llm_api_key", "llm_provider"): + self.assertNotIn(stale, self.inputs) + self.assertNotIn(f"inputs.{stale}", ACTION) + + def test_credentials_resolve_before_the_checkout_and_the_engine_install(self) -> None: + """Fail-fast is positional: preflight is worth little after a minute of setup.""" + preflight = ACTION.index("- name: Check LLM configuration") + for later in ("- name: Checkout analysis target", "- name: Install CodeBoarding"): + self.assertLess(preflight, ACTION.index(later), f"{later} runs before preflight") + + def test_the_generic_failure_comment_never_buries_the_actionable_one(self) -> None: + """Both write the same sticky comment, and the generic one runs on `failure()`. + + Without the guard, a run stopped for a missing secret posts the input and secret to + fix, then immediately replaces it with "see the workflow logs" -- sending the reader + to hunt for what they had just been told. + """ + start = ACTION.index("- name: Post review failure") + condition = ACTION[start : ACTION.index("message:", start)] + self.assertIn("steps.llm.outputs.error == ''", condition) + + def test_python_is_available_before_the_credential_check_runs(self) -> None: + """The check is a Python program, so a runner without a system python3 would fail a + configuration that is perfectly valid.""" + self.assertLess( + ACTION.index("- name: Setup Python"), + ACTION.index("- name: Check LLM configuration"), + ) + + def test_a_refused_run_reports_and_then_fails(self) -> None: + report = ACTION.index("- name: Report LLM configuration failure") + stop = ACTION.index("- name: Stop on LLM configuration failure") + self.assertLess(report, stop, "the run fails before it explains why") + self.assertIn("continue-on-error: true", ACTION[ACTION.index("id: llm") : report]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_llm_contract.py b/tests/test_llm_contract.py new file mode 100644 index 0000000..6f9eb86 --- /dev/null +++ b/tests/test_llm_contract.py @@ -0,0 +1,249 @@ +"""The credential contract: exactly one explicitly named source, or a named failure. + +Every case here is a workflow someone could plausibly write. The point of the table is +that each one either resolves to precisely what it asked for, or is refused with a code +and a message naming the input to fix -- never quietly resolved to something else. +""" + +from __future__ import annotations + +import importlib.util +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +_spec = importlib.util.spec_from_file_location("credential_check", ROOT / "scripts" / "action" / "credential_check.py") +credential_check = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(credential_check) + +OIDC = {"ACTIONS_ID_TOKEN_REQUEST_URL": "https://oidc.example/token"} + + +class ContractTests(unittest.TestCase): + def setUp(self) -> None: + self.table = credential_check.load_table() + + def resolve(self, **environ: str) -> dict: + return credential_check.resolve(self.table, environ) + + def refuse(self, **environ: str) -> credential_check.ConfigError: + with self.assertRaises(credential_check.ConfigError) as caught: + credential_check.resolve(self.table, environ) + return caught.exception + + # -- accepted shapes --------------------------------------------------- + + def test_every_provider_resolves_from_its_own_inputs(self) -> None: + """The table is the contract: each provider must be reachable through it.""" + for name, provider in self.table["providers"].items(): + with self.subTest(provider=name): + inputs = { + f"CB_IN_{i.upper()}": "value" + for i, var in provider["inputs"].items() + if var in provider["selection_envs"] + } + plan = self.resolve(CB_IN_LLM=name, **inputs) + self.assertEqual(plan["provider"], name) + self.assertEqual(plan["tier"], "byok") + self.assertTrue( + any(plan["env"].get(var) for var in provider["selection_envs"]), + f"{name} resolved without a variable core selects it by", + ) + + def test_hosted_and_license_are_named_not_inferred(self) -> None: + hosted = self.resolve(CB_IN_LLM="hosted", **OIDC) + self.assertEqual(hosted["tier"], "hosted") + self.assertEqual(hosted["env"], {}) + + licensed = self.resolve(CB_IN_LLM="license", CB_IN_LICENSE_KEY="lic", **OIDC) + self.assertEqual(licensed["tier"], "license") + self.assertEqual(licensed["license"], "lic") + + def test_licence_alongside_a_provider_key_is_recorded_not_refused(self) -> None: + """A CodeBoarding plan and your own tokens are two different questions.""" + plan = self.resolve(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k", CB_IN_LICENSE_KEY="lic") + self.assertEqual(plan["tier"], "byok+license") + self.assertEqual(plan["provider"], "anthropic") + + def test_one_spelling_per_provider_and_nothing_else(self) -> None: + """Casing and surrounding space are forgiven; a second spelling is not. + + Aliases look free and are not: each one is another value to document, test and keep + in step with the picker, and the refusal already lists what is accepted. + """ + for value in ("aws_bedrock", "AWS_BEDROCK", " aws_bedrock "): + with self.subTest(value=value): + plan = self.resolve(CB_IN_LLM=value, CB_IN_AWS_BEDROCK_API_KEY="k") + self.assertEqual(plan["provider"], "aws_bedrock") + for value in ("aws", "bedrock", "aws-bedrock", "gemini"): + with self.subTest(rejected=value): + error = self.refuse(CB_IN_LLM=value, CB_IN_AWS_BEDROCK_API_KEY="k") + self.assertEqual(error.code, "unknown_llm") + self.assertIn("aws_bedrock", error.message, "the refusal must name what to use") + + def test_each_providers_inputs_are_named_after_the_value_that_selects_it(self) -> None: + """`llm: X` always pairs with `X_api_key`, so the pairing never has to be looked up.""" + for name, provider in self.table["providers"].items(): + for input_name in provider["inputs"]: + with self.subTest(provider=name, input=input_name): + self.assertTrue(input_name.startswith(f"{name}_"), input_name) + + def test_the_table_carries_no_alias_map(self) -> None: + self.assertNotIn("aliases", self.table) + + def test_pasted_key_wrappers_are_stripped(self) -> None: + plan = self.resolve(CB_IN_LLM="openrouter", CB_IN_OPENROUTER_API_KEY=" 'OPENROUTER_API_KEY=\"sk-x\"' \n") + self.assertEqual(plan["env"]["OPENROUTER_API_KEY"], "sk-x") + + def test_endpoint_only_providers_resolve_without_a_key(self) -> None: + for name, endpoint in (("ollama", "OLLAMA_BASE_URL"), ("litellm", "LITELLM_BASE_URL")): + with self.subTest(provider=name): + plan = self.resolve(CB_IN_LLM=name, **{f"CB_IN_{name.upper()}_BASE_URL": "http://host:1234"}) + self.assertEqual(plan["env"][endpoint], "http://host:1234") + + # -- refused shapes ---------------------------------------------------- + + def test_an_undeclared_workflow_is_refused_rather_than_defaulted(self) -> None: + error = self.refuse() + self.assertEqual(error.code, "missing_llm") + self.assertIn("required", error.message) + + def test_a_named_provider_without_its_key_names_the_input_and_the_secret(self) -> None: + error = self.refuse(CB_IN_LLM="anthropic") + self.assertEqual(error.code, "missing_provider_key") + self.assertIn("anthropic_api_key", error.message) + self.assertIn("ANTHROPIC_API_KEY", error.message) + + def test_a_key_alone_does_not_configure_an_endpoint_selected_provider(self) -> None: + """Core selects ollama by its endpoint, so a key alone leaves it unusable.""" + for name in ("ollama", "litellm"): + with self.subTest(provider=name): + error = self.refuse(CB_IN_LLM=name, **{f"CB_IN_{name.upper()}_API_KEY": "k"}) + self.assertEqual(error.code, "missing_provider_key") + self.assertIn(f"{name}_base_url", error.message) + self.assertNotIn("repository secret", error.message) + + def test_a_second_providers_key_is_refused_rather_than_ignored(self) -> None: + error = self.refuse(CB_IN_LLM="anthropic", CB_IN_ANTHROPIC_API_KEY="k", CB_IN_OPENAI_API_KEY="other") + self.assertEqual(error.code, "foreign_provider_key") + self.assertIn("openai_api_key", error.message) + + def test_hosted_refuses_to_share_a_workflow_with_a_provider_key(self) -> None: + error = self.refuse(CB_IN_LLM="hosted", CB_IN_ANTHROPIC_API_KEY="k", **OIDC) + self.assertEqual(error.code, "hosted_with_provider_key") + self.assertIn("llm: anthropic", error.message) + + def test_license_refuses_to_share_a_workflow_with_a_provider_key(self) -> None: + """The mirror of the hosted case, and it shipped untested: `llm: license` runs on + CodeBoarding's credentials, so a provider key beside it asks for two things at once.""" + error = self.refuse(CB_IN_LLM="license", CB_IN_LICENSE_KEY="lic", CB_IN_ANTHROPIC_API_KEY="k", **OIDC) + self.assertEqual(error.code, "license_with_provider_key") + self.assertIn("anthropic_api_key", error.message) + self.assertIn("llm: anthropic", error.message) + + def test_hosted_refuses_a_licence_it_would_not_spend(self) -> None: + error = self.refuse(CB_IN_LLM="hosted", CB_IN_LICENSE_KEY="lic", **OIDC) + self.assertEqual(error.code, "hosted_with_license") + self.assertIn("llm: license", error.message) + + def test_license_without_a_licence_key_is_refused(self) -> None: + error = self.refuse(CB_IN_LLM="license", **OIDC) + self.assertEqual(error.code, "missing_license_key") + self.assertIn("CODEBOARDING_LICENSE", error.message) + + def test_hosted_tiers_require_the_oidc_permission(self) -> None: + for value, extra in (("hosted", {}), ("license", {"CB_IN_LICENSE_KEY": "lic"})): + with self.subTest(llm=value): + error = self.refuse(CB_IN_LLM=value, **extra) + self.assertEqual(error.code, "missing_id_token") + self.assertIn("id-token: write", error.message) + + def test_an_unknown_provider_lists_the_ones_that_exist(self) -> None: + error = self.refuse(CB_IN_LLM="claude") + self.assertEqual(error.code, "unknown_llm") + self.assertIn("anthropic", error.message) + + def test_a_remedy_links_the_secrets_page_and_shows_the_line_to_add(self) -> None: + """ "Add the secret and wire it" is a description of the fix, not the fix. + + The reader is someone who has never edited a workflow. They get the page to click + and the exact YAML, in the file it belongs in, indented as it will sit there. + """ + runner = { + "GITHUB_REPOSITORY": "acme/widgets", + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_WORKFLOW_REF": "acme/widgets/.github/workflows/codeboarding.yml@refs/pull/7/merge", + } + error = self.refuse(CB_IN_LLM="anthropic", **runner) + self.assertIn("https://github.com/acme/widgets/settings/secrets/actions/new", error.details) + self.assertIn(".github/workflows/codeboarding.yml", error.details) + self.assertIn("```yaml", error.details) + self.assertIn("anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}", error.details) + self.assertIn("llm: anthropic", error.details) + + def test_a_remedy_degrades_to_prose_off_a_runner(self) -> None: + """No GITHUB_REPOSITORY means no link to build, so say it without one.""" + error = self.refuse(CB_IN_LLM="anthropic") + self.assertNotIn("](", error.details, "a half-built link is worse than none") + self.assertIn("Add a repository secret", error.details) + self.assertIn("your CodeBoarding workflow", error.details) + + def test_the_licence_and_permission_remedies_are_copyable_too(self) -> None: + licence = self.refuse(CB_IN_LLM="license", GITHUB_REPOSITORY="acme/widgets", **OIDC) + self.assertIn("secrets/actions/new", licence.details) + self.assertIn("license_key: ${{ secrets.CODEBOARDING_LICENSE }}", licence.details) + + oidc = self.refuse(CB_IN_LLM="hosted") + self.assertIn("permissions:", oidc.details) + self.assertIn("id-token: write", oidc.details) + self.assertIn("```yaml", oidc.details) + + def test_the_annotation_stays_one_line_however_rich_the_remedy(self) -> None: + """`::error::` cannot carry newlines, so the two renderings must stay separate.""" + for environ in ( + {"CB_IN_LLM": "anthropic", "GITHUB_REPOSITORY": "acme/widgets"}, + {"CB_IN_LLM": "license", **OIDC}, + {"CB_IN_LLM": "hosted"}, + {}, + ): + with self.subTest(environ=environ): + error = self.refuse(**environ) + self.assertNotIn("\n", error.message) + + def test_every_declared_refusal_is_exercised_by_this_file(self) -> None: + """No refusal ships untested. Walks one configuration per code and asserts the set + it produces is exactly the set the module declares it can raise, so adding a code + without a case here fails rather than going unnoticed.""" + cases = [ + {}, + {"CB_IN_LLM": "not_a_provider"}, + {"CB_IN_LLM": "anthropic"}, + {"CB_IN_LLM": "license", **OIDC}, + {"CB_IN_LLM": "hosted"}, + {"CB_IN_LLM": "hosted", "CB_IN_ANTHROPIC_API_KEY": "k", **OIDC}, + {"CB_IN_LLM": "hosted", "CB_IN_LICENSE_KEY": "lic", **OIDC}, + {"CB_IN_LLM": "license", "CB_IN_LICENSE_KEY": "lic", "CB_IN_ANTHROPIC_API_KEY": "k", **OIDC}, + {"CB_IN_LLM": "anthropic", "CB_IN_ANTHROPIC_API_KEY": "k", "CB_IN_OPENAI_API_KEY": "o"}, + ] + seen = {self.refuse(**case).code for case in cases} + self.assertEqual(seen, set(credential_check.ERROR_CODES)) + + def test_every_refusal_carries_a_code_and_an_actionable_message(self) -> None: + cases = [ + {}, + {"CB_IN_LLM": "nope"}, + {"CB_IN_LLM": "anthropic"}, + {"CB_IN_LLM": "hosted", "CB_IN_LICENSE_KEY": "l", **OIDC}, + {"CB_IN_LLM": "license", **OIDC}, + {"CB_IN_LLM": "hosted"}, + ] + for environ in cases: + with self.subTest(environ=environ): + error = self.refuse(**environ) + self.assertTrue(error.code and error.code.islower()) + self.assertTrue(error.message.endswith(".")) + self.assertGreater(len(error.message), 40) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_provider_table_drift.py b/tests/test_provider_table_drift.py new file mode 100644 index 0000000..13d0a0b --- /dev/null +++ b/tests/test_provider_table_drift.py @@ -0,0 +1,111 @@ +"""The action's provider table must match the CodeBoarding release action.yml pins. + +The table exists because credentials have to be validated before the engine is installed, +which rules out asking the engine at run time. That copy is only safe while something +fails when it stops matching -- otherwise bumping the pin silently makes providers +unreachable (core added one, the table did not) or accepted here and broken later (core +removed one). This test is that something. It runs in the `core-compatibility` CI job, +which installs the pinned release; without the engine present it skips. +""" + +from __future__ import annotations + +import json +import re +import unittest +from importlib import metadata +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +TABLE = json.loads((ROOT / "scripts" / "action" / "supported-providers.json").read_text(encoding="utf-8")) + + +def _core_providers(): + """The pinned engine's provider table, or None when the engine is not installed. + + The distinction matters more than it looks. Skipping when the package is ABSENT is + what lets the stdlib-only suite run anywhere. Skipping when it is PRESENT but fails + to import would quietly retire this whole file the moment a pin moved a module or + dropped a dependency, which is exactly the bump this test exists to catch. So the + installed-ness question is asked of the distribution, and any import failure after + that is raised rather than swallowed. + """ + try: + metadata.version("codeboarding") + except metadata.PackageNotFoundError: + return None + from agents.llm_config import LLM_PROVIDERS + + return LLM_PROVIDERS + + +CORE_PROVIDERS = _core_providers() + + +def core_name(name: str) -> str: + """The engine's name for a provider we may expose under a friendlier one.""" + return TABLE["providers"][name].get("core", name) + + +@unittest.skipIf(CORE_PROVIDERS is None, "the pinned CodeBoarding release is not installed") +class ProviderTableDriftTests(unittest.TestCase): + def test_the_table_pins_the_release_action_yml_installs(self) -> None: + pinned = re.search(r"'codeboarding==([^']+)'", (ROOT / "action.yml").read_text()) + self.assertIsNotNone(pinned, "action.yml no longer pins a CodeBoarding release") + self.assertEqual( + TABLE["engine"], + pinned.group(1), + "supported-providers.json records a different release than action.yml installs", + ) + + def test_the_same_providers_exist_on_both_sides(self) -> None: + self.assertEqual( + sorted(core_name(n) for n in TABLE["providers"]), + sorted(CORE_PROVIDERS), + "the action and the pinned engine disagree about which providers exist", + ) + + def test_selection_variables_match_core_exactly(self) -> None: + """Selection is the whole rule: 'configured' means core would select it.""" + for name, provider in TABLE["providers"].items(): + with self.subTest(provider=name): + self.assertEqual( + sorted(provider["selection_envs"]), + sorted(CORE_PROVIDERS[core_name(name)].selection_envs), + f"{name}'s selection variables drifted from the engine", + ) + + def test_every_provider_key_reaches_the_variable_core_reads(self) -> None: + for name, provider in TABLE["providers"].items(): + api_key_env = CORE_PROVIDERS[core_name(name)].api_key_env + if api_key_env is None: + continue # e.g. aws, whose SDK reads its own bearer token variable + with self.subTest(provider=name): + self.assertIn( + api_key_env, + provider["inputs"].values(), + f"{name} has no input that sets {api_key_env}", + ) + + def test_no_selection_variable_escapes_the_table(self) -> None: + """with-auth.sh strips foreign selectors using this table; a variable missing + from it is one core could still be selected by.""" + core = {var for c in CORE_PROVIDERS.values() for var in c.selection_envs} + known = { + var for p in TABLE["providers"].values() for var in list(p["selection_envs"]) + list(p["inputs"].values()) + } + self.assertEqual(sorted(core - known), []) + + def test_every_renamed_provider_still_names_a_real_one(self) -> None: + """A `core` translation may only point at a provider the engine actually has.""" + for name, provider in TABLE["providers"].items(): + if "core" in provider: + with self.subTest(provider=name): + self.assertIn(provider["core"], CORE_PROVIDERS) + + def test_the_hosted_tier_names_a_real_provider(self) -> None: + self.assertIn(core_name(TABLE["hosted_provider"]), CORE_PROVIDERS) + + +if __name__ == "__main__": + unittest.main()