Skip to content

feat: add ClinePass provider - #81

Open
Daltonganger wants to merge 2 commits into
duolahypercho:mainfrom
Daltonganger:feat/clinepass-provider
Open

feat: add ClinePass provider#81
Daltonganger wants to merge 2 commits into
duolahypercho:mainfrom
Daltonganger:feat/clinepass-provider

Conversation

@Daltonganger

Copy link
Copy Markdown

Summary

  • add ClinePass as an API-key provider with its documented OpenAI-compatible endpoint and model catalog
  • normalize ClinePass requests by omitting undocumented reasoning_effort, thinking, and top_p fields while preserving streaming and tool calls
  • expose the provider in desktop onboarding and document its key handling

Testing

  • npm run check
  • node --test test/registry.test.mjs test/routing.test.mjs test/provider-credentials.test.mjs test/provider-onboarding.test.mjs
  • cargo test accepts_only_known_provider_ids
  • macOS Swift build

Notes

No authenticated live ClinePass request was run: it requires a user-owned API key and can consume quota.

@duolahypercho

Copy link
Copy Markdown
Owner

Reviewed — the scope is clean and this is close to merge-ready. The provider entry follows the registry pattern, the requestProfile branch in normalizeBody() mirrors the existing kimi-k3 handling, and the routing test exercises streaming and tool calls against a mock upstream, which is the right coverage given that a live test needs a user-owned key.

One blocker: the branch is CONFLICTING with current main. The Command Code provider family landed in b66a93d and touched the same surfaces this PR extends — config/providers.json, apps/desktop/src-tauri/src/main.rs, the tray's provider short-name table, AGENTS.md, SECURITY.md, and the registry/routing tests. I count 8 conflicted regions.

Could you rebase onto main? Two things to watch while resolving:

  1. test/registry.test.mjs — the provider-count and known-ID assertions now include the Command Code family, so the ClinePass additions need to layer on top of those, not replace them.
  2. The tray's shortName(forRegistryProvider:) switch gained Command Code cases; keep both sets.

After the rebase, npm run check + the four test files you already ran + cargo test accepts_only_known_provider_ids should confirm the resolution. Happy to merge once it's green.

@Daltonganger
Daltonganger force-pushed the feat/clinepass-provider branch from f552c3a to 7f61445 Compare August 7, 2026 19:04
@Daltonganger

Copy link
Copy Markdown
Author

Rebased onto current main and resolved the Command Code conflicts while preserving both Command Code and ClinePass behavior. Also added official ClinePass qwen3.8-max.

Validated with:

  • git diff --check
  • npm run check
  • ClinePass registry/routing/credential/onboarding tests: 47 passing
  • cargo test accepts_only_known_provider_ids
  • macOS swift build

@Daltonganger

Copy link
Copy Markdown
Author

Follow-up: ClinePass does not document incoming reasoning-effort controls, so the PR now omits the Codex effort selector for ClinePass models. The request profile still strips unsupported fields before forwarding.

Validated with npm run check and focused catalog/registry/routing tests (64 passing).

@duolahypercho

Copy link
Copy Markdown
Owner

Reviewed against the "Ship a new provider to every installer" checklist in AGENTS.md. Merges cleanly against current main despite 46 files having moved, and the whole suite is green — 495 tests, 490 pass, 0 fail, plus cargo test accepts_only_known_provider_ids and a clean swift build.

The credential work is genuinely solid, and that's the part that mattered most. I traced it end to end. config/clinepass/clinepass.json holds only pointers — env var, secret filename, keychain service — never a literal. upstreamHeaders strips any caller-supplied Authorization/X-Api-Key and injects the server-resolved key, and your new test in test/routing.test.mjs proves it by sending X-Api-Key: "must-not-forward" and asserting the upstream never sees it. doctor and support-bundle have zero ClinePass-specific code and only ever report credential presence. This matches the deepseek pattern with no deviation. SECURITY.md is updated correctly. And multiAgentVersion is correctly absent on all 12 models, so they retain conservative v1 — that restraint is the right call and I'm glad it's there.

Four things block it.

1. Tray usage badge is factually wrong for this provider. IslandOverlay.swift:592:

if provider.hasSuffix("-api") || provider == "deepseek" { return "METERED API" }
return "OAUTH ROUTE"

deepseek needed an explicit carve-out because its id doesn't end in -api. clinepass has exactly the same shape and didn't get one, so the Dynamic Island will label ClinePass traffic as OAUTH ROUTE when it's a plain metered API key. One-line fix.

2. No provider icon. Nothing under Sources/Resources/ProviderIcons/, no row in PROVIDER-ICON-SOURCES.md, and assetName at IslandOverlay.swift:936-945 has no clinepass case, so it falls back to the generic cpu mark. Providers currently missing icons (zai-coding, qwen-plan, ollama-cloud) all predate that rule and are grandfathered; this is new work created after it.

3. The per-model capability data looks templated. All 12 models declare an identical contextWindow: 131072 and autoCompact: 110000 — across DeepSeek, Kimi, GLM, Qwen, MiniMax, and MiMo families. Six unrelated vendors sharing one exact figure reads as a copied default rather than per-model research, and AGENTS.md is explicit: "Never infer... context size... from the model name." If any of these are wrong, auto_compact_token_limit is wrong too, which fails silently as context mismanagement rather than as an error. bin/discover-models clinepass would settle it once a key exists.

4. The catalog-shape change is novel and unverified. src/catalog.mjs:305-309 deletes default_reasoning_level and supported_reasoning_levels from the outward-facing entry. Every other requestProfile in the codebase only alters the outbound request body — this is the first to change what Codex's picker itself sees. For contrast, grok-oauth/grok-4.5 also has exactly one reasoning level and still keeps both keys. Your unit test proves the code does what it claims, but whether Codex's client renders correctly with those keys missing has never been observed, and that failure mode is a silently misrendered picker rather than an exception.

The request-profile work itself is right: the clinepass branch sits ahead of the else if chain so it's mutually exclusive by construction, and all 12 model files carry "requestProfile": "clinepass" — I checked every one.

What I'd suggest. Items 1 and 2 are mechanical. Item 3 needs either the docs or a discovery run. Then one authenticated ./bin/test-model 'clinepass/<model>' --live --yes plus a look at the real picker to confirm item 4. AGENTS.md is unambiguous that a provider isn't done until the checklist passes, and I'd rather not merge a provider we'd be claiming support for without having ever spoken to it.

I can't run the live check — it needs your key and spends your quota. If you'd rather not, say so and we can decide whether to land it explicitly marked experimental instead.

Minor, non-blocking: no dedicated branch in src/provider-account-usage.mjs, so ClinePass falls to the generic default. That's fine if ClinePass has no balance endpoint — worth a line in the PR saying so, since qwen-plan, ollama-cloud, and commandcode all got explicit branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants