-
Notifications
You must be signed in to change notification settings - Fork 2
feat: require an explicit llm input and never fall back to hosted credentials #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
d4ae9e9
feat: require an explicit llm input and never fall back to hosted cre…
Svilen-Stefanov 4c33369
fix(review): address review feedback and run our own repos on the lic…
Svilen-Stefanov a14cd8d
chore: drop an unrelated working note from this branch
Svilen-Stefanov 2bad192
refactor: one accepted spelling per provider, and drop code nothing r…
Svilen-Stefanov 45e5aa8
refactor: name the credential module for what it owns
Svilen-Stefanov c39d5b0
fix(review): strip the last foreign selector, and hash the backend
Svilen-Stefanov f0cc70f
test: declare the refusal codes, and cover the one that shipped untested
Svilen-Stefanov e4c801f
refactor: name the credential files for what they do
Svilen-Stefanov e3f476a
feat(review): give a refused run the page to click and the line to copy
Svilen-Stefanov 17c0e8c
feat(review): say which credential actually pays, and stop when the c…
Svilen-Stefanov 0b8f29f
feat(review): put the review on the run page as well as the pull request
Svilen-Stefanov c668bf8
fix(review): require both OIDC variables, and stop claiming a key tha…
Svilen-Stefanov 107f35b
fix(review): do not tell hosted users which upstream we route them to
Svilen-Stefanov 4a5209b
fix(review): stop corrupting keys that contain '=', and never publish…
Svilen-Stefanov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,9 @@ jobs: | |
| runs-on: ubuntu-latest | ||
| timeout-minutes: 60 | ||
| steps: | ||
| - uses: CodeBoarding/CodeBoarding-action@v2 | ||
| - uses: CodeBoarding/CodeBoarding-action@v1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is really good catch, there is no v2 💀 |
||
| 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,96 @@ 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". **Your key always wins.** A direct provider call never reaches | ||
| CodeBoarding, so the licence is recorded and reported but not spent, and nothing meters | ||
| that combination today. The job summary says so on every run, rather than leaving you to | ||
| infer it from the tier name. | ||
|
|
||
| ### 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` (empty | ||
| on the hosted tiers, where the upstream is ours rather than yours), 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 +258,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 +279,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 +296,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. | | ||
| | `<provider>_api_key` | both | empty | That provider's key, e.g. `anthropic_api_key`. See [Providers](#providers). | | ||
| | `<provider>_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 +310,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. Empty on `hosted` and `license`: which upstream the proxy routes to is CodeBoarding's decision, not your configuration. | | ||
| | `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. | | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine no need to change it. But tbh it sounds like it should be "provider"
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use provider often for the LLM provider (Anthropic etc.) so provider is probably not the best word either but maybe tier? Nvm, tier doesn't quite work either. I'll keep it for now.