fix(acp): allow configured provider auth - #934
Conversation
🦋 Changeset detectedLatest commit: 9c1b97f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This feature will be so useful for me, eagerly waiting for it to be done! Good luck with the PR, hope it gets merged soon✨ |
|
This would also be very good for my use case, is this still being considered? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c1b97f0fc
ℹ️ 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".
| } | ||
|
|
||
| function hasProviderValue(provider: ProviderConfig, envKey: string): boolean { | ||
| return nonEmptyString(provider.apiKey) !== undefined || hasEnvValue(provider, envKey); |
There was a problem hiding this comment.
Honor shell API-key env vars in the auth gate
When a user configures a default model/provider but supplies credentials via the ACP process environment (for example OPENAI_API_KEY, KIMI_API_KEY, or GOOGLE_API_KEY) instead of config.toml api_key/env, this check still returns unauthenticated because it only reads the parsed config. The actual runtime providers fall back to process.env for these keys (e.g. packages/kosong/src/providers/openai-legacy.ts:454), so session/new and authenticate can return auth_required before createSession gets a chance to use credentials that would work in the normal CLI path.
Useful? React with 👍 / 👎.
Related Issue
Resolve #799
Problem
kimi acponly treated OAuth tokens as authenticated. A user with a configured default model/provider and API-key credentials could use the normal CLI, but ACP still returnedauth_requiredand asked for terminal login.What changed
api_key+oauth, and keeping invalid configs behindauth_requiredValidation
pnpm --filter @moonshot-ai/acp-adapter exec vitest run test/auth-gate.test.tspnpm --filter @moonshot-ai/acp-adapter run testpnpm --filter @moonshot-ai/acp-adapter run typecheckpnpm --filter @moonshot-ai/acp-adapter run buildpnpm --filter @moonshot-ai/kimi-code run typecheckpnpm exec oxlint --type-aware packages/acp-adapter/src/server.ts packages/acp-adapter/test/auth-gate.test.ts --quietgit diff --checkReview
Ran read-only subagent review twice. First pass found two auth-gate edge cases around identifier trimming and mixed OAuth/API-key config; both are fixed. Second pass found no blockers. Ran a separate public-info scan; no sensitive/internal data found.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.