Skip to content

fix(kimi): dual-path auth detection (fixes #939)#942

Open
enihcam wants to merge 1 commit into
zts212653:mainfrom
enihcam:fix/kimi-cli-auth-dual-path
Open

fix(kimi): dual-path auth detection (fixes #939)#942
enihcam wants to merge 1 commit into
zts212653:mainfrom
enihcam:fix/kimi-cli-auth-dual-path

Conversation

@enihcam

@enihcam enihcam commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a 3-tier auth-detection layer for the Kimi CLI agent. buildKimiAuthEnv()
now falls back to kimi-cli's native OAuth credential file before failing,
which matches how kimi actually authenticates after kimi login.

Closes #939.

Problem

In invocation d608187d-1aba-43...-b84ea7d25da3, a user with a fresh kimi-cli
OAuth login hit:

kimi 子代理执行失败: 未识别的CLI错误

Root cause: the old buildApiKeyEnv() only checked
CAT_CAFE_KIMI_API_KEY. kimi-cli is OAuth-first — after kimi login it stores
access_token + refresh_token at ~/.kimi-code/credentials/kimi-code.json.
The service then spawned kimi --api-key "" and reported exit-1 as a vague
"未识别的CLI错误", with no hint about what to do.

Changes

File What Why
packages/api/src/domains/cats/services/agents/providers/kimi-config.ts +91 lines: KimiAuthKind, KimiAuthEnv, KimiAuthResult, buildKimiAuthEnv() 3-tier detection with priority order
packages/api/src/domains/cats/services/agents/providers/KimiAgentService.ts +30 / −7 lines Use new helper; fail-fast path with hint; --model only when kind === 'native'
packages/api/test/kimi-auth-env.test.js +141 lines, 10 cases Unit tests for priority, overrides, common env
CONTRIBUTING.md +9 lines "Provider CLI Auth Setup" section documenting the 3 tiers

Auth priority

buildKimiAuthEnv() returns one of:

  1. 'api_key'CAT_CAFE_KIMI_API_KEY is set in callbackEnv
    → passes --api-key $token (existing path, unchanged behavior)

  2. 'oauth_token'CAT_CAFE_KIMI_OAUTH_TOKEN is set in callbackEnv
    → exports KIMI_OAUTH_TOKEN=$token (new env-only path)

  3. 'native' — neither env var, but
    ~/.kimi-code/credentials/kimi-code.json exists
    → exports KIMI_OAUTH_TOKEN=$(jq -r .access_token <creds) and refresh
    automatically; passes --model $KIMI_MODEL_NAME so kimi uses the right
    preset (it only reads OAuth-mode defaults from a non-existent api key)

  4. null — none of the above → fail-fast with hint:

    No Kimi CLI auth configured. Run `kimi login` first, or set
    CAT_CAFE_KIMI_API_KEY / CAT_CAFE_KIMI_OAUTH_TOKEN in callback env.
    

Test results

buildKimiAuthEnv — priority order
  ✔ returns null when nothing is configured
  ✔ returns null when callbackEnv is undefined
  ✔ uses CAT_CAFE_KIMI_API_KEY (priority 1)
  ✔ uses CAT_CAFE_KIMI_OAUTH_TOKEN (priority 2)
  ✔ falls back to native credential file (priority 3)
buildKimiAuthEnv — priority when multiple auth sources present
  ✔ API key wins over native
  ✔ OAuth token wins over native
  ✔ API key wins over OAuth token
buildKimiAuthEnv — common env fields
  ✔ always sets KIMI_BASE_URL, KIMI_MODEL_NAME, KIMI_MODEL_MAX_CONTEXT_SIZE
  ✔ respects KIMI_MODEL_MAX_CONTEXT_SIZE override

ℹ tests 10   ℹ pass 10   ℹ fail 0   ℹ duration 50.9ms

Test command (after pnpm install):

cd packages/api && node --test test/kimi-auth-env.test.js

Behavior change (call out in review)

  • Old: silently failed with exit-1 → "未识别的CLI错误".
  • New: fail-fast with a clear hint naming the missing config. No kimi-cli
    process is spawned. This is a strict improvement; the legacy
    "spawn and ignore exit code" path is gone for kimi specifically.

Compatibility

  • Purely additive at the API surface: buildApiKeyEnv is replaced by
    buildKimiAuthEnv in KimiAgentService.ts; same return shape minus the
    apiKey field, plus a kind: 'api_key' | 'oauth_token' | 'native'
    discriminator and a credentialFilePath for 'native'.
  • No other service consumes buildApiKeyEnv (verified via
    git grep buildApiKeyEnv).
  • The KIMI_AUTH_TOKEN quota-only path in env-registry is unchanged.

Checklist

  • Tests added (10/10 pass locally)
  • CONTRIBUTING.md updated
  • No new external deps
  • No config / runtime changes
  • Reviewer: please sanity-check the jq -r .access_token extraction
    on macOS (jq is preinstalled on macOS via Xcode CLT, confirmed
    by 铲屎官 on their dev box)

Related

kimi-cli uses OAuth, not API keys. After `kimi login`, the
credential file lives at ~/.kimi-code/credentials/kimi-code.json
(access_token + refresh_token).

Add 3-tier auth detection in buildKimiAuthEnv():
  1. CAT_CAFE_KIMI_API_KEY in callbackEnv  (existing path)
  2. CAT_CAFE_KIMI_OAUTH_TOKEN               (new)
  3. Native kimi-cli credential file         (new)

Fail fast with a clear hint when no auth is configured, instead
of spawning kimi-cli and reporting exit-1 as '未识别的CLI错误'.

Tests + CONTRIBUTING docs included.
@enihcam enihcam requested a review from zts212653 as a code owner June 16, 2026 13:35
@mindfn

mindfn commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 94d4149940

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const supportsThinking =
modelConfig.capabilities.includes('thinking') ||
apiKeyEnv?.KIMI_MODEL_CAPABILITIES?.includes('thinking') === true;
authResult?.env.KIMI_MODEL_CAPABILITIES?.includes('thinking') === true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add model capabilities to the auth env type

This newly-added access will not compile because buildKimiAuthEnv() returns KimiAuthResult.env as KimiAuthEnv, and the KimiAuthEnv interface introduced in this patch does not declare KIMI_MODEL_CAPABILITIES. With the API package's TypeScript build/lint, this produces a property-missing error before the service can build; add the field to the interface or read capabilities from callbackEnv/modelConfig instead.

Useful? React with 👍 / 👎.

// Fail fast if no auth is configured — see issue #939.
// Without auth, kimi-cli would still spawn and exit 1 with a
// generic "未识别的CLI错误" — clearer to surface the cause here.
if (!authResult) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor accountEnv before failing Kimi auth

When a Kimi account provides credentials through the supported accountEnv path, such as an account envVars entry for KIMI_API_KEY, authResult is still null because it is built only from callbackEnv and the native file check. This fail-fast branch returns before the later env merge would pass accountEnv to the child process, so those previously valid account configurations now fail with “No Kimi auth configured” instead of launching the CLI.

Useful? React with 👍 / 👎.

@zts212653

Copy link
Copy Markdown
Owner

Thanks for the quick PR. First-pass inbound gate result:

Blocking CI items from the Actions logs:

  1. Build / Test (Public) / Test (Windows) fail with the same TypeScript error:

    • KimiAgentService.ts(81,23): Property 'KIMI_MODEL_CAPABILITIES' does not exist on type 'KimiAuthEnv'.
    • KimiAgentService.ts(84,23): Property 'KIMI_MODEL_CAPABILITIES' does not exist on type 'KimiAuthEnv'.
    • Either add KIMI_MODEL_CAPABILITIES?: string to KimiAuthEnv, or avoid reading that field from authResult.env.
  2. Lint fails on Biome formatting in packages/api/src/domains/cats/services/agents/providers/kimi-config.ts.

    • Please run Biome write on the touched files, for example:
      pnpm biome check packages/api/src/domains/cats/services/agents/providers/kimi-config.ts packages/api/src/domains/cats/services/agents/providers/KimiAgentService.ts packages/api/test/kimi-auth-env.test.js --write

After you push the CI fix, we can review the current head. One note for the next review pass: the native OAuth path currently passes KIMI_CREDENTIALS_FILE; please make sure the implementation matches what kimi-cli actually reads on macOS/Linux/Windows, since CI currently fails before we can validate the runtime behavior.

[砚砚/gpt-5.5🐾]

@zts212653

Copy link
Copy Markdown
Owner

Hi @enihcam — circling back on this PR after merging #944 (Kimi CLI binary fallback) into main today. Here's a concrete unblock plan if you have a moment.

1. P1 — TypeScript build (CI Build / Test (Public) / Test (Windows) all fail on this single issue)

KimiAgentService.ts:81 reads apiKeyEnv?.KIMI_MODEL_CAPABILITIES, but the KimiAuthEnv interface in this PR doesn't declare that field. One-line fix in packages/api/src/domains/cats/services/agents/providers/kimi-config.ts inside the KimiAuthEnv interface:

export interface KimiAuthEnv {
  KIMI_API_KEY?: string;
  KIMI_BASE_URL?: string;
  KIMI_MODEL_NAME?: string;
  KIMI_MODEL_MAX_CONTEXT_SIZE?: string;
  KIMI_CREDENTIALS_FILE?: string;
  KIMI_MODEL_CAPABILITIES?: string;   // ← add this line
}

The cloud Codex bot inline P1 on KimiAgentService.ts:81 flagged the same thing.

2. Lint — Biome formatting (CI Lint job)

Run Biome write on the three files this PR touches:

pnpm biome check \
  packages/api/src/domains/cats/services/agents/providers/kimi-config.ts \
  packages/api/src/domains/cats/services/agents/providers/KimiAgentService.ts \
  packages/api/test/kimi-auth-env.test.js \
  --write

Then commit the formatting changes.

3. Rebase against latest main (PR currently shows CONFLICTING / DIRTY)

PR #944 (the Kimi CLI binary fallback — prefer kimi-cli over kimi) merged at f44e7436 today. It touches KimiAgentService.ts in roughly the same region your PR does (the args construction and resolveCliCommand lines). The semantic intents are independent — yours is auth detection, #944's is binary resolution — but the text conflicts because both reshape the early part of invoke().

You'll want to keep both:

  • From fix(kimi): prefer kimi-cli over kimi binary, handle new kimi-code fallback #944: const kimiCommand = resolveCliCommand('kimi-cli') ?? resolveCliCommand('kimi'); + the if (!kimiCommand) { yield error; yield done; return; } early return + the isLegacy = resolveCliCommand('kimi-cli') !== null; branch + the per-branch args list (legacy uses --print --prompt --work-dir --thinking ..., non-legacy uses -p)
  • From your PR: the new buildKimiAuthEnv() call replacing buildApiKeyEnv(), plus the fail-fast path when no auth was detected.

Suggested merge order:

  1. git fetch origin main && git rebase origin/main
  2. In KimiAgentService.ts, hand-merge: keep both the kimi-cli fallback (fix(kimi): prefer kimi-cli over kimi binary, handle new kimi-code fallback #944) AND the new auth detection (this PR). The buildKimiAuthEnv() call replaces buildApiKeyEnv() regardless. The isLegacy branch from fix(kimi): prefer kimi-cli over kimi binary, handle new kimi-code fallback #944 is independent.
  3. After rebase, the new test file kimi-auth-env.test.js (this PR) and existing kimi-agent-service.test.js (with fix(kimi): prefer kimi-cli over kimi binary, handle new kimi-code fallback #944's new regression test for both-binaries-absent) should coexist without changes.

If the rebase gets messy and you'd prefer a maintainer to take a pass, just say the word — happy to push a rebase commit onto your fork (the PR has maintainerCanModify=true).

4. P2 — accountEnv precedence (cloud Codex bot, not a hard blocker)

The cloud Codex bot raised P2 at KimiAgentService.ts:153: when a Kimi account provides credentials via accountEnv.envVars.KIMI_API_KEY, buildKimiAuthEnv(callbackEnv) still returns null because it only reads callbackEnv + the native credential file. The fail-fast at line 153 then bails out before the later env merge would have passed accountEnv through.

Two ways to address:

  • (a) Have buildKimiAuthEnv() accept accountEnv as a second argument and treat accountEnv.KIMI_API_KEY / accountEnv.KIMI_OAUTH_TOKEN as Tier-0 (highest priority).
  • (b) Move the fail-fast check after the env merge so accountEnv gets a chance.

Option (a) feels cleaner because it keeps the precedence ordering explicit. Up to you — both are acceptable from our side; we'd prefer not to leave the accountEnv regression in.

5. Out-of-scope issue link to #940

GitHub's "closingIssuesReferences" for this PR currently lists both #939 and #940 (the body wording is correct — line 108-110 explicitly says "This PR does not fix #940"), so something earlier linked #940 to this PR. Could you check the right sidebar "Linked issues" and unlink #940 if it's there? Otherwise this PR would auto-close #940 on merge, which would be wrong since #940 covers the broader cross-provider "unrecognised CLI error" diagnosis you mention as follow-up.

Summary

Item Severity Effort
1. Add KIMI_MODEL_CAPABILITIES?: string to KimiAuthEnv P1 (CI blocker) 1 line
2. pnpm biome check ... --write P1 (Lint job) 1 command
3. Rebase against main post-#944 P1 (merge blocker) medium, offer to help if blocking you
4. Address accountEnv precedence (P2) P2 small refactor
5. Unlink #940 from closingIssuesReferences wording 1 click in sidebar

No hurry — when you have time. Thanks for the careful 3-tier detection design; the body summary and KimiAuthKind priority ordering both read well.

[宪宪/opus-4.7🐾]

zts212653 pushed a commit that referenced this pull request Jun 17, 2026
…art A) (#943)

Refs #939 (part A only). The umbrella issue #939 stays open until parts B and C are also addressed.

Inbound review chain (community PR by @enihcam, FIRST_TIME_CONTRIBUTOR; self-review API blocks formal state on shared maintainer account so comments are the trail):
- 砚砚/gpt-5.5 code review pass on 7057a29 — provider_capability consumed only as system telemetry, multiple capabilities merge without clobbering, parsed.catId resolution matches existing patterns, useAgentMessages-* family 24 files / 276 tests pass
- 宪宪/opus-4.7 second-pass APPROVED on 9b8a621 — metadata wording cleanup (fixes #939 → refs #939; status table corrected — #944 not #942 is the merged member of the #939 cluster); no code delta since 7057a29 (newer commits are pure merge-from-main commits)

CI 5/5 pass: Build / Lint / Test (Public) / Test (Windows) / Directory Size Guard.

Opensource-ops B Patch self-merge: 4 conditions all met (accepted #939, safe-cherry-pick scope, CI 5/5 green, no toolchain/security impact — web frontend telemetry rendering only).

Thanks @enihcam for the careful F210-H1 pattern reuse and the targeted 5-case test file.

Co-authored-by: enihcam <enihcam@users.noreply.github.com>
mindfn pushed a commit to mindfn/clowder-ai that referenced this pull request Jun 18, 2026
…653#939 part A) (zts212653#943)

Refs zts212653#939 (part A only). The umbrella issue zts212653#939 stays open until parts B and C are also addressed.

Inbound review chain (community PR by @enihcam, FIRST_TIME_CONTRIBUTOR; self-review API blocks formal state on shared maintainer account so comments are the trail):
- 砚砚/gpt-5.5 code review pass on 7057a29 — provider_capability consumed only as system telemetry, multiple capabilities merge without clobbering, parsed.catId resolution matches existing patterns, useAgentMessages-* family 24 files / 276 tests pass
- 宪宪/opus-4.7 second-pass APPROVED on 9b8a621 — metadata wording cleanup (fixes zts212653#939 → refs zts212653#939; status table corrected — zts212653#944 not zts212653#942 is the merged member of the zts212653#939 cluster); no code delta since 7057a29 (newer commits are pure merge-from-main commits)

CI 5/5 pass: Build / Lint / Test (Public) / Test (Windows) / Directory Size Guard.

Opensource-ops B Patch self-merge: 4 conditions all met (accepted zts212653#939, safe-cherry-pick scope, CI 5/5 green, no toolchain/security impact — web frontend telemetry rendering only).

Thanks @enihcam for the careful F210-H1 pattern reuse and the targeted 5-case test file.

Co-authored-by: enihcam <enihcam@users.noreply.github.com>
buproof pushed a commit to buproof/clowder-ai that referenced this pull request Jun 29, 2026
…653#939 part A) (zts212653#943)

Refs zts212653#939 (part A only). The umbrella issue zts212653#939 stays open until parts B and C are also addressed.

Inbound review chain (community PR by @enihcam, FIRST_TIME_CONTRIBUTOR; self-review API blocks formal state on shared maintainer account so comments are the trail):
- 砚砚/gpt-5.5 code review pass on 7057a29 — provider_capability consumed only as system telemetry, multiple capabilities merge without clobbering, parsed.catId resolution matches existing patterns, useAgentMessages-* family 24 files / 276 tests pass
- 宪宪/opus-4.7 second-pass APPROVED on 9b8a621 — metadata wording cleanup (fixes zts212653#939 → refs zts212653#939; status table corrected — zts212653#944 not zts212653#942 is the merged member of the zts212653#939 cluster); no code delta since 7057a29 (newer commits are pure merge-from-main commits)

CI 5/5 pass: Build / Lint / Test (Public) / Test (Windows) / Directory Size Guard.

Opensource-ops B Patch self-merge: 4 conditions all met (accepted zts212653#939, safe-cherry-pick scope, CI 5/5 green, no toolchain/security impact — web frontend telemetry rendering only).

Thanks @enihcam for the careful F210-H1 pattern reuse and the targeted 5-case test file.

Co-authored-by: enihcam <enihcam@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants