TEMPORARY: verify credential failure path (close me) - #105
Closed
Svilen-Stefanov wants to merge 6 commits into
Closed
TEMPORARY: verify credential failure path (close me)#105Svilen-Stefanov wants to merge 6 commits into
Svilen-Stefanov wants to merge 6 commits into
Conversation
…dentials The action resolved credentials by precedence: a provider key if present, a licence if present, otherwise CodeBoarding's hosted tier. An empty value was therefore indistinguishable from "no preference", so a repository that selected Anthropic and had not added its secret yet ran green on CodeBoarding's hosted OpenRouter tier -- a different vendor, a different model, our money -- and nothing in the run said so. Credentials are now named, not inferred. `llm` is required and takes `hosted`, `license`, or a provider name; each provider has its own `<name>_api_key` input. Anything ambiguous is refused: a named provider without its key, a hosted tier carrying a provider key, a licence where it would not be spent, a second provider's key. Refusals happen before the checkout and the engine install, and name the input and the secret to fix. A licence alongside a provider key stays valid and is reported as `byok+license`. Metering that combination needs proxy work and is not in scope. The provider table is mirrored from the pinned engine rather than hand-copied per site, and the foreign-selector list with-auth.sh strips is now derived from it. That list had already fallen behind: it was missing ORCAROUTER_API_KEY, which 0.13.10 added, so an inherited value could select a provider the workflow never asked for. A drift test installs the pinned release in CI and fails when the two disagree. Ships as feat: rather than feat!: on purpose -- see AGENTS.md. A major bump would freeze v1 and leave every existing workflow on the old silent-fallback behaviour permanently, which is the opposite of the intent. BREAKING: `llm_api_key` and `llm_provider` are replaced by `llm` plus per-provider inputs. Workflows that set neither must add `llm: hosted`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ence Four things the Codex review was right about: - The credential check is a Python program and ran before `setup-python`. A hosted runner ships a system python3 and would never notice; a self-hosted one without it would fail a valid configuration. Python is provisioned first now, which still leaves the check ahead of the engine install. - `Post review failure` writes the same sticky comment on `failure()`, so a run stopped for a missing secret posted the fix and then buried it under "see the workflow logs". It now stands down when the credential check is what failed. - The drift test treated any `agents.llm_config` import error as "engine not installed" and skipped, so a pin that moved a module would retire the very check meant to catch it. Installed-ness is asked of the distribution; an import failure after that is raised. - The foreign-variable list spared every variable the selected provider could use rather than the ones the run resolved. `llm: openai` with only `openai_base_url` therefore let an inherited OPENAI_API_KEY credential the run: the same silent substitution this contract removes, one provider narrower. Also switches this repo's own workflows from `llm: hosted` to `llm: license`. CodeBoarding's repositories run on the CodeBoarding plan, and the secret is already configured here. The README examples stay on `hosted`, which is the right starting point for someone reading them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/github-storage-scopes.md is an uncommitted local draft about Actions storage scopes. It has nothing to do with the credential contract and was swept in by a `git add -A`; it belongs wherever its author decides, not here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eaches Aliases were three extra values to document, test and keep in step with the picker, in exchange for accepting a spelling nobody was asked to type. The table is now keyed by the ONE value `llm:` accepts, each provider's inputs are named after it (`llm: X` always pairs with `X_api_key`), and an unrecognised value is refused with the accepted list. Bedrock keeps the friendlier `aws_bedrock` rather than the engine's internal `aws`; a `core` field carries that translation and nothing else does, so the drift test still compares against the engine name exactly. Also removed: the `hosted` field the resolver emitted and nothing read, and the OIDC check in configure-auth.sh, which preflight had already made unreachable. One decision point was the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`resolve_llm` described half the job. The module also writes every sentence the user reads about a credential problem: preflight puts the message in the step output, and action.yml posts that same string as the pull request comment, the error annotation and the job summary. Naming it `llm_credentials` and saying so in the docstring keeps the rule and its explanation together, which is the point of having them in one file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Temporary. Names a provider with no key so the run must fail fast and say which secret to add.
Contributor
CodeBoarding review - not configured
No analysis ran, and no CodeBoarding hosted usage was consumed. run 33087153970 - attempt 1 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d03c51978
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # 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 |
There was a problem hiding this comment.
Every eligible run of .github/workflows/codeboarding.yml now selects llm: anthropic without passing anthropic_api_key. The new preflight resolver rejects exactly this configuration with missing_provider_key, so automatic reviews and /codeboarding commands stop before checkout or analysis and never produce the architecture diagram. Restore the licensed configuration or wire the Anthropic secret before retaining this workflow change.
AGENTS.md reference: AGENTS.md:L51-L56
Useful? React with 👍 / 👎.
| | `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_region` | both | empty | Bedrock region. Core defaults to `us-east-1`. | |
There was a problem hiding this comment.
When a Bedrock user follows this input reference, aws_region is not a declared action input; action.yml declares aws_bedrock_region. GitHub therefore ignores the documented value, leaving Core to use its default region and potentially failing or targeting the wrong region. Change this row to aws_bedrock_region, consistent with the provider table above.
Useful? React with 👍 / 👎.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Throwaway PR to verify the failure path end to end on a real runner: the workflow names
llm: anthropicwith no key, so the run must fail in its first seconds and post the secret to add. Closing as soon as I have read the result.